DevTools: properly restore selected DOMNode in Elements panel.

third_party/WebKit/Source/devtools/front_end/elements/ElementsPanel.js

 /*
  * Copyright (C) 2007, 2008 Apple Inc.  All rights reserved.
  * Copyright (C) 2008 Matt Lilek <webkit@mattlilek.com>
  * Copyright (C) 2009 Joseph Pecoraro
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
@@ skipping 311 matching lines @@
      * @param {!WebInspector.Event} event
      */
     _selectedNodeChanged: function(event)
     {
         var selectedNode = /** @type {?WebInspector.DOMNode} */ (event.data);
         for (var i = 0; i < this._treeOutlines.length; ++i) {
             if (!selectedNode || selectedNode.domModel() !== this._treeOutlines[i].domModel())
                 this._treeOutlines[i].selectDOMNode(null);
         }
 
-        if (!selectedNode && this._lastValidSelectedNode)
-            this._selectedPathOnReset = this._lastValidSelectedNode.path();
-
         this._breadcrumbs.setSelectedNode(selectedNode);
 
         WebInspector.context.setFlavor(WebInspector.DOMNode, selectedNode);
 
         if (!selectedNode)
             return;
         selectedNode.setAsInspectedNode();
-        this._lastValidSelectedNode = selectedNode;
+        if (!this._isSettingDefaultSelection) {
+            this._selectedNodeOnReset = selectedNode;
+            this._hasNonDefaultSelectedNode = true;
+        }
 
         var executionContexts = selectedNode.target().runtimeModel.executionContexts();
         var nodeFrameId = selectedNode.frameId();
         for (var context of executionContexts) {
             if (context.frameId === nodeFrameId) {
                 WebInspector.context.setFlavor(WebInspector.ExecutionContext, context);
                 break;
             }
         }
     },
@@ skipping 22 matching lines @@
 
         var treeOutline = WebInspector.ElementsTreeOutline.forDOMModel(domModel);
         treeOutline.rootDOMNode = inspectedRootDocument;
 
         if (!inspectedRootDocument) {
             if (this.isShowing())
                 domModel.requestDocument();
             return;
         }
 
+        this._hasNonDefaultSelectedNode = false;
         WebInspector.domBreakpointsSidebarPane.restoreBreakpoints(inspectedRootDocument);
 
+        var defaultSelection = inspectedRootDocument.body || inspectedRootDocument.documentElement;
+        if (this._omitDefaultSelection || !defaultSelection)
+            return;
+
+        this._restoreNode(domModel, this._selectedNodeOnReset)
+            .then(onNodeRestored.bind(this, this._selectedNodeOnReset))

[dgozman, 2016/10/19 01:10:38]

missing semicolon

[lushnikov, 2016/10/19 20:52:11]

Done.

+
         /**
+         * @param {?WebInspector.DOMNode} staleNode
+         * @param {?WebInspector.DOMNode} restoredNode
          * @this {WebInspector.ElementsPanel}
-         * @param {?WebInspector.DOMNode} candidateFocusNode
          */
-        function selectNode(candidateFocusNode)
+        function onNodeRestored(staleNode, restoredNode)
         {
-            if (!candidateFocusNode)
-                candidateFocusNode = inspectedRootDocument.body || inspectedRootDocument.documentElement;
-
-            if (!candidateFocusNode)
+            if (staleNode !== this._selectedNodeOnReset)
                 return;
-
-            if (!this._pendingNodeReveal) {
-                this.selectDOMNode(candidateFocusNode);
-                if (treeOutline.selectedTreeElement)
-                    treeOutline.selectedTreeElement.expand();
+            if (restoredNode) {
+                this._preselectNode(restoredNode);
+                this._lastSelectedNodeSelectedForTest();
+            } else {
+                this._preselectNode(defaultSelection);

[dgozman, 2016/10/19 01:10:38]

Should recalc default selection.

[lushnikov, 2016/10/19 20:52:11]

Done.

             }
         }
-
-        /**
-         * @param {?DOMAgent.NodeId} nodeId
-         * @this {WebInspector.ElementsPanel}
-         */
-        function selectLastSelectedNode(nodeId)
-        {
-            if (this.selectedDOMNode()) {
-                // Focused node has been explicitly set while reaching out for the last selected node.
-                return;
-            }
-            var node = nodeId ? domModel.nodeForId(nodeId) : null;
-            selectNode.call(this, node);
-            this._lastSelectedNodeSelectedForTest();
-        }
-
-        if (this._omitDefaultSelection)
-            return;
-
-        if (this._selectedPathOnReset)
-            domModel.pushNodeByPathToFrontend(this._selectedPathOnReset, selectLastSelectedNode.bind(this));
-        else
-            selectNode.call(this, null);
-        delete this._selectedPathOnReset;
     },
 
     _lastSelectedNodeSelectedForTest: function() { },
 
     /**
+     * @param {!WebInspector.DOMNode} node
+     */
+    _preselectNode: function(node)

[dgozman, 2016/10/19 01:10:38]

_setDefaultSelectedNode

[dgozman, 2016/10/19 01:10:38]

Let's make this one responsible for default selection as well.

[lushnikov, 2016/10/19 20:52:11]

The default selection seems to fit better in the onNodeRestored method;
I left it there.

[lushnikov, 2016/10/19 20:52:11]

Done.

+    {
+        if (this._hasNonDefaultSelectedNode || this._pendingNodeReveal)
+            return;
+        var treeOutline = WebInspector.ElementsTreeOutline.forDOMModel(node.domModel());
+        if (!treeOutline)
+            return;
+        this._isSettingDefaultSelection = true;

[dgozman, 2016/10/19 01:10:38]

nit: _isSettingDefaultSelectedNode

[lushnikov, 2016/10/19 20:52:11]

Done.

+        this.selectDOMNode(node);
+        this._isSettingDefaultSelection = false;
+        if (treeOutline.selectedTreeElement)
+            treeOutline.selectedTreeElement.expand();
+    },
+
+    /**
+     * @param {!WebInspector.DOMModel} domModel
+     * @param {?WebInspector.DOMNode} staleNode
+     * @return {!Promise<?WebInspector.DOMNode>}
+     */
+    _restoreNode: function(domModel, staleNode)

[dgozman, 2016/10/19 01:10:38]

Let's make this inner function of _documentUpdated.

[lushnikov, 2016/10/19 20:52:11]

Done.

+    {
+        var nodePath = staleNode ? staleNode.path() : null;
+        if (!nodePath)
+            return Promise.resolve(/** @type {?WebInspector.DOMNode} */(null));
+        return domModel.pushNodeByPathToFrontendPromise(nodePath);

[dgozman, 2016/10/19 01:10:38]

Let's maybe use callback version?

[lushnikov, 2016/10/19 20:52:11]

Done.

+    },
+
+    /**
      * @override
      */
     searchCanceled: function()
     {
         delete this._searchQuery;
         this._hideSearchHighlights();
 
         this._searchableView.updateSearchMatchesCount(0);
 
         delete this._currentSearchResultIndex;
@@ skipping 268 matching lines @@
                 treeOutline.domModel().redo();
                 event.handled = true;
             }
         }
 
         if (WebInspector.isEditing() && event.keyCode !== WebInspector.KeyboardShortcut.Keys.F2.code)
             return;
 
         var treeOutline = null;
         for (var i = 0; i < this._treeOutlines.length; ++i) {
-            if (this._treeOutlines[i].selectedDOMNode() === this._lastValidSelectedNode)
+            if (this._treeOutlines[i].selectedDOMNode())
                 treeOutline = this._treeOutlines[i];
         }
         if (!treeOutline)
             return;
 
         if (!treeOutline.editing()) {
             handleUndoRedo.call(null, treeOutline);
             if (event.handled) {
                 this._stylesWidget.forceUpdate();
                 return;
@@ skipping 375 matching lines @@
     /**
      * @override
      * @param {!WebInspector.DOMNode} node
      * @return {?{title: string, color: string}}
      */
     decorate: function(node)
     {
         return { color: "orange", title: WebInspector.UIString("Element state: %s", ":" + WebInspector.CSSModel.fromNode(node).pseudoState(node).join(", :")) };
     }
 }