Web Inspector: scope chain details sidebar doesn't update values modified via console

• Component: Web Inspector
• Status: RESOLVED FIXED

[BJ Burg]

Test case:

when paused, add a new property to some object visible in the scope chain details sidebar. The new property won't be added to the tree until you take a step in the debugger.

[Radar WebKit Bug Importer]

<rdar://problem/15801345>

[Chris J. Shull]

Created attachment 224125 [details]
Patch

[Timothy Hatcher]

Comment on attachment 224125 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=224125&action=review

Thanks for looking into this!

> Source/WebInspectorUI/UserInterface/JavaScriptLogViewController.js:228
> +            WebInspector.scopeChainDetailsSidebarPanel.needsRefresh();

This would be better as an event that the sidebar observes. Having this low-level view class directly call a higher-level class is a layering violation.

RuntimeManager would be a good place to fire the event that the sidebar observes.

[Timothy Hatcher]

Having RuntimeManager fire it after doing an evaluate would catch any other place we evaluate expressions.

[Chris J. Shull]

Created attachment 224166 [details]
Patch

[Chris J. Shull]

Thanks for the feedback, uploaded new patch.

Two things I wasn’t sure about:
1) I didn’t add any data to the event, since it seems like most events don’t include data unless something will use it. Happy to add if you want though.
2) I didn’t refactor RuntimeManager#evaluateInInspectedWindow() to not have a callback at all, since having a callback made more sense for the things which were calling that method (since they needed to have callbacks specific to the invocation, as opposed to generic like the event is used.) Plus, one of the other args changes what args get passed to the callback.

[Joseph Pecoraro]

Comment on attachment 224166 [details]
Patch

You need to bind.(this) on both evalCallback calls. There is one on RuntimeManager.js a few lines down (line 75 after your patches). Otherwise, just typing in the console normally (not paused) will throw an exception right?

[Chris J. Shull]

Created attachment 224233 [details]
Patch

[Chris J. Shull]

Yikes! Thank you for catching that.
Patch updated.

[WebKit Commit Bot]

Comment on attachment 224233 [details]
Patch

Clearing flags on attachment: 224233

Committed r164163: <http://trac.webkit.org/changeset/164163>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.