Summary: | [Cocoa] Web Inspector: move browser domain activation methods back to WKWebView and UIDelegate | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | BJ Burg <bburg> | ||||||||
Component: | Web Inspector | Assignee: | BJ Burg <bburg> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | bburg, ews-watchlist, hi, inspector-bugzilla-changes, joepeck, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
BJ Burg
2021-01-08 13:27:22 PST
Created attachment 417301 [details]
Patch v1.0
Created attachment 417306 [details]
Patch v1.1
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) Created attachment 417470 [details]
Patch v1.2
Comment on attachment 417470 [details]
Patch v1.2
Needs to be landed manually with internal changes.
Committed r271424: <https://trac.webkit.org/changeset/271424> All reviewed patches have been landed. Closing bug and clearing flags on attachment 417470 [details]. |