Bug 217109 - Web Inspector: exempt API::SharedJSContext from remote inspection and automatic inspection
Summary: Web Inspector: exempt API::SharedJSContext from remote inspection and automat...
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: 2020-09-29 16:29 PDT by BJ Burg
Modified: 2020-09-30 09:48 PDT (History)
4 users (show)

See Also:


Attachments
Proposed Fix (1.96 KB, patch)
2020-09-29 16:33 PDT, BJ Burg
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description BJ Burg 2020-09-29 16:29:32 PDT
This is only used to deserialize objects, there is no point to allowing inspection.
Comment 1 BJ Burg 2020-09-29 16:29:47 PDT
<rdar://problem/69386559>
Comment 2 BJ Burg 2020-09-29 16:33:41 PDT
Created attachment 410066 [details]
Proposed Fix
Comment 3 Mark Lam 2020-09-29 16:38:16 PDT
Comment on attachment 410066 [details]
Proposed Fix

r=me
Comment 4 BJ Burg 2020-09-29 17:11:40 PDT
There is some concern that setting the property will happen too late and this will already be paused underneath -[JSContext initWithVirtualMachine:].

Alternate ideas:
- temporarily disable automatic inspection when initializing that JSContext.
- add SPI to JSContext to specifically disable automatic inspection.
- in the pauseWaitingForAutomaticInspection wait loop, check if remote inspection has been disabled and bail if so.
- make webinspectord detect when an automatic inspection candidate has disabled remote inspection and delete any associated automatic inspection sessions.
Comment 5 BJ Burg 2020-09-30 09:17:31 PDT
Comment on attachment 410066 [details]
Proposed Fix

Let's proceed, if this is not enough we'll find out soon.
Comment 6 EWS 2020-09-30 09:35:05 PDT
Committed r267793: <https://trac.webkit.org/changeset/267793>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 410066 [details].
Comment 7 BJ Burg 2020-09-30 09:48:50 PDT
Oof, looks like we should be doing this instead:

previous = JSRemoteInspectorGetInspectionEnabledByDefault()
JSRemoteInspectorSetInspectionEnabledByDefault(false)
// initialize the JSContext, disable remote inspection explicitly.
JSRemoteInspectorSetInspectionEnabledByDefault(previous)