Skip to content

Commit

Permalink
Revert of DevTools: keep widgets in widget hierarchy upon hide, split…
Browse files Browse the repository at this point in the history 
… attach/detach cycle from show/hide. (patchset #3 id:40001 of https://codereview.chromium.org/2157363006/ )

Reason for revert:
REverting as this broke devtools on Tip-of-tree

[17701:17701:0721/140403:ERROR:CONSOLE(266)] "Uncaught (in promise) NotFoundError: Failed to execute 'insertBefore' on 'Node': The node before which the new node is to be inserted is not a child of this node.", source: chrome-devtools://devtools/bundled/ui/Widget.js (266)

TBR=pfeldman, dgozman

NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Review-Url: https://codereview.chromium.org/2170063002
Cr-Commit-Position: refs/heads/master@{#406938}
  • Loading branch information
@aslushnikov
aslushnikov authored and Commit bot committed Jul 21, 2016
1 parent e0158a2 commit 5db55e4
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 96 deletions.
4 changes: 2 additions & 2 deletions front_end/extensions/ExtensionPanel.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ WebInspector.ExtensionSidebarPane.prototype = {
delete this._objectPropertiesView;
}
if (this._extensionView)
this._extensionView.detach();
this._extensionView.detach(true);

this._extensionView = new WebInspector.ExtensionView(this._server, this._id, url, "extension fill");
this._extensionView.show(this.element);
Expand Down Expand Up @@ -286,7 +286,7 @@ WebInspector.ExtensionSidebarPane.prototype = {
if (this._objectPropertiesView)
return;
if (this._extensionView) {
this._extensionView.detach();
this._extensionView.detach(true);
delete this._extensionView;
}
this._objectPropertiesView = new WebInspector.ExtensionNotifierView(this._server, this._id);
Expand Down
2 changes: 1 addition & 1 deletion front_end/main/Main.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1023,7 +1023,7 @@ WebInspector.TargetCrashedScreen.show = function(debuggerModel)
dialog.setWrapsContent(true);
dialog.addCloseButton();
dialog.setDimmed(true);
var hideBound = dialog.detach.bind(dialog);
var hideBound = dialog.detach.bind(dialog, false);
debuggerModel.addEventListener(WebInspector.DebuggerModel.Events.GlobalObjectCleared, hideBound);

new WebInspector.TargetCrashedScreen(onHide).show(dialog.element);
Expand Down
2 changes: 1 addition & 1 deletion front_end/ui/Dialog.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ WebInspector.Dialog.prototype = {
{
var closeButton = this.contentElement.createChild("div", "dialog-close-button", "dt-close-button");
closeButton.gray = true;
closeButton.addEventListener("click", this.detach.bind(this), false);
closeButton.addEventListener("click", this.detach.bind(this, false), false);
},

/**
Expand Down
16 changes: 7 additions & 9 deletions front_end/ui/SplitWidget.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,8 @@ WebInspector.SplitWidget.prototype = {
if (widget) {
widget.element.classList.add("insertion-point-main");
widget.element.classList.remove("insertion-point-sidebar");
widget.attach(this.element, this._sidebarWidget ? this._sidebarWidget.element : null);
if (this._showMode === WebInspector.SplitWidget.ShowMode.OnlyMain || this._showMode === WebInspector.SplitWidget.ShowMode.Both)
widget.showWidget();
widget.show(this.element);
}
},

Expand All @@ -177,9 +176,8 @@ WebInspector.SplitWidget.prototype = {
if (widget) {
widget.element.classList.add("insertion-point-sidebar");
widget.element.classList.remove("insertion-point-main");
widget.attach(this.element);
if (this._showMode === WebInspector.SplitWidget.ShowMode.OnlySidebar || this._showMode === WebInspector.SplitWidget.ShowMode.Both)
widget.showWidget();
widget.show(this.element);
}
},

Expand Down Expand Up @@ -318,13 +316,13 @@ WebInspector.SplitWidget.prototype = {
if (sideToShow) {
// Make sure main is first in the children list.
if (sideToShow === this._mainWidget)
this._mainWidget.showWidget();
this._mainWidget.show(this.element, this._sidebarWidget ? this._sidebarWidget.element : null);
else
this._sidebarWidget.showWidget();
this._sidebarWidget.show(this.element);
}
if (sideToHide) {
this._detaching = true;
sideToHide.hideWidget();
sideToHide.detach();
delete this._detaching;
}

Expand Down Expand Up @@ -382,9 +380,9 @@ WebInspector.SplitWidget.prototype = {

// Make sure main is the first in the children list.
if (this._sidebarWidget)
this._sidebarWidget.showWidget();
this._sidebarWidget.show(this.element);
if (this._mainWidget)
this._mainWidget.showWidget();
this._mainWidget.show(this.element, this._sidebarWidget ? this._sidebarWidget.element : null);
// Order widgets in DOM properly.
this.setSecondIsSidebar(this._secondIsSidebar);

Expand Down
22 changes: 8 additions & 14 deletions front_end/ui/TabbedPane.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,6 @@ WebInspector.TabbedPane.prototype = {
else
this._tabs.push(tab);
this._tabsHistory.push(tab);
view.attach(this.element);
if (this._tabsHistory[0] === tab && this.isShowing())
this.selectTab(tab.id, userGesture);
this._updateTabElements();
Expand Down Expand Up @@ -259,7 +258,6 @@ WebInspector.TabbedPane.prototype = {
this._tabs.splice(this._tabs.indexOf(tab), 1);
if (tab._shown)
this._hideTabElement(tab);
tab.view.detach();

var eventData = { tabId: id, view: tab.view, isUserGesture: userGesture };
this.dispatchEventToListeners(WebInspector.TabbedPane.EventTypes.TabClosed, eventData);
Expand Down Expand Up @@ -425,17 +423,13 @@ WebInspector.TabbedPane.prototype = {
changeTabView: function(id, view)
{
var tab = this._tabsById[id];
if (tab.view === view)
return;

var isSelected = this._currentTab && this._currentTab.id === id;
if (isSelected)
this._hideTab(tab);
tab.view.detach();
tab.view = view;
tab.view.attach(this.element);
if (isSelected)
if (this._currentTab && this._currentTab.id === tab.id) {
if (tab.view !== view)
this._hideTab(tab);
tab.view = view;
this._showTab(tab);
} else
tab.view = view;
},

onResize: function()
Expand Down Expand Up @@ -765,7 +759,7 @@ WebInspector.TabbedPane.prototype = {
_showTab: function(tab)
{
tab.tabElement.classList.add("selected");
tab.view.showWidget();
tab.view.show(this.element);
this._updateTabSlider();
},

Expand All @@ -791,7 +785,7 @@ WebInspector.TabbedPane.prototype = {
_hideTab: function(tab)
{
tab.tabElement.classList.remove("selected");
tab.view.hideWidget();
tab.view.detach();
},

/**
Expand Down
98 changes: 29 additions & 69 deletions front_end/ui/Widget.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ WebInspector.Widget = function(isWebComponent)
}
this._isWebComponent = isWebComponent;
this.element.__widget = this;
this._visible = false;
this._visible = true;
this._isRoot = false;
this._isShowing = false;
this._children = [];
Expand Down Expand Up @@ -119,7 +119,7 @@ WebInspector.Widget.prototype = {
{
if (this._isRoot)
return true;
return !!this._parentWidget && this._parentWidget.isShowing();
return this._parentWidget && this._parentWidget.isShowing();
},

/**
Expand Down Expand Up @@ -209,49 +209,28 @@ WebInspector.Widget.prototype = {
* @param {?Element=} insertBefore
*/
show: function(parentElement, insertBefore)
{
this.attach(parentElement, insertBefore);
this.showWidget();
},

/**
* @param {?Element} parentElement
* @param {?Element=} insertBefore
*/
attach: function(parentElement, insertBefore)
{
WebInspector.Widget.__assert(parentElement, "Attempt to attach widget with no parent element");

// Update widget hierarchy.
var currentParent = parentElement;
while (currentParent && !currentParent.__widget)
currentParent = currentParent.parentElementOrShadowHost();
var newParentWidget = currentParent ? currentParent.__widget : null;

if (this._parentWidget && newParentWidget !== this._parentWidget) {
// Reparent.
this.detach();
}
if (this.element.parentElement !== parentElement) {
if (this.element.parentElement)
this.detach();

if (newParentWidget) {
if (this._parentWidget !== newParentWidget) {
this._parentWidget = newParentWidget;
var currentParent = parentElement;
while (currentParent && !currentParent.__widget)
currentParent = currentParent.parentElementOrShadowHost();

if (currentParent) {
this._parentWidget = currentParent.__widget;
this._parentWidget._children.push(this);
}
this._isRoot = false;
} else {
WebInspector.Widget.__assert(this._isRoot, "Attempt to attach widget to orphan node");
this._isRoot = false;
} else
WebInspector.Widget.__assert(this._isRoot, "Attempt to attach widget to orphan node");
} else if (this._visible) {
return;
}

this._parentElement = parentElement;
this._insertBeforeElement = insertBefore;
},

showWidget: function()
{
WebInspector.Widget.__assert(this._parentElement, "Attempt to show detached widget");
if (this._visible)
return;
this._visible = true;

if (this._parentIsShowing())
Expand All @@ -260,12 +239,12 @@ WebInspector.Widget.prototype = {
this.element.classList.remove("hidden");

// Reparent
if (this.element.parentElement !== this._parentElement) {
WebInspector.Widget._incrementWidgetCounter(this._parentElement, this.element);
if (this._insertBeforeElement)
WebInspector.Widget._originalInsertBefore.call(this._parentElement, this.element, this._insertBeforeElement);
if (this.element.parentElement !== parentElement) {
WebInspector.Widget._incrementWidgetCounter(parentElement, this.element);
if (insertBefore)
WebInspector.Widget._originalInsertBefore.call(parentElement, this.element, insertBefore);
else
WebInspector.Widget._originalAppendChild.call(this._parentElement, this.element);
WebInspector.Widget._originalAppendChild.call(parentElement, this.element);
}

if (this._parentIsShowing())
Expand All @@ -277,51 +256,35 @@ WebInspector.Widget.prototype = {
this._processOnResize();
},

hideWidget: function()
{
this._hideWidget();
},

/**
* @param {boolean=} overrideHideOnDetach
* @return {boolean}
*/
_hideWidget: function(overrideHideOnDetach)
detach: function(overrideHideOnDetach)
{
WebInspector.Widget.__assert(this._parentElement, "Attempt to hide detached widget");
if (!this._visible)
return false;
this._visible = false;
var parentElement = this._parentElement;
var parentElement = this.element.parentElement;
if (!parentElement)
return;

if (this._parentIsShowing())
this._processWillHide();

if (!overrideHideOnDetach && this.shouldHideOnDetach()) {
this.element.classList.add("hidden");
this._visible = false;
if (this._parentIsShowing())
this._processWasHidden();
if (this._parentWidget && this._hasNonZeroConstraints())
this._parentWidget.invalidateConstraints();
return true;
return;
}

// Force legal removal
WebInspector.Widget._decrementWidgetCounter(parentElement, this.element);
WebInspector.Widget._originalRemoveChild.call(parentElement, this.element);

this._visible = false;
if (this._parentIsShowing())
this._processWasHidden();
return true;
},

detach: function()
{
if (!this._parentWidget)
return;
var wasShown = this._hideWidget(true);
if (!wasShown)
return;

// Update widget hierarchy.
if (this._parentWidget) {
Expand All @@ -331,13 +294,10 @@ WebInspector.Widget.prototype = {
this._parentWidget.childWasDetached(this);
var parent = this._parentWidget;
this._parentWidget = null;
this._parentElement = null;
this._insertBeforeElement = null;
if (this._hasNonZeroConstraints())
parent.invalidateConstraints();
} else {
} else
WebInspector.Widget.__assert(this._isRoot, "Removing non-root widget from DOM");
}
},

detachChildWidgets: function()
Expand Down

0 comments on commit 5db55e4

Please sign in to comment.