No subject

<span class="quote">&gt; Source/WebInspectorUI/UserInterface/Views/ContentViewContainer.js:420
&gt; +    _placeTombstonesForContentView(contentView)</span >

s/place/add/

<span class="quote">&gt; Source/WebInspectorUI/UserInterface/Views/ContentViewContainer.js:431
&gt; +        for (let entry of this._backForwardList) {</span >

This sort of assumes that backForwardList won't be mutated by _hideEntry(). This may be true now, but taking a copy for iterating may be safer.

<span class="quote">&gt; Source/WebInspectorUI/UserInterface/Views/ContentViewContainer.js:447
&gt; +    _reviveTombstonesForContentView(contentView)</span >

s/revive/clear/

<span class="quote">&gt; Source/WebInspectorUI/UserInterface/Views/ContentViewContainer.js:456
&gt; +        for (let entry of this._backForwardList) {</span >

Ditto.

<span class="quote">&gt; Source/WebInspectorUI/UserInterface/Views/ContentViewContainer.js:465
&gt; +    _disassociateFromContentView(contentView, tombstone)</span >

What is the intended type of the 'tombstone' argument? It's used as a boolean,  but it's not named like one.

<span class="quote">&gt; Source/WebInspectorUI/UserInterface/Views/ContentViewContainer.js:472
&gt; +            const onlyFirst = true;</span >

Thanks for using a named argument! But, could there be multiple tombstones for the same content view (ie., when switching between two content views several times, then another container transfers it away)?

<span class="quote">&gt; Source/WebInspectorUI/UserInterface/Views/ContentViewContainer.js:484
&gt; +        let otherInterestedContentViews = contentView.__tombstoneContentViewContainers;</span >

This variable's name doesn't match its type. It's a content view container. Also the 'interested' terminology is weird, how about:

let associatedContainers = ...

<span class="quote">&gt;&gt; Source/WebInspectorUI/UserInterface/Views/ContentViewContainer.js:486
&gt;&gt; +            if (otherInterestedContentViews[0] !== this)
&gt; 
&gt; So, if I am following this right, I think the reason why you're using [0] here is because you're only interested in passing the contentView to the most recent interested view if one exists and is not the current one.  If so, could you add a brief statement explaining something to that effect (about the [0]).  If not, definitely add a comment.</span >

There seem to be two cases here when dissociating the content view:

- the container is the parent, or
- the container has a tombstone

In the second case, this code would transfer the content view to some other container. But I would have expected |this| to never be an interested container at this point since we removed it from the tombstone list in the branch above (modulo my comment). So why is this if check necessary?</pre>
        </div>
      </p>
      <hr>
      <span>You are receiving this mail because:</span>
      
      <ul>
          <li>You are the assignee for the bug.</li>
      </ul>
    </body>
</html>
--1455301448.d2708.30515--