Bug 224841 - v2: Web Inspector: exempt API::SharedJSContext from remote inspection and automatic inspection
Summary: v2: Web Inspector: exempt API::SharedJSContext from remote inspection and aut...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: BJ Burg
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-04-20 16:18 PDT by BJ Burg
Modified: 2021-04-22 11:01 PDT (History)
4 users (show)

See Also:


Attachments
Patch v1.0 (2.28 KB, patch)
2021-04-20 16:23 PDT, BJ Burg
hi: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description BJ Burg 2021-04-20 16:18:43 PDT
The fix for bug 217109 was insufficient, I have a better fix.
Comment 1 BJ Burg 2021-04-20 16:20:18 PDT
<rdar://69386559>
Comment 2 BJ Burg 2021-04-20 16:23:54 PDT
Created attachment 426611 [details]
Patch v1.0
Comment 3 Devin Rousso 2021-04-20 16:34:02 PDT
Comment on attachment 426611 [details]
Patch v1.0

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

r=me

Out of curiosity, does this prevent inspection forever?  Or does it just prevent auto-attach/auto-pause?  Would be nice to still be able to inspect this later on if possible :)

> Source/WebKit/UIProcess/API/Cocoa/APISerializedScriptValueCocoa.mm:52
>              [m_context _setRemoteInspectionEnabled:NO];

I think we can remove this since it's only set `if (JSRemoteInspectorGetInspectionEnabledByDefault())`.
Comment 4 BJ Burg 2021-04-22 08:33:03 PDT
(In reply to Devin Rousso from comment #3)
> Comment on attachment 426611 [details]
> Patch v1.0
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=426611&action=review
> 
> r=me
> 
> Out of curiosity, does this prevent inspection forever?  Or does it just
> prevent auto-attach/auto-pause?  Would be nice to still be able to inspect
> this later on if possible :)

The default is to not allow inspection, so yes, this would continue to not be listed. This could be fixed (i.e, for engineering purposes) by making JSRemoteInspectorGetInspectionEnabledByDefault always return true, or changing the default value of m_enabled in JSC::RemoteInspectionTarget. But in general, I don't think there is much point in exposing this context outside of engineering builds.

> 
> > Source/WebKit/UIProcess/API/Cocoa/APISerializedScriptValueCocoa.mm:52
> >              [m_context _setRemoteInspectionEnabled:NO];
> 
> I think we can remove this since it's only set `if
> (JSRemoteInspectorGetInspectionEnabledByDefault())`.

True, it would default to false as explained above.
Comment 5 BJ Burg 2021-04-22 11:00:59 PDT
Committed r276446 (236906@main): <https://commits.webkit.org/236906@main>