[webkit-reviews] review granted: [Bug 191052] Web Inspector: Separate target initialization from frontend initialization : [Attachment 353412] [PATCH] Proposed Fix

[bugzilla-daemon at webkit.org]

Brian Burg <bburg at apple.com> has granted Joseph Pecoraro <joepeck at webkit.org>'s
request for review:
Bug 191052: Web Inspector: Separate target initialization from frontend
initialization
https://bugs.webkit.org/show_bug.cgi?id=191052

Attachment 353412: [PATCH] Proposed Fix

https://bugs.webkit.org/attachment.cgi?id=353412&action=review




--- Comment #14 from Brian Burg <bburg at apple.com> ---
Comment on attachment 353412
  --> https://bugs.webkit.org/attachment.cgi?id=353412
[PATCH] Proposed Fix

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

r=me

> Source/WebInspectorUI/UserInterface/Base/Main.js:150
> +    this.printStylesEnabled = false;

It's weird that this is not a WI.Setting like other settings above whose value
would need to be re-sent to each new target.

> Source/WebInspectorUI/UserInterface/Controllers/ConsoleManager.js:165
> +	   target.ConsoleAgent.getLoggingChannels((error, channels) => {

I would have used a promise, but I don't care either way.

> Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js:46
> +	  
WI.settings.pauseForInternalScripts.addEventListener(WI.Setting.Event.Changed,
this._pauseForInternalScriptsDidChange);

Why not pass `this`?