Bug 192487 – Web Inspector: Move TreeOutlineGroup coordination out of TreeElement

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Matt Baker]

TreeOutlineGroup should respond to SelectionDidChange events from TreeOutlines it tracks, rather than the current intrusive approach where TreeElement calls back into the group.

[Radar WebKit Bug Importer]

<rdar://problem/46543431>

[Matt Baker]

Created attachment 356775 [details]
Patch

[EWS Watchlist]

Comment on attachment 356775 [details]
Patch

Attachment 356775 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10299193

New failing tests:
fast/visual-viewport/tiled-drawing/zoomed-fixed-scrolling-layers-state.html

[EWS Watchlist]

Created attachment 356777 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[Devin Rousso]

Comment on attachment 356775 [details]
Patch

r+

I did notice some unrealted selection issues (not sure if it relates to something that's already been filed).

* STEPS TO REPRODUCE:
1. open WebInspector
2. show the Resources tab to any JS file (starts the FormatterWorker)
3. open WebInspector2
4. set a breakpoint in WebInspector2 on `WI.showRepresentedObject
5. do something in WebInspector to trigger that pause (e.g. switch tabs or select something in a navigation sidebar)
6. switch call frames in WebInspector2 to the last frame of the Page, and then switch again to the FormatterWorker's Idle frame
7. step over
 => the first frame of Page and FormatterWorker's Idle frame are both selected

[Devin Rousso]

(In reply to Devin Rousso from comment #5)
> I did notice some unrealted selection issues (not sure if it relates to something that's already been filed).
> 
> * STEPS TO REPRODUCE:
> 1. open WebInspector
> 2. show the Resources tab to any JS file (starts the FormatterWorker)
> 3. open WebInspector2
> 4. set a breakpoint in WebInspector2 on `WI.showRepresentedObject
> 5. do something in WebInspector to trigger that pause (e.g. switch tabs or select something in a navigation sidebar)
> 6. switch call frames in WebInspector2 to the last frame of the Page, and then switch again to the FormatterWorker's Idle frame
> 7. step over
>  => the first frame of Page and FormatterWorker's Idle frame are both selected
An assertion is also thrown on TreeElement.js:538:23

[Matt Baker]

Created attachment 356988 [details]
Patch for landing

[Matt Baker]

(In reply to Devin Rousso from comment #6)
> (In reply to Devin Rousso from comment #5)
> > I did notice some unrealted selection issues (not sure if it relates to something that's already been filed).
> > 
> > * STEPS TO REPRODUCE:
> > 1. open WebInspector
> > 2. show the Resources tab to any JS file (starts the FormatterWorker)
> > 3. open WebInspector2
> > 4. set a breakpoint in WebInspector2 on `WI.showRepresentedObject
> > 5. do something in WebInspector to trigger that pause (e.g. switch tabs or select something in a navigation sidebar)
> > 6. switch call frames in WebInspector2 to the last frame of the Page, and then switch again to the FormatterWorker's Idle frame
> > 7. step over
> >  => the first frame of Page and FormatterWorker's Idle frame are both selected
> An assertion is also thrown on TreeElement.js:538:23

I removed the assertion, which was added in https://webkit.org/b/192353. We already check that TreeElement.prototype.deselect is called by the TreeOutline on behalf of the SelectionController after an item is deselected.

[WebKit Commit Bot]

Comment on attachment 356988 [details]
Patch for landing

Clearing flags on attachment: 356988

Committed r239051: <https://trac.webkit.org/changeset/239051>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.