<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[196508] trunk/Source/WebInspectorUI</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/196508">196508</a></dd>
<dt>Author</dt> <dd>commit-queue@webkit.org</dd>
<dt>Date</dt> <dd>2016-02-12 14:15:14 -0800 (Fri, 12 Feb 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>Web Inspector: Tabs: Conflicts with multiple Formatters per SourceCode
https://bugs.webkit.org/show_bug.cgi?id=144717
<rdar://problem/20845163>
Patch by Joseph Pecoraro <pecoraro@apple.com> on 2016-02-12
Reviewed by Timothy Hatcher.
The underlying issue here is that each tab may create its own ContentView,
and therefore SourceCodeTextEditor, per-SourceCode. Each SourceCodeTextEditor
was mutating the SourceCode's state without listening for or expecting
updates from the other. This causes a bunch of different issues:
- editing in one tab does not get reflected in another tab for
the same resource. This is common when using the Search tab
to find and make an edit, then debug in another tab.
- one tab may auto format (pretty print) a resource and set
the formatter on the SourceCode to make SourceCodeLocations
know about formatted locations. However, a jump to location
that opens a new ContentView for the same Resource will
start out un-formatted, and misunderstand the location.
This often results in an unexpected jump to 0:0.
The solution taken by this change is to have a single ContentView
per represented object. When that ContentView gets shown in a new
ContentViewContainer it gets transferred, leaving a tombstone in the
previous ContentViewContainer that can be revived later. This keeps
back foward lists with expected values. It also means there is a
single ContentView that doesn't need to worry about having the
state of its represented object getting overrun.
Currently this makes the assumption that we won't ever show multiple
ContentViews for the same represented object at the same time. This
may need to change if we were to support split pane editor or
something like that.
This also makes the assumption that ContentViewContainer's showEntry
and hideEntry do not modify the back forward list. That has not been
the case, and I think it is safe to assume it will never be the case.
The contracts this patch maintains:
- a ContentView is always owned by one ViewContainer.
This ViewContainer is the one showing the ContentView.
- when another ViewContainer wants to share the ContentView
ownership is transferred. Creating tombstones in the old
ViewContainer and Reviving tombstones in the new ViewContainer.
- ViewContainer's have a tombstone per-BackForwardEntry that
references the ContentView.
- In order to ensure a ContentView always gets closed, when
the owning ViewContainer would close the ContentView it
checks if it should instead transfer ownership of the
ContentView to another interested ViewContainer.
This also maintains the contract that a ContentView should only be
closed once. When the ContentView is transferred between two
ContentViewContainers it should hide/show from the old to the new.
The last ContentViewContainer to reference a ContentView should
be the one to close the ContentView.
* UserInterface/Models/BackForwardEntry.js:
(WebInspector.BackForwardEntry):
(WebInspector.BackForwardEntry.prototype.get tombstone):
(WebInspector.BackForwardEntry.prototype.set tombstone):
(WebInspector.BackForwardEntry.prototype.prepareToShow):
(WebInspector.BackForwardEntry.prototype.prepareToHide):
Tombstone state and assertions that we don't show/hide tombstones,
that should all be done before a back forward entry has become a tombstone.
* UserInterface/Views/ContentView.js:
(WebInspector.ContentView.contentViewForRepresentedObject):
(WebInspector.ContentView.closedContentViewForRepresentedObject):
(WebInspector.ContentView.resolvedRepresentedObjectForRepresentedObject):
Helpers for getting / creating / clearing the single ContentView that
is associated with a represented object.
* UserInterface/Views/ContentViewContainer.js:
(WebInspector.ContentViewContainer.prototype.contentViewForRepresentedObject):
(WebInspector.ContentViewContainer.prototype.showContentView):
Eliminate code that dealt with multiple content views per represented object.
That is replaced by multiple ContentViewContainers per ContentView.
(WebInspector.ContentViewContainer.prototype.replaceContentView):
This is called in special places where we don't need to worry about a tombstone.
It is an in replace of a content view.
(WebInspector.ContentViewContainer.closeAllContentViewsOfPrototype):
(WebInspector.ContentViewContainer.prototype.closeContentView):
(WebInspector.ContentViewContainer.prototype.closeAllContentViews):
(WebInspector.ContentViewContainer.prototype._disassociateFromContentView):
Deal with closing BackForwardEntrys that are tombstones.
(WebInspector.ContentViewContainer.prototype._takeOwnershipOfContentView):
(WebInspector.ContentViewContainer.prototype._placeTombstonesForContentView):
(WebInspector.ContentViewContainer.prototype._clearTombstonesForContentView):
Helpers for transfering ownership of a ContentView to a ContentViewContainer.
There is always one owner of the ContentView. Non-owners have tombstone
BackForward entries.
(WebInspector.ContentViewContainer.prototype._showEntry):
If we are showing a tombstone, gain ownership.
(WebInspector.ContentViewContainer.prototype._hideEntry):
This may happen in closing, for simplicity we bail here instead of include
messy logic at the call site. We would have already hidden this entry
when making it a tombstone.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebInspectorUIChangeLog">trunk/Source/WebInspectorUI/ChangeLog</a></li>
<li><a href="#trunkSourceWebInspectorUIUserInterfaceModelsBackForwardEntryjs">trunk/Source/WebInspectorUI/UserInterface/Models/BackForwardEntry.js</a></li>
<li><a href="#trunkSourceWebInspectorUIUserInterfaceViewsContentViewjs">trunk/Source/WebInspectorUI/UserInterface/Views/ContentView.js</a></li>
<li><a href="#trunkSourceWebInspectorUIUserInterfaceViewsContentViewContainerjs">trunk/Source/WebInspectorUI/UserInterface/Views/ContentViewContainer.js</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebInspectorUIChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/ChangeLog (196507 => 196508)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/ChangeLog 2016-02-12 22:13:56 UTC (rev 196507)
+++ trunk/Source/WebInspectorUI/ChangeLog 2016-02-12 22:15:14 UTC (rev 196508)
</span><span class="lines">@@ -1,3 +1,114 @@
</span><ins>+2016-02-12 Joseph Pecoraro <pecoraro@apple.com>
+
+ Web Inspector: Tabs: Conflicts with multiple Formatters per SourceCode
+ https://bugs.webkit.org/show_bug.cgi?id=144717
+ <rdar://problem/20845163>
+
+ Reviewed by Timothy Hatcher.
+
+ The underlying issue here is that each tab may create its own ContentView,
+ and therefore SourceCodeTextEditor, per-SourceCode. Each SourceCodeTextEditor
+ was mutating the SourceCode's state without listening for or expecting
+ updates from the other. This causes a bunch of different issues:
+
+ - editing in one tab does not get reflected in another tab for
+ the same resource. This is common when using the Search tab
+ to find and make an edit, then debug in another tab.
+
+ - one tab may auto format (pretty print) a resource and set
+ the formatter on the SourceCode to make SourceCodeLocations
+ know about formatted locations. However, a jump to location
+ that opens a new ContentView for the same Resource will
+ start out un-formatted, and misunderstand the location.
+ This often results in an unexpected jump to 0:0.
+
+ The solution taken by this change is to have a single ContentView
+ per represented object. When that ContentView gets shown in a new
+ ContentViewContainer it gets transferred, leaving a tombstone in the
+ previous ContentViewContainer that can be revived later. This keeps
+ back foward lists with expected values. It also means there is a
+ single ContentView that doesn't need to worry about having the
+ state of its represented object getting overrun.
+
+ Currently this makes the assumption that we won't ever show multiple
+ ContentViews for the same represented object at the same time. This
+ may need to change if we were to support split pane editor or
+ something like that.
+
+ This also makes the assumption that ContentViewContainer's showEntry
+ and hideEntry do not modify the back forward list. That has not been
+ the case, and I think it is safe to assume it will never be the case.
+
+ The contracts this patch maintains:
+
+ - a ContentView is always owned by one ViewContainer.
+ This ViewContainer is the one showing the ContentView.
+
+ - when another ViewContainer wants to share the ContentView
+ ownership is transferred. Creating tombstones in the old
+ ViewContainer and Reviving tombstones in the new ViewContainer.
+
+ - ViewContainer's have a tombstone per-BackForwardEntry that
+ references the ContentView.
+
+ - In order to ensure a ContentView always gets closed, when
+ the owning ViewContainer would close the ContentView it
+ checks if it should instead transfer ownership of the
+ ContentView to another interested ViewContainer.
+
+ This also maintains the contract that a ContentView should only be
+ closed once. When the ContentView is transferred between two
+ ContentViewContainers it should hide/show from the old to the new.
+ The last ContentViewContainer to reference a ContentView should
+ be the one to close the ContentView.
+
+ * UserInterface/Models/BackForwardEntry.js:
+ (WebInspector.BackForwardEntry):
+ (WebInspector.BackForwardEntry.prototype.get tombstone):
+ (WebInspector.BackForwardEntry.prototype.set tombstone):
+ (WebInspector.BackForwardEntry.prototype.prepareToShow):
+ (WebInspector.BackForwardEntry.prototype.prepareToHide):
+ Tombstone state and assertions that we don't show/hide tombstones,
+ that should all be done before a back forward entry has become a tombstone.
+
+ * UserInterface/Views/ContentView.js:
+ (WebInspector.ContentView.contentViewForRepresentedObject):
+ (WebInspector.ContentView.closedContentViewForRepresentedObject):
+ (WebInspector.ContentView.resolvedRepresentedObjectForRepresentedObject):
+ Helpers for getting / creating / clearing the single ContentView that
+ is associated with a represented object.
+
+ * UserInterface/Views/ContentViewContainer.js:
+ (WebInspector.ContentViewContainer.prototype.contentViewForRepresentedObject):
+ (WebInspector.ContentViewContainer.prototype.showContentView):
+ Eliminate code that dealt with multiple content views per represented object.
+ That is replaced by multiple ContentViewContainers per ContentView.
+
+ (WebInspector.ContentViewContainer.prototype.replaceContentView):
+ This is called in special places where we don't need to worry about a tombstone.
+ It is an in replace of a content view.
+
+ (WebInspector.ContentViewContainer.closeAllContentViewsOfPrototype):
+ (WebInspector.ContentViewContainer.prototype.closeContentView):
+ (WebInspector.ContentViewContainer.prototype.closeAllContentViews):
+ (WebInspector.ContentViewContainer.prototype._disassociateFromContentView):
+ Deal with closing BackForwardEntrys that are tombstones.
+
+ (WebInspector.ContentViewContainer.prototype._takeOwnershipOfContentView):
+ (WebInspector.ContentViewContainer.prototype._placeTombstonesForContentView):
+ (WebInspector.ContentViewContainer.prototype._clearTombstonesForContentView):
+ Helpers for transfering ownership of a ContentView to a ContentViewContainer.
+ There is always one owner of the ContentView. Non-owners have tombstone
+ BackForward entries.
+
+ (WebInspector.ContentViewContainer.prototype._showEntry):
+ If we are showing a tombstone, gain ownership.
+
+ (WebInspector.ContentViewContainer.prototype._hideEntry):
+ This may happen in closing, for simplicity we bail here instead of include
+ messy logic at the call site. We would have already hidden this entry
+ when making it a tombstone.
+
</ins><span class="cx"> 2016-02-11 Joseph Pecoraro <pecoraro@apple.com>
</span><span class="cx">
</span><span class="cx"> Web Inspector: SourceCodeTextEditor close() generates removeEventListener warnings
</span></span></pre></div>
<a id="trunkSourceWebInspectorUIUserInterfaceModelsBackForwardEntryjs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/UserInterface/Models/BackForwardEntry.js (196507 => 196508)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/UserInterface/Models/BackForwardEntry.js 2016-02-12 22:13:56 UTC (rev 196507)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/BackForwardEntry.js 2016-02-12 22:15:14 UTC (rev 196508)
</span><span class="lines">@@ -32,6 +32,11 @@
</span><span class="cx">
</span><span class="cx"> this._contentView = contentView;
</span><span class="cx">
</span><ins>+ // ContentViews may be shared across multiple ContentViewContainers.
+ // A BackForwardEntry containing a tombstone doesn't actually have
+ // the real ContentView, and so should not close it.
+ this._tombstone = false;
+
</ins><span class="cx"> // Cookies are compared with Object.shallowEqual, so should not store objects or arrays.
</span><span class="cx"> this._cookie = cookie || {};
</span><span class="cx"> this._scrollPositions = [];
</span><span class="lines">@@ -52,8 +57,20 @@
</span><span class="cx"> return Object.shallowCopy(this._cookie);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ get tombstone()
+ {
+ return this._tombstone;
+ }
+
+ set tombstone(tombstone)
+ {
+ this._tombstone = tombstone;
+ }
+
</ins><span class="cx"> prepareToShow(shouldCallShown)
</span><span class="cx"> {
</span><ins>+ console.assert(!this._tombstone, "Should not be calling shown on a tombstone");
+
</ins><span class="cx"> this._restoreFromCookie();
</span><span class="cx">
</span><span class="cx"> this.contentView.visible = true;
</span><span class="lines">@@ -64,6 +81,8 @@
</span><span class="cx">
</span><span class="cx"> prepareToHide()
</span><span class="cx"> {
</span><ins>+ console.assert(!this._tombstone, "Should not be calling hidden on a tombstone");
+
</ins><span class="cx"> this.contentView.visible = false;
</span><span class="cx"> this.contentView.hidden();
</span><span class="cx">
</span><span class="lines">@@ -111,7 +130,7 @@
</span><span class="cx"> if (!element)
</span><span class="cx"> continue;
</span><span class="cx">
</span><del>- var position = { scrollTop: element.scrollTop, scrollLeft: element.scrollLeft };
</del><ins>+ let position = {scrollTop: element.scrollTop, scrollLeft: element.scrollLeft};
</ins><span class="cx"> if (this.contentView.shouldKeepElementsScrolledToBottom)
</span><span class="cx"> position.isScrolledToBottom = element.isScrolledToBottom();
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebInspectorUIUserInterfaceViewsContentViewjs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/UserInterface/Views/ContentView.js (196507 => 196508)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/UserInterface/Views/ContentView.js 2016-02-12 22:13:56 UTC (rev 196507)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/ContentView.js 2016-02-12 22:15:14 UTC (rev 196508)
</span><span class="lines">@@ -39,7 +39,7 @@
</span><span class="cx"> this._parentContainer = null;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- // Public
</del><ins>+ // Static
</ins><span class="cx">
</span><span class="cx"> static createFromRepresentedObject(representedObject, extraArguments)
</span><span class="cx"> {
</span><span class="lines">@@ -140,6 +140,54 @@
</span><span class="cx"> throw new Error("Can't make a ContentView for an unknown representedObject.");
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ static contentViewForRepresentedObject(representedObject, onlyExisting, extraArguments)
+ {
+ console.assert(representedObject);
+
+ let resolvedRepresentedObject = WebInspector.ContentView.resolvedRepresentedObjectForRepresentedObject(representedObject);
+ let existingContentView = resolvedRepresentedObject[WebInspector.ContentView.ContentViewForRepresentedObjectSymbol];
+ console.assert(!existingContentView || existingContentView instanceof WebInspector.ContentView);
+ if (existingContentView)
+ return existingContentView;
+
+ if (onlyExisting)
+ return null;
+
+ let newContentView = WebInspector.ContentView.createFromRepresentedObject(representedObject, extraArguments);
+ console.assert(newContentView instanceof WebInspector.ContentView);
+ if (!newContentView)
+ return null;
+
+ console.assert(newContentView.representedObject === resolvedRepresentedObject, "createFromRepresentedObject and resolvedRepresentedObjectForRepresentedObject are out of sync for type", representedObject.constructor.name);
+ newContentView.representedObject[WebInspector.ContentView.ContentViewForRepresentedObjectSymbol] = newContentView;
+ return newContentView;
+ }
+
+ static closedContentViewForRepresentedObject(representedObject)
+ {
+ let resolvedRepresentedObject = WebInspector.ContentView.resolvedRepresentedObjectForRepresentedObject(representedObject);
+ resolvedRepresentedObject[WebInspector.ContentView.ContentViewForRepresentedObjectSymbol] = null;
+ }
+
+ static resolvedRepresentedObjectForRepresentedObject(representedObject)
+ {
+ if (representedObject instanceof WebInspector.Frame)
+ return representedObject.mainResource;
+
+ if (representedObject instanceof WebInspector.Breakpoint) {
+ if (representedObject.sourceCodeLocation)
+ return representedObject.sourceCodeLocation.displaySourceCode;
+ }
+
+ if (representedObject instanceof WebInspector.DOMSearchMatchObject)
+ return WebInspector.frameResourceManager.mainFrame.domTree;
+
+ if (representedObject instanceof WebInspector.SourceCodeSearchMatchObject)
+ return representedObject.sourceCode;
+
+ return representedObject;
+ }
+
</ins><span class="cx"> static isViewable(representedObject)
</span><span class="cx"> {
</span><span class="cx"> if (representedObject instanceof WebInspector.Frame)
</span><span class="lines">@@ -345,3 +393,5 @@
</span><span class="cx"> NumberOfSearchResultsDidChange: "content-view-number-of-search-results-did-change",
</span><span class="cx"> NavigationItemsDidChange: "content-view-navigation-items-did-change"
</span><span class="cx"> };
</span><ins>+
+WebInspector.ContentView.ContentViewForRepresentedObjectSymbol = Symbol("content-view-for-represented-object");
</ins></span></pre></div>
<a id="trunkSourceWebInspectorUIUserInterfaceViewsContentViewContainerjs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/UserInterface/Views/ContentViewContainer.js (196507 => 196508)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/UserInterface/Views/ContentViewContainer.js 2016-02-12 22:13:56 UTC (rev 196507)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/ContentViewContainer.js 2016-02-12 22:15:14 UTC (rev 196508)
</span><span class="lines">@@ -63,47 +63,7 @@
</span><span class="cx">
</span><span class="cx"> contentViewForRepresentedObject(representedObject, onlyExisting, extraArguments)
</span><span class="cx"> {
</span><del>- console.assert(representedObject);
- if (!representedObject)
- return null;
-
- // Iterate over all the known content views for the representedObject (if any) and find one that doesn't
- // have a parent container or has this container as its parent.
- var contentView = null;
- for (var i = 0; representedObject.__contentViews && i < representedObject.__contentViews.length; ++i) {
- var currentContentView = representedObject.__contentViews[i];
- if (!currentContentView._parentContainer || currentContentView._parentContainer === this) {
- contentView = currentContentView;
- break;
- }
- }
-
- console.assert(!contentView || contentView instanceof WebInspector.ContentView);
- if (contentView instanceof WebInspector.ContentView)
- return contentView;
-
- // Return early to avoid creating a new content view when onlyExisting is true.
- if (onlyExisting)
- return null;
-
- // No existing content view found, make a new one.
- contentView = WebInspector.ContentView.createFromRepresentedObject(representedObject, extraArguments);
-
- console.assert(contentView, "Unknown representedObject", representedObject);
- if (!contentView)
- return null;
-
- // The representedObject can change in the constructor for ContentView. Remember the
- // contentViews on the real representedObject and not the one originally supplied.
- // The main case for this is a Frame being passed in and the main Resource being used.
- representedObject = contentView.representedObject;
-
- // Remember this content view for future calls.
- if (!representedObject.__contentViews)
- representedObject.__contentViews = [];
- representedObject.__contentViews.push(contentView);
-
- return contentView;
</del><ins>+ return WebInspector.ContentView.contentViewForRepresentedObject(representedObject, onlyExisting, extraArguments);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> showContentViewForRepresentedObject(representedObject, extraArguments)
</span><span class="lines">@@ -123,11 +83,10 @@
</span><span class="cx"> if (!(contentView instanceof WebInspector.ContentView))
</span><span class="cx"> return null;
</span><span class="cx">
</span><del>- // Don't allow showing a content view that is already associated with another container.
- // Showing a content view that is already associated with this container is allowed.
- console.assert(!contentView.parentContainer || contentView.parentContainer === this);
- if (contentView.parentContainer && contentView.parentContainer !== this)
- return null;
</del><ins>+ // ContentViews can be shared between containers. If this content view is
+ // not owned by us, it may need to be transferred to this container.
+ if (contentView.parentContainer !== this)
+ this._takeOwnershipOfContentView(contentView);
</ins><span class="cx">
</span><span class="cx"> var currentEntry = this.currentBackForwardEntry;
</span><span class="cx"> var provisionalEntry = new WebInspector.BackForwardEntry(contentView, cookie);
</span><span class="lines">@@ -158,7 +117,7 @@
</span><span class="cx"> });
</span><span class="cx">
</span><span class="cx"> if (shouldDissociateContentView)
</span><del>- this._disassociateFromContentView(removedEntries[i].contentView);
</del><ins>+ this._disassociateFromContentView(removedEntries[i].contentView, removedEntries[i].tombstone);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> // Associate with the new content view.
</span><span class="lines">@@ -218,7 +177,7 @@
</span><span class="cx"> this._hideEntry(this.currentBackForwardEntry);
</span><span class="cx">
</span><span class="cx"> // Disassociate with the old content view.
</span><del>- this._disassociateFromContentView(oldContentView);
</del><ins>+ this._disassociateFromContentView(oldContentView, false);
</ins><span class="cx">
</span><span class="cx"> // Associate with the new content view.
</span><span class="cx"> newContentView._parentContainer = this;
</span><span class="lines">@@ -226,6 +185,7 @@
</span><span class="cx"> // Replace all occurrences of oldContentView with newContentView in the back/forward list.
</span><span class="cx"> for (var i = 0; i < this._backForwardList.length; ++i) {
</span><span class="cx"> if (this._backForwardList[i].contentView === oldContentView) {
</span><ins>+ console.assert(!this._backForwardList[i].tombstone);
</ins><span class="cx"> let currentCookie = this._backForwardList[i].cookie;
</span><span class="cx"> this._backForwardList[i] = new WebInspector.BackForwardEntry(newContentView, currentCookie);
</span><span class="cx"> }
</span><span class="lines">@@ -278,7 +238,7 @@
</span><span class="cx"> --this._currentIndex;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- this._disassociateFromContentView(entry.contentView);
</del><ins>+ this._disassociateFromContentView(entry.contentView, entry.tombstone);
</ins><span class="cx">
</span><span class="cx"> // Remove the item from the back/forward list.
</span><span class="cx"> this._backForwardList.splice(i, 1);
</span><span class="lines">@@ -334,7 +294,7 @@
</span><span class="cx"> --this._currentIndex;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- this._disassociateFromContentView(entry.contentView);
</del><ins>+ this._disassociateFromContentView(entry.contentView, entry.tombstone);
</ins><span class="cx">
</span><span class="cx"> // Remove the item from the back/forward list.
</span><span class="cx"> this._backForwardList.splice(i, 1);
</span><span class="lines">@@ -364,7 +324,7 @@
</span><span class="cx"> var entry = this._backForwardList[i];
</span><span class="cx"> if (entry.contentView === visibleContentView)
</span><span class="cx"> this._hideEntry(entry);
</span><del>- this._disassociateFromContentView(entry.contentView);
</del><ins>+ this._disassociateFromContentView(entry.contentView, entry.tombstone);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> this._backForwardList = [];
</span><span class="lines">@@ -417,8 +377,74 @@
</span><span class="cx">
</span><span class="cx"> // Private
</span><span class="cx">
</span><del>- _disassociateFromContentView(contentView)
</del><ins>+ _takeOwnershipOfContentView(contentView)
</ins><span class="cx"> {
</span><ins>+ console.assert(contentView.parentContainer !== this, "We already have ownership of the ContentView");
+ if (contentView.parentContainer === this)
+ return;
+
+ if (contentView.parentContainer)
+ contentView.parentContainer._placeTombstonesForContentView(contentView);
+
+ contentView._parentContainer = this;
+
+ this._clearTombstonesForContentView(contentView);
+ }
+
+ _placeTombstonesForContentView(contentView)
+ {
+ console.assert(contentView.parentContainer === this);
+
+ // Ensure another ContentViewContainer doesn't close this ContentView while we still have it.
+ let tombstoneContentViewContainers = this._tombstoneContentViewContainersForContentView(contentView);
+ console.assert(!tombstoneContentViewContainers.includes(this));
+
+ let visibleContentView = this.currentContentView;
+
+ for (let entry of this._backForwardList) {
+ if (entry.contentView !== contentView)
+ continue;
+
+ if (entry.contentView === visibleContentView) {
+ this._hideEntry(entry);
+ visibleContentView = null;
+ }
+
+ console.assert(!entry.tombstone);
+ entry.tombstone = true;
+
+ tombstoneContentViewContainers.push(this);
+ }
+ }
+
+ _clearTombstonesForContentView(contentView)
+ {
+ console.assert(contentView.parentContainer === this);
+
+ let tombstoneContentViewContainers = this._tombstoneContentViewContainersForContentView(contentView);
+ const onlyFirst = false;
+ tombstoneContentViewContainers.remove(this, onlyFirst);
+
+ for (let entry of this._backForwardList) {
+ if (entry.contentView !== contentView)
+ continue;
+
+ console.assert(entry.tombstone);
+ entry.tombstone = false;
+ }
+ }
+
+ _disassociateFromContentView(contentView, isTombstone)
+ {
+ // Just remove one of our tombstone back references.
+ // There may be other back/forward entries that need a reference.
+ if (isTombstone) {
+ let tombstoneContentViewContainers = this._tombstoneContentViewContainersForContentView(contentView);
+ const onlyFirst = true;
+ tombstoneContentViewContainers.remove(this, onlyFirst);
+ return;
+ }
+
</ins><span class="cx"> console.assert(!contentView.visible);
</span><span class="cx">
</span><span class="cx"> if (!contentView._parentContainer)
</span><span class="lines">@@ -426,17 +452,33 @@
</span><span class="cx">
</span><span class="cx"> contentView._parentContainer = null;
</span><span class="cx">
</span><del>- var representedObject = contentView.representedObject;
- if (representedObject && representedObject.__contentViews)
- representedObject.__contentViews.remove(contentView);
</del><ins>+ // If another ContentViewContainer has tombstones for this, just transfer
+ // ownership to that ContentViewContainer and avoid closing the ContentView.
+ // We don't care who we transfer this to, so just use the first.
+ let tombstoneContentViewContainers = this._tombstoneContentViewContainersForContentView(contentView);
+ if (tombstoneContentViewContainers && tombstoneContentViewContainers.length) {
+ tombstoneContentViewContainers[0]._takeOwnershipOfContentView(contentView);
+ return;
+ }
</ins><span class="cx">
</span><span class="cx"> contentView.closed();
</span><ins>+
+ if (contentView.representedObject)
+ WebInspector.ContentView.closedContentViewForRepresentedObject(contentView.representedObject);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> _showEntry(entry, shouldCallShown)
</span><span class="cx"> {
</span><span class="cx"> console.assert(entry instanceof WebInspector.BackForwardEntry);
</span><span class="cx">
</span><ins>+ // We may be showing a tombstone from a BackForward list or when re-showing a container
+ // that had previously had the content view transferred away from it.
+ // Take over the ContentView.
+ if (entry.tombstone) {
+ this._takeOwnershipOfContentView(entry.contentView);
+ console.assert(!entry.tombstone);
+ }
+
</ins><span class="cx"> if (!this.subviews.includes(entry.contentView))
</span><span class="cx"> this.addSubview(entry.contentView)
</span><span class="cx">
</span><span class="lines">@@ -447,12 +489,27 @@
</span><span class="cx"> {
</span><span class="cx"> console.assert(entry instanceof WebInspector.BackForwardEntry);
</span><span class="cx">
</span><ins>+ // If this was a tombstone, the content view should already have been
+ // hidden when we placed the tombstone.
+ if (entry.tombstone)
+ return;
+
</ins><span class="cx"> entry.prepareToHide();
</span><span class="cx"> if (this.subviews.includes(entry.contentView))
</span><span class="cx"> this.removeSubview(entry.contentView)
</span><span class="cx"> }
</span><ins>+
+ _tombstoneContentViewContainersForContentView(contentView)
+ {
+ let tombstoneContentViewContainers = contentView[WebInspector.ContentViewContainer.TombstoneContentViewContainersSymbol];
+ if (!tombstoneContentViewContainers)
+ tombstoneContentViewContainers = contentView[WebInspector.ContentViewContainer.TombstoneContentViewContainersSymbol] = [];
+ return tombstoneContentViewContainers;
+ }
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> WebInspector.ContentViewContainer.Event = {
</span><span class="cx"> CurrentContentViewDidChange: "content-view-container-current-content-view-did-change"
</span><span class="cx"> };
</span><ins>+
+WebInspector.ContentViewContainer.TombstoneContentViewContainersSymbol = Symbol("content-view-container-tombstone-content-view-containers");
</ins></span></pre>
</div>
</div>
</body>
</html>