Changeset 184130 in webkit


Ignore:
Timestamp:
May 11, 2015, 4:18:14 PM (11 years ago)
Author:
timothy@apple.com
Message:

Web Inspector: REGRESSION (Tabs): Issues reloading a resource with breakpoints
​https://bugs.webkit.org/show_bug.cgi?id=144650

Fix a number of issues with Debugger tab and navigation/reloading:

  • Close old content views in the Debugger tab when main frame navigates.
  • Prune old resource tree elements before attempting to restore a cookie that might match an old resource.
  • Allow breakpoint selections to be restored from a saved cookie.
  • Fix an assert when closing a content view that isn't the current index, but is the current view.
  • Avoid calling closed() multiple times when a ContentView is in the back/forward list more than once.
  • Make restoreStateFromCookie properly set and use the causedByNavigation argument for a longer restore delay.
  • Create a new cookie object per tab instead of it being cumulative from the previous cookie.

Reviewed by Brian Burg.

  • UserInterface/Base/Main.js:

(WebInspector._mainResourceDidChange): Delay calling _restoreCookieForOpenTabs to give time for sidebars
and tabs to respond to the main resource change.
(WebInspector._restoreCookieForOpenTabs): Rename causedByReload to causedByNavigation. Nothing special about
reload since we restore on all navigation.

  • UserInterface/Views/ContentView.js:

(WebInspector.ContentView): Support Breakpoint as a represented object, which happens during a cookie restore.
(WebInspector.ContentView.isViewable): Ditto.

  • UserInterface/Views/ContentViewContainer.js:

(WebInspector.ContentViewContainer.prototype.closeAllContentViews): Disassociate if the view is current and not just
the current entry index. This matches other close functions. This fixes an assert in _disassociateFromContentView.
(WebInspector.ContentViewContainer.prototype._disassociateFromContentView): Don't disassociate multiple times. This
avoids calling the closed() function on a view more than once.

  • UserInterface/Views/DebuggerSidebarPanel.js:

(WebInspector.DebuggerSidebarPanel.prototype.saveStateToCookie):
(WebInspector.DebuggerSidebarPanel.prototype._mainResourceDidChange): Renamed from _mainResourceChanged.
Close all content views if this is the main frame. Also prune all old resources. Doing this now avoids a flash
of having old and new resources in the tree caused by the default delay in NavigationSidebarPanel's _checkForOldResources.

  • UserInterface/Views/NavigationSidebarPanel.js:

(WebInspector.NavigationSidebarPanel): Set _autoPruneOldTopLevelResourceTreeElements for later.
(WebInspector.NavigationSidebarPanel.prototype.get contentTreeOutlineToAutoPrune): Deleted.
(WebInspector.NavigationSidebarPanel.prototype.showDefaultContentView): Fix typo.
(WebInspector.NavigationSidebarPanel.prototype.showDefaultContentViewForTreeElement): Fix whitespace.
(WebInspector.NavigationSidebarPanel.prototype.pruneOldResourceTreeElements): Added. Broken out from
_checkForOldResources.delayedWork so it can be called manually. Also check all visible tree outlines.
(WebInspector.NavigationSidebarPanel.prototype._treeElementAddedOrChanged): Pass treeElement in an array.
(WebInspector.NavigationSidebarPanel.prototype._checkForOldResourcesIfNeeded): Added.
(WebInspector.NavigationSidebarPanel.prototype._checkForOldResources): Call pruneOldResourceTreeElements on a delay.
(WebInspector.NavigationSidebarPanel.prototype._checkForOldResources.delayedWork): Deleted.
(WebInspector.NavigationSidebarPanel.prototype._checkOutlinesForPendingViewStateCookie): Call _checkForOldResourcesIfNeeded.
(WebInspector.NavigationSidebarPanel.prototype._checkElementsForPendingViewStateCookie): Remove array folding code.

  • UserInterface/Views/TabContentView.js:

(WebInspector.TabContentView.prototype.restoreStateFromCookie): Rename causedByReload to causedByNavigation.
(WebInspector.TabContentView.prototype.saveStateToCookie): Don't allow the cookie to build on the old cookie.

Location:
trunk/Source/WebInspectorUI
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebInspectorUI/ChangeLog

    r184108 r184130  
     12015-05-11  Timothy Hatcher  <timothy@apple.com>
     2
     3        Web Inspector: REGRESSION (Tabs): Issues reloading a resource with breakpoints
     4        https://bugs.webkit.org/show_bug.cgi?id=144650
     5
     6        Fix a number of issues with Debugger tab and navigation/reloading:
     7        - Close old content views in the Debugger tab when main frame navigates.
     8        - Prune old resource tree elements before attempting to restore a cookie that might match an old resource.
     9        - Allow breakpoint selections to be restored from a saved cookie.
     10        - Fix an assert when closing a content view that isn't the current index, but is the current view.
     11        - Avoid calling closed() multiple times when a ContentView is in the back/forward list more than once.
     12        - Make restoreStateFromCookie properly set and use the causedByNavigation argument for a longer restore delay.
     13        - Create a new cookie object per tab instead of it being cumulative from the previous cookie.
     14
     15        Reviewed by Brian Burg.
     16
     17        * UserInterface/Base/Main.js:
     18        (WebInspector._mainResourceDidChange): Delay calling _restoreCookieForOpenTabs to give time for sidebars
     19        and tabs to respond to the main resource change.
     20        (WebInspector._restoreCookieForOpenTabs): Rename causedByReload to causedByNavigation. Nothing special about
     21        reload since we restore on all navigation.
     22
     23        * UserInterface/Views/ContentView.js:
     24        (WebInspector.ContentView): Support Breakpoint as a represented object, which happens during a cookie restore.
     25        (WebInspector.ContentView.isViewable): Ditto.
     26
     27        * UserInterface/Views/ContentViewContainer.js:
     28        (WebInspector.ContentViewContainer.prototype.closeAllContentViews): Disassociate if the view is current and not just
     29        the current entry index. This matches other close functions. This fixes an assert in _disassociateFromContentView.
     30        (WebInspector.ContentViewContainer.prototype._disassociateFromContentView): Don't disassociate multiple times. This
     31        avoids calling the closed() function on a view more than once.
     32
     33        * UserInterface/Views/DebuggerSidebarPanel.js:
     34        (WebInspector.DebuggerSidebarPanel.prototype.saveStateToCookie):
     35        (WebInspector.DebuggerSidebarPanel.prototype._mainResourceDidChange): Renamed from _mainResourceChanged.
     36        Close all content views if this is the main frame. Also prune all old resources. Doing this now avoids a flash
     37        of having old and new resources in the tree caused by the default delay in NavigationSidebarPanel's _checkForOldResources.
     38
     39        * UserInterface/Views/NavigationSidebarPanel.js:
     40        (WebInspector.NavigationSidebarPanel): Set _autoPruneOldTopLevelResourceTreeElements for later.
     41        (WebInspector.NavigationSidebarPanel.prototype.get contentTreeOutlineToAutoPrune): Deleted.
     42        (WebInspector.NavigationSidebarPanel.prototype.showDefaultContentView): Fix typo.
     43        (WebInspector.NavigationSidebarPanel.prototype.showDefaultContentViewForTreeElement): Fix whitespace.
     44        (WebInspector.NavigationSidebarPanel.prototype.pruneOldResourceTreeElements): Added. Broken out from
     45        _checkForOldResources.delayedWork so it can be called manually. Also check all visible tree outlines.
     46        (WebInspector.NavigationSidebarPanel.prototype._treeElementAddedOrChanged): Pass treeElement in an array.
     47        (WebInspector.NavigationSidebarPanel.prototype._checkForOldResourcesIfNeeded): Added.
     48        (WebInspector.NavigationSidebarPanel.prototype._checkForOldResources): Call pruneOldResourceTreeElements on a delay.
     49        (WebInspector.NavigationSidebarPanel.prototype._checkForOldResources.delayedWork): Deleted.
     50        (WebInspector.NavigationSidebarPanel.prototype._checkOutlinesForPendingViewStateCookie): Call _checkForOldResourcesIfNeeded.
     51        (WebInspector.NavigationSidebarPanel.prototype._checkElementsForPendingViewStateCookie): Remove array folding code.
     52
     53        * UserInterface/Views/TabContentView.js:
     54        (WebInspector.TabContentView.prototype.restoreStateFromCookie): Rename causedByReload to causedByNavigation.
     55        (WebInspector.TabContentView.prototype.saveStateToCookie): Don't allow the cookie to build on the old cookie.
     56
    1572015-05-11  Timothy Hatcher  <timothy@apple.com>
    258

  • trunk/Source/WebInspectorUI/UserInterface/Base/Main.js

    r184045 r184130  
    11261126    this._inProvisionalLoad = false;
    11271127
    1128     this._restoreCookieForOpenTabs();
     1128    // Run cookie restoration after we are sure all of the Tabs and NavigationSidebarPanels
     1129    // have updated with respect to the main resource change.
     1130    setTimeout(this._restoreCookieForOpenTabs.bind(this, true));
    11291131
    11301132    this._updateDownloadToolbarButton();
    … …  
    11431145};
    11441146
    1145 WebInspector._restoreCookieForOpenTabs = function(causedByReload)
     1147WebInspector._restoreCookieForOpenTabs = function(causedByNavigation)
    11461148{
    11471149    for (var tabBarItem of this.tabBar.tabBarItems) {
    … …  
    11491151        if (!(tabContentView instanceof WebInspector.TabContentView))
    11501152            continue;
    1151         tabContentView.restoreStateFromCookie(causedByReload);
     1153        tabContentView.restoreStateFromCookie(causedByNavigation);
    11521154    }
    11531155};

  • trunk/Source/WebInspectorUI/UserInterface/Views/ContentView.js

    r183816 r184130  
    5858        }
    5959
     60        if (representedObject instanceof WebInspector.Breakpoint) {
     61            if (representedObject.sourceCodeLocation)
     62                return new WebInspector.ContentView(representedObject.sourceCodeLocation.displaySourceCode, extraArguments);
     63        }
     64
    6065        if (representedObject instanceof WebInspector.DOMStorageObject)
    6166            return new WebInspector.DOMStorageContentView(representedObject, extraArguments);
    … …  
    148153    if (representedObject instanceof WebInspector.Timeline)
    149154        return true;
     155    if (representedObject instanceof WebInspector.Breakpoint)
     156        return representedObject.sourceCodeLocation;
    150157    if (representedObject instanceof WebInspector.DOMStorageObject)
    151158        return true;

  • trunk/Source/WebInspectorUI/UserInterface/Views/ContentViewContainer.js

    r183733 r184130  
    280280        }
    281281
    282         var oldCurrentContentView = this.currentContentView;
     282        var visibleContentView = this.currentContentView;
    283283
    284284        var backForwardListDidChange = false;
    … …  
    289289                continue;
    290290
    291             if (entry.contentView === oldCurrentContentView)
     291            if (entry.contentView === visibleContentView)
    292292                this._hideEntry(entry);
    293293
    … …  
    308308        console.assert(currentEntry || (!currentEntry && this._currentIndex === -1));
    309309
    310         if (currentEntry && currentEntry.contentView !== oldCurrentContentView || backForwardListDidChange) {
     310        if (currentEntry && currentEntry.contentView !== visibleContentView || backForwardListDidChange) {
    311311            this._showEntry(currentEntry, true);
    312312            this.dispatchEventToListeners(WebInspector.ContentViewContainer.Event.CurrentContentViewDidChange);
    … …  
    336336        }
    337337
    338         var oldCurrentContentView = this.currentContentView;
     338        var visibleContentView = this.currentContentView;
    339339
    340340        var backForwardListDidChange = false;
    … …  
    345345                continue;
    346346
    347             if (entry.contentView === oldCurrentContentView)
     347            if (entry.contentView === visibleContentView)
    348348                this._hideEntry(entry);
    349349
    … …  
    364364        console.assert(currentEntry || (!currentEntry && this._currentIndex === -1));
    365365
    366         if (currentEntry && currentEntry.contentView !== oldCurrentContentView || backForwardListDidChange) {
     366        if (currentEntry && currentEntry.contentView !== visibleContentView || backForwardListDidChange) {
    367367            this._showEntry(currentEntry, true);
    368368            this.dispatchEventToListeners(WebInspector.ContentViewContainer.Event.CurrentContentViewDidChange);
    … …  
    376376            return;
    377377        }
     378
     379        var visibleContentView = this.currentContentView;
    378380
    379381        // Hide and disassociate with all the content views.
    380382        for (var i = 0; i < this._backForwardList.length; ++i) {
    381383            var entry = this._backForwardList[i];
    382             if (i === this._currentIndex)
     384            if (entry.contentView === visibleContentView)
    383385                this._hideEntry(entry);
    384386            this._disassociateFromContentView(entry.contentView);
    … …  
    450452    {
    451453        console.assert(!contentView.visible);
     454
     455        if (!contentView._parentContainer)
     456            return;
    452457
    453458        contentView._parentContainer = null;

  • trunk/Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js

    r183768 r184130  
    3232        this.contentBrowser = contentBrowser;
    3333
    34         WebInspector.Frame.addEventListener(WebInspector.Frame.Event.MainResourceDidChange, this._mainResourceChanged, this);
     34        WebInspector.Frame.addEventListener(WebInspector.Frame.Event.MainResourceDidChange, this._mainResourceDidChange, this);
    3535        WebInspector.Frame.addEventListener(WebInspector.Frame.Event.ResourceWasAdded, this._resourceAdded, this);
    3636
    … …  
    228228        var representedObject = selectedTreeElement.representedObject;
    229229
    230         if (representedObject === WebInspector.debuggerManager.allExceptionsBreakpoint)
     230        if (representedObject === WebInspector.debuggerManager.allExceptionsBreakpoint) {
    231231            cookie[WebInspector.DebuggerSidebarPanel.SelectedAllExceptionsCookieKey] = true;
    232 
    233         if (representedObject === WebInspector.debuggerManager.allUncaughtExceptionsBreakpoint)
     232            return;
     233        }
     234
     235        if (representedObject === WebInspector.debuggerManager.allUncaughtExceptionsBreakpoint) {
    234236            cookie[WebInspector.DebuggerSidebarPanel.SelectedAllUncaughtExceptionsCookieKey] = true;
     237            return;
     238        }
    235239
    236240        super.saveStateToCookie(cookie);
    … …  
    379383    }
    380384
    381     _mainResourceChanged(event)
    382     {
     385    _mainResourceDidChange(event)
     386    {
     387        if (event.target.isMainFrame()) {
     388            // Aggressively prune resources now so the old resources are removed before
     389            // the new main resource is added below. This avoids a visual flash when the
     390            // prune normally happens on a later event loop cycle.
     391            this.pruneStaleResourceTreeElements();
     392            this.contentBrowser.contentViewContainer.closeAllContentViews();
     393        }
     394
    383395        var resource = event.target.mainResource;
    384396        this._addTreeElementForSourceCodeToContentTreeOutline(resource);

  • trunk/Source/WebInspectorUI/UserInterface/Views/NavigationSidebarPanel.js

    r184108 r184130  
    2626WebInspector.NavigationSidebarPanel = class NavigationSidebarPanel extends WebInspector.SidebarPanel
    2727{
    28     constructor(identifier, displayName, autoPruneOldTopLevelResourceTreeElements, wantsTopOverflowShadow, element, role, label)
     28    constructor(identifier, displayName, shouldAutoPruneStaleTopLevelResourceTreeElements, wantsTopOverflowShadow, element, role, label)
    2929    {
    3030        super(identifier, displayName, element, role, label || displayName);
    … …  
    6969        this._generateDisclosureTrianglesIfNeeded();
    7070
    71         if (autoPruneOldTopLevelResourceTreeElements) {
    72             WebInspector.Frame.addEventListener(WebInspector.Frame.Event.MainResourceDidChange, this._checkForOldResources, this);
    73             WebInspector.Frame.addEventListener(WebInspector.Frame.Event.ChildFrameWasRemoved, this._checkForOldResources, this);
    74             WebInspector.Frame.addEventListener(WebInspector.Frame.Event.ResourceWasRemoved, this._checkForOldResources, this);
     71        this._shouldAutoPruneStaleTopLevelResourceTreeElements = shouldAutoPruneStaleTopLevelResourceTreeElements || false;
     72
     73        if (this._shouldAutoPruneStaleTopLevelResourceTreeElements) {
     74            WebInspector.Frame.addEventListener(WebInspector.Frame.Event.MainResourceDidChange, this._checkForStaleResources, this);
     75            WebInspector.Frame.addEventListener(WebInspector.Frame.Event.ChildFrameWasRemoved, this._checkForStaleResources, this);
     76            WebInspector.Frame.addEventListener(WebInspector.Frame.Event.ResourceWasRemoved, this._checkForStaleResources, this);
    7577        }
    7678    }
    … …  
    120122
    121123        this._updateFilter();
    122     }
    123 
    124     get contentTreeOutlineToAutoPrune()
    125     {
    126         return this._contentTreeOutline;
    127124    }
    128125
    … …  
    187184    showDefaultContentView()
    188185    {
    189         // Implemneted by subclasses if needed to show a content view when no existing tree element is selected.
     186        // Implemented by subclasses if needed to show a content view when no existing tree element is selected.
    190187    }
    191188
    … …  
    196193        if (!treeElement || !treeElement.representedObject)
    197194            return;
     195
    198196        this.contentBrowser.showContentViewForRepresentedObject(treeElement.representedObject);
    199197        treeElement.revealAndSelect(true, false, true, true);
    … …  
    418416    }
    419417
     418    // Protected
     419
     420    pruneStaleResourceTreeElements()
     421    {
     422        if (this._checkForStaleResourcesTimeoutIdentifier) {
     423            clearTimeout(this._checkForStaleResourcesTimeoutIdentifier);
     424            this._checkForStaleResourcesTimeoutIdentifier = undefined;
     425        }
     426
     427        for (var contentTreeOutline of this._visibleContentTreeOutlines) {
     428            // Check all the ResourceTreeElements at the top level to make sure their Resource still has a parentFrame in the frame hierarchy.
     429            // If the parentFrame is no longer in the frame hierarchy we know it was removed due to a navigation or some other page change and
     430            // we should remove the issues for that resource.
     431            for (var i = contentTreeOutline.children.length - 1; i >= 0; --i) {
     432                var treeElement = contentTreeOutline.children[i];
     433                if (!(treeElement instanceof WebInspector.ResourceTreeElement))
     434                    continue;
     435
     436                var resource = treeElement.resource;
     437                if (!resource.parentFrame || resource.parentFrame.isDetached())
     438                    contentTreeOutline.removeChildAtIndex(i, true, true);
     439            }
     440        }
     441    }
     442
    420443    // Private
    421444   
    … …  
    532555
    533556        if (this.selected)
    534             this._checkElementsForPendingViewStateCookie(treeElement);
     557            this._checkElementsForPendingViewStateCookie([treeElement]);
    535558
    536559        this.treeElementAddedOrChanged(treeElement);
    … …  
    589612    }
    590613
    591     _checkForOldResources(event)
    592     {
    593         if (this._checkForOldResourcesTimeoutIdentifier)
    594             return;
    595 
    596         function delayedWork()
    597         {
    598             delete this._checkForOldResourcesTimeoutIdentifier;
    599 
    600             var contentTreeOutline = this.contentTreeOutlineToAutoPrune;
    601 
    602             // Check all the ResourceTreeElements at the top level to make sure their Resource still has a parentFrame in the frame hierarchy.
    603             // If the parentFrame is no longer in the frame hierarchy we know it was removed due to a navigation or some other page change and
    604             // we should remove the issues for that resource.
    605             for (var i = contentTreeOutline.children.length - 1; i >= 0; --i) {
    606                 var treeElement = contentTreeOutline.children[i];
    607                 if (!(treeElement instanceof WebInspector.ResourceTreeElement))
    608                     continue;
    609 
    610                 var resource = treeElement.resource;
    611                 if (!resource.parentFrame || resource.parentFrame.isDetached())
    612                     contentTreeOutline.removeChildAtIndex(i, true, true);
    613             }
    614 
    615             if (typeof this._updateEmptyContentPlaceholder === "function")
    616                 this._updateEmptyContentPlaceholder();
    617         }
    618 
    619         // Check on a delay to coalesce multiple calls to _checkForOldResources.
    620         this._checkForOldResourcesTimeoutIdentifier = setTimeout(delayedWork.bind(this), 0);
     614    _checkForStaleResourcesIfNeeded()
     615    {
     616        if (!this._checkForStaleResourcesTimeoutIdentifier || !this._shouldAutoPruneStaleTopLevelResourceTreeElements)
     617            return;
     618        this.pruneStaleResourceTreeElements();
     619    }
     620
     621    _checkForStaleResources(event)
     622    {
     623        console.assert(this._shouldAutoPruneStaleTopLevelResourceTreeElements);
     624
     625        if (this._checkForStaleResourcesTimeoutIdentifier)
     626            return;
     627
     628        // Check on a delay to coalesce multiple calls to _checkForStaleResources.
     629        this._checkForStaleResourcesTimeoutIdentifier = setTimeout(this.pruneStaleResourceTreeElements.bind(this));
    621630    }
    622631
    … …  
    633642        if (!this._pendingViewStateCookie)
    634643            return;
     644
     645        this._checkForStaleResourcesIfNeeded();
    635646
    636647        var visibleTreeElements = [];
    … …  
    678689            });
    679690        }
    680 
    681         if (!(treeElements instanceof Array))
    682             treeElements = [treeElements];
    683691
    684692        var matchedElement = null;

  • trunk/Source/WebInspectorUI/UserInterface/Views/TabContentView.js

    r183340 r184130  
    108108    },
    109109
    110     restoreStateFromCookie: function(causedByReload)
     110    restoreStateFromCookie: function(causedByNavigation)
    111111    {
    112112        if (!this.navigationSidebarPanel)
    113113            return;
    114114
    115         var matchTypeOnlyDelayForReload = 2000;
     115        var matchTypeOnlyDelayForNavigation = 2000;
    116116        var matchTypeOnlyDelayForReopen = 1000;
    117117
    118         var relaxMatchDelay = causedByReload ? matchTypeOnlyDelayForReload : matchTypeOnlyDelayForReopen;
     118        var relaxMatchDelay = causedByNavigation ? matchTypeOnlyDelayForNavigation : matchTypeOnlyDelayForReopen;
    119119        this.navigationSidebarPanel.restoreStateFromCookie(this._cookieSetting.value || {}, relaxMatchDelay);
    120120    },
    … …  
    125125            return;
    126126
    127         var cookie = this._cookieSetting.value || {};
     127        var cookie = {};
    128128        this.navigationSidebarPanel.saveStateToCookie(cookie);
    129129        this._cookieSetting.value = cookie;
Note: See TracChangeset for help on using the changeset viewer.