Bug 217109

Summary:

Web Inspector: exempt API::SharedJSContext from remote inspection and automatic inspection

Product:

WebKit

Reporter:

Blaze Burg <bburg>

Component:

Web Inspector

Assignee:

Blaze Burg <bburg>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bburg, inspector-bugzilla-changes, mark.lam, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Attachments:

Description	Flags
Proposed Fix	none

Blaze Burg

Reported 2020-09-29 16:29:32 PDT

This is only used to deserialize objects, there is no point to allowing inspection.

Attachments
Proposed Fix (1.96 KB, patch) 2020-09-29 16:33 PDT, Blaze Burg	no flags	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Blaze Burg

Comment 1 2020-09-29 16:29:47 PDT

<rdar://problem/69386559>

Blaze Burg

Comment 2 2020-09-29 16:33:41 PDT

Created attachment 410066 [details] Proposed Fix

Mark Lam

Comment 3 2020-09-29 16:38:16 PDT

Comment on attachment 410066 [details] Proposed Fix r=me

Blaze Burg

Comment 4 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.

Blaze Burg

Comment 5 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.

EWS

Comment 6 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].

Blaze Burg

Comment 7 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)

Note You need to log in before you can comment on or make changes to this bug.