Bug 230573 – [Cocoa] backport showConsole() and showResources() for RemoteWebInspectorUI

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit API

[Blaze Burg]

.

[Radar WebKit Bug Importer]

<rdar://problem/83365814>

[Blaze Burg]

In the process of writing an API test for _WKInspectorExtensionDelegate, I realized that there is no way to reliably select extension tabs and select non-extension tabs. For the purposes of testing (and future API unification), we should hoist showResources() and showConsole() to the _WKInspectorExtensionHost protocol.

[Blaze Burg]

Created attachment 438850 [details]
Patch v1.0

[Patrick Angle]

Comment on attachment 438850 [details]
Patch v1.0

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

> Source/WebKit/ChangeLog:19
> +        * UIProcess/API/Cocoa/_WKInspectorExtensionHost.h:
> +        Hoist showResources() and showConsole() to this shared protocol.
> +        This doesn't really have anything to do with extensions, but it will
> +        suffice until a more generic protocol is created for APIs that should
> +        exist for both _WKInspector and _WKInspectorRemoteViewController.

It is a shame that we don't have a separate generic protocol for this stuff yet, particularly given that `close` is already declared here, which also isn't directly extension related either. I'm torn on perpetuating this misdirection further. Is there any reason we either go ahead and create that more generic protocol or rename this protocol to better reflect what it is?

[Blaze Burg]

Created attachment 438884 [details]
Patch v1.1

[Blaze Burg]

(In reply to Patrick Angle from comment #4)
> Comment on attachment 438850 [details]
> Patch v1.0
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=438850&action=review
> 
> > Source/WebKit/ChangeLog:19
> > +        * UIProcess/API/Cocoa/_WKInspectorExtensionHost.h:
> > +        Hoist showResources() and showConsole() to this shared protocol.
> > +        This doesn't really have anything to do with extensions, but it will
> > +        suffice until a more generic protocol is created for APIs that should
> > +        exist for both _WKInspector and _WKInspectorRemoteViewController.
> 
> It is a shame that we don't have a separate generic protocol for this stuff
> yet, particularly given that `close` is already declared here, which also
> isn't directly extension related either. I'm torn on perpetuating this
> misdirection further. Is there any reason we either go ahead and create that
> more generic protocol or rename this protocol to better reflect what it is?

The new patch v1.1 incorporates this suggestion.

[Patrick Angle]

Comment on attachment 438884 [details]
Patch v1.1

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

Provisional r+ from me

> Source/WebKit/WebKit.xcodeproj/project.pbxproj:6
> +	objectVersion = 54;

Probably don’t want to increment this? I imagine we will want to do that in a separate bug and across the entire project even if this is intended. I know this has caused issues on older Xcode versions in the past.

[Blaze Burg]

Created attachment 438951 [details]
Patch v1.2

Latest patch v1.2 addresses all comments to date.

[Devin Rousso]

Comment on attachment 438951 [details]
Patch v1.2

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

> Source/WebKit/UIProcess/API/Cocoa/_WKInspectorIBActions.h:46
> + * This will open the associated Web Inspector if it is not already open.

Is this actually true?

> Source/WebKit/UIProcess/API/Cocoa/_WKInspectorIBActions.h:52
> + * This will open the associated Web Inspector if it is not already open.

ditto (:46)

[Blaze Burg]

(In reply to Devin Rousso from comment #9)
> Comment on attachment 438951 [details]
> Patch v1.2
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=438951&action=review
> 
> > Source/WebKit/UIProcess/API/Cocoa/_WKInspectorIBActions.h:46
> > + * This will open the associated Web Inspector if it is not already open.
> 
> Is this actually true?
> 
> > Source/WebKit/UIProcess/API/Cocoa/_WKInspectorIBActions.h:52
> > + * This will open the associated Web Inspector if it is not already open.
> 
> ditto (:46)

No. I have removed the comment in the newest patch version.

Most of the IBActions have no effect unless the inspector frontend is loaded. Ultimately they will wait for frontend to finish loading in InspectorFrontendAPIDispatcher.

It is recommended for API users to call -show before doing more things with _WKInspector.

[Devin Rousso]

Comment on attachment 438951 [details]
Patch v1.2

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

r=me based on described changes to API doc

>>> Source/WebKit/UIProcess/API/Cocoa/_WKInspectorIBActions.h:46
>>> + * This will open the associated Web Inspector if it is not already open.
>> 
>> Is this actually true?
> 
> No. I have removed the comment in the newest patch version.
> 
> Most of the IBActions have no effect unless the inspector frontend is loaded. Ultimately they will wait for frontend to finish loading in InspectorFrontendAPIDispatcher.
> 
> It is recommended for API users to call -show before doing more things with _WKInspector.

Gotcha.  That sounds fine :)

[Blaze Burg]

Committed r283119 (242177@main): <https://commits.webkit.org/242177@main>