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: Blaze Burg
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-09-29 16:29 PDT by Blaze 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, Blaze Burg 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Blaze Burg 2020-09-29 16:29:32 PDT 
This is only used to deserialize objects, there is no point to allowing inspection.
Comment 1 Blaze Burg 2020-09-29 16:29:47 PDT 
<rdar://problem/69386559>
Comment 2 Blaze 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 Blaze 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 Blaze 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 Blaze 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)