Bug 220480 – [Cocoa] Web Inspector: move browser domain activation methods back to WKWebView and UIDelegate

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

[Blaze Burg]

After some recent cleanups in WKInspectorDelegate and related code, it's no longer necessary to keep these backend methods in an inspector-specific delegate.

[Blaze Burg]

<rdar://problem/71977924>

[Blaze Burg]

Created attachment 417301 [details]
Patch v1.0

[Blaze Burg]

Created attachment 417306 [details]
Patch v1.1

[Devin Rousso]

Comment on attachment 417306 [details]
Patch v1.1

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

r=me

> Source/WebKit/ChangeLog:70
> +        Drive-by: rename m_page to m_inspectedPage to emphasize that this
> +        class exists as part of Web Inspector's backend, not its frontend.

+1 nice!

> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:1660
> +

Style: I'd remove this newline

> Source/WebKit/UIProcess/Cocoa/UIDelegate.mm:1568
> +    [(id <WKUIDelegatePrivate>)delegate _webViewDidEnableInspectorBrowserDomain:m_uiDelegate->m_webView.get().get()];

NIT: I'd do the cast on :1564 so that if `delegate` is needed for other things it doesn't have to be casted multiple times

> Source/WebKit/UIProcess/Cocoa/UIDelegate.mm:1583
> +    [(id <WKUIDelegatePrivate>)delegate _webViewDidDisableInspectorBrowserDomain:m_uiDelegate->m_webView.get().get()];

ditto (:1568)

> Source/WebKit/UIProcess/Inspector/WebPageInspectorController.cpp:252
> +    if (agent)

NIT: I prefer to use `m_enabledBrowserAgent` for stuff like this in case there's some funky logic inside `operator=` that prevents the `m_enabledBrowserAgent` from actually changing

> Source/WebKit/UIProcess/Inspector/WebPageInspectorController.cpp:260
> +    if (auto* browserAgent = enabledBrowserAgent())

NIT: you could just use `m_enabledBrowserAgent`

> Source/WebKit/UIProcess/Inspector/WebPageInspectorController.cpp:266
> +    if (auto* browserAgent = enabledBrowserAgent())

ditto (:260)

[Blaze Burg]

Created attachment 417470 [details]
Patch v1.2

[Blaze Burg]

Comment on attachment 417470 [details]
Patch v1.2

Needs to be landed manually with internal changes.

[EWS]

Committed r271424: <https://trac.webkit.org/changeset/271424>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 417470 [details].