Title: [212166] trunk/Source/WebInspectorUI
Revision
212166
Author
mattba...@apple.com
Date
2017-02-10 16:44:42 -0800 (Fri, 10 Feb 2017)

Log Message

Web Inspector: ContentViewContainer can have redundant back-forward entries after ContentView close
https://bugs.webkit.org/show_bug.cgi?id=168105

Reviewed by Joseph Pecoraro.

* UserInterface/Models/BackForwardEntry.js:
(WebInspector.BackForwardEntry.prototype.isEqual):
Make check for equal ContentView/cookie reusable.

* UserInterface/Views/ContentViewContainer.js:
(WebInspector.ContentViewContainer.prototype.showContentView):
Use BackForwardEntry.prototype.isEqual.
(WebInspector.ContentViewContainer.prototype.replaceContentView):
(WebInspector.ContentViewContainer.prototype.closeContentView):
Clean-up the BackForwardEntry list after changes that can cause
identical entries to become adjacent.

(WebInspector.ContentViewContainer.prototype._removeIdenticalAdjacentBackForwardEntries):
Remove consecutive entries with the same ContentView and cookie.
(WebInspector.ContentViewContainer):
(WebInspector.ContentViewContainer.closeAllContentViewsOfPrototype): Deleted.
Drive-by cleanup: removed dead code.

Modified Paths

Diff

Modified: trunk/Source/WebInspectorUI/ChangeLog (212165 => 212166)


--- trunk/Source/WebInspectorUI/ChangeLog	2017-02-11 00:19:06 UTC (rev 212165)
+++ trunk/Source/WebInspectorUI/ChangeLog	2017-02-11 00:44:42 UTC (rev 212166)
@@ -1,5 +1,30 @@
 2017-02-10  Matt Baker  <mattba...@apple.com>
 
+        Web Inspector: ContentViewContainer can have redundant back-forward entries after ContentView close
+        https://bugs.webkit.org/show_bug.cgi?id=168105
+
+        Reviewed by Joseph Pecoraro.
+
+        * UserInterface/Models/BackForwardEntry.js:
+        (WebInspector.BackForwardEntry.prototype.isEqual):
+        Make check for equal ContentView/cookie reusable.
+
+        * UserInterface/Views/ContentViewContainer.js:
+        (WebInspector.ContentViewContainer.prototype.showContentView):
+        Use BackForwardEntry.prototype.isEqual.
+        (WebInspector.ContentViewContainer.prototype.replaceContentView):
+        (WebInspector.ContentViewContainer.prototype.closeContentView):
+        Clean-up the BackForwardEntry list after changes that can cause
+        identical entries to become adjacent.
+
+        (WebInspector.ContentViewContainer.prototype._removeIdenticalAdjacentBackForwardEntries):
+        Remove consecutive entries with the same ContentView and cookie.
+        (WebInspector.ContentViewContainer):
+        (WebInspector.ContentViewContainer.closeAllContentViewsOfPrototype): Deleted.
+        Drive-by cleanup: removed dead code.
+
+2017-02-10  Matt Baker  <mattba...@apple.com>
+
         REGRESSION (r211829): Web Inspector: Elements tab is blank when added after page load
         https://bugs.webkit.org/show_bug.cgi?id=168142
 

Modified: trunk/Source/WebInspectorUI/UserInterface/Models/BackForwardEntry.js (212165 => 212166)


--- trunk/Source/WebInspectorUI/UserInterface/Models/BackForwardEntry.js	2017-02-11 00:19:06 UTC (rev 212165)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/BackForwardEntry.js	2017-02-11 00:44:42 UTC (rev 212166)
@@ -97,6 +97,13 @@
         this._saveScrollPositions();
     }
 
+    isEqual(other)
+    {
+        if (!other)
+            return false;
+        return this._contentView === other._contentView && Object.shallowEqual(this._cookie, other._cookie);
+    }
+
     // Private
 
     _restoreFromCookie()

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/ContentViewContainer.js (212165 => 212166)


--- trunk/Source/WebInspectorUI/UserInterface/Views/ContentViewContainer.js	2017-02-11 00:19:06 UTC (rev 212165)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/ContentViewContainer.js	2017-02-11 00:44:42 UTC (rev 212166)
@@ -106,7 +106,7 @@
             provisionalEntry = new WebInspector.BackForwardEntry(contentView, cookie);
 
         // Don't do anything if we would have added an identical back/forward list entry.
-        if (currentEntry && currentEntry.contentView === contentView && Object.shallowEqual(provisionalEntry.cookie, currentEntry.cookie)) {
+        if (provisionalEntry.isEqual(currentEntry)) {
             const shouldCallShown = false;
             currentEntry.prepareToShow(shouldCallShown);
             return currentEntry.contentView;
@@ -203,6 +203,8 @@
             }
         }
 
+        this._removeIdenticalAdjacentBackForwardEntries();
+
         // Re-show the current entry, because its content view instance was replaced.
         if (currentlyShowing) {
             this._showEntry(this.currentBackForwardEntry, true);
@@ -210,62 +212,6 @@
         }
     }
 
-    closeAllContentViewsOfPrototype(constructor)
-    {
-        if (!this._backForwardList.length) {
-            console.assert(this._currentIndex === -1);
-            return;
-        }
-
-        // Do a check to see if all the content views are instances of this prototype.
-        // If they all are we can use the quicker closeAllContentViews method.
-        var allSamePrototype = true;
-        for (var i = this._backForwardList.length - 1; i >= 0; --i) {
-            if (!(this._backForwardList[i].contentView instanceof constructor)) {
-                allSamePrototype = false;
-                break;
-            }
-        }
-
-        if (allSamePrototype) {
-            this.closeAllContentViews();
-            return;
-        }
-
-        var visibleContentView = this.currentContentView;
-
-        var backForwardListDidChange = false;
-        // Hide and disassociate with all the content views that are instances of the constructor.
-        for (var i = this._backForwardList.length - 1; i >= 0; --i) {
-            var entry = this._backForwardList[i];
-            if (!(entry.contentView instanceof constructor))
-                continue;
-
-            if (entry.contentView === visibleContentView)
-                this._hideEntry(entry);
-
-            if (this._currentIndex >= i) {
-                // Decrement the currentIndex since we will remove an item in the back/forward array
-                // that it the current index or comes before it.
-                --this._currentIndex;
-            }
-
-            this._disassociateFromContentView(entry.contentView, entry.tombstone);
-
-            // Remove the item from the back/forward list.
-            this._backForwardList.splice(i, 1);
-            backForwardListDidChange = true;
-        }
-
-        var currentEntry = this.currentBackForwardEntry;
-        console.assert(currentEntry || (!currentEntry && this._currentIndex === -1));
-
-        if (currentEntry && currentEntry.contentView !== visibleContentView || backForwardListDidChange) {
-            this._showEntry(currentEntry, true);
-            this.dispatchEventToListeners(WebInspector.ContentViewContainer.Event.CurrentContentViewDidChange);
-        }
-    }
-
     closeContentView(contentViewToClose)
     {
         if (!this._backForwardList.length) {
@@ -289,7 +235,6 @@
         }
 
         var visibleContentView = this.currentContentView;
-
         var backForwardListDidChange = false;
         // Hide and disassociate with all the content views that are the same as contentViewToClose.
         for (var i = this._backForwardList.length - 1; i >= 0; --i) {
@@ -302,7 +247,7 @@
 
             if (this._currentIndex >= i) {
                 // Decrement the currentIndex since we will remove an item in the back/forward array
-                // that it the current index or comes before it.
+                // that is the current index or comes before it.
                 --this._currentIndex;
             }
 
@@ -313,6 +258,9 @@
             backForwardListDidChange = true;
         }
 
+        if (backForwardListDidChange)
+            this._removeIdenticalAdjacentBackForwardEntries();
+
         var currentEntry = this.currentBackForwardEntry;
         console.assert(currentEntry || (!currentEntry && this._currentIndex === -1));
 
@@ -518,6 +466,29 @@
             tombstoneContentViewContainers = contentView[WebInspector.ContentViewContainer.TombstoneContentViewContainersSymbol] = [];
         return tombstoneContentViewContainers;
     }
+
+    _removeIdenticalAdjacentBackForwardEntries()
+    {
+        if (this._backForwardList.length < 2)
+            return;
+
+        let previousEntry;
+        for (let i = this._backForwardList.length - 1; i >= 0; --i) {
+            let entry = this._backForwardList[i];
+            if (!entry.isEqual(previousEntry)) {
+                previousEntry = entry;
+                continue;
+            }
+
+           if (this._currentIndex >= i) {
+                // Decrement the currentIndex since we will remove an item in the back/forward array
+                // that is the current index or comes before it.
+                --this._currentIndex;
+            }
+
+            this._backForwardList.splice(i, 1);
+        }
+    }
 };
 
 WebInspector.ContentViewContainer.Event = {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to