[webkit-reviews] review granted: [Bug 231338] [Cocoa] Web Inspector: provide a way for _WKInspectorExtension clients to be notified when the inspected page navigates : [Attachment 440620] Patch v1.1

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted BJ Burg <bburg at apple.com>'s
request for review:
Bug 231338: [Cocoa] Web Inspector: provide a way for _WKInspectorExtension
clients to be notified when the inspected page navigates
https://bugs.webkit.org/show_bug.cgi?id=231338

Attachment 440620: Patch v1.1

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




--- Comment #7 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 440620
  --> https://bugs.webkit.org/attachment.cgi?id=440620
Patch v1.1

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

r=me

>
Source/WebInspectorUI/UserInterface/Controllers/WebInspectorExtensionController
.js:37
> +	   this._mainFrameDidChangeListener =
WI.Frame.addEventListener(WI.Frame.Event.MainResourceDidChange,
this._handleMainResourceDidChange, this);

you don't need to create `this._mainFrameDidChangeListener` for reasons I
described previously

>
Source/WebInspectorUI/UserInterface/Controllers/WebInspectorExtensionController
.js:264
> +	  
InspectorFrontendHost.inspectedPageDidNavigate(WI.networkManager.mainFrame.url)
;

NIT: It feels slightly odd to have this be so generically named and yet only
used by `WI.WebInspectorExtensionController`.  How awful would it be to just
remove these changes here and instead _always_ invoke this inside
`WI._mainResourceDidChange` in
`Source/WebInspectorUI/UserInterface/Base/Main.js` and then just let the
extension code in the UIProcess handle whether or not to actually do things?