blink::ScriptForbiddenScope does not offer real guarantees. |
|||
Issue descriptionTL;DR: I'm a scope, not a cop. ScriptForbiddenScope [0] seems to offer a way to mark sections where JavaScript must not execute. But it relies on certain places to check for ScriptForbiddenScope::IsScriptForbidden(). In V8, we actually offer v8::DisallowJavaScriptExecutionScope [1] to make sure no JavaScript gets executed. How about we have [1] as member in [0]? [0] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/bindings/ScriptForbiddenScope.h [1] https://cs.chromium.org/chromium/src/v8/include/v8.h?l=6850&rcl=73c55f57fe8506011ff854b15026ca765b669700
,
Jan 19 2018
Sounds good to me. ScriptForbiddenScope::IsScriptForbidden() is placed in BeforeCallEnteredCallback (https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/bindings/V8PerIsolateData.cpp?type=cs&q=isscriptforbidden+callback&sq=package:chromium&l=52), which runs every time a V8 API that may invoke JavaScript is called. So I think the check is properly working. However, if we can export v8::DisallowJavaScriptExecutionScope to Blink, it should be simpler and better.
,
Jan 19 2018
Not sure what you mean by exporting. It's already part of V8's API.
,
Jan 19 2018
I just mean that Blink should just use v8::DisallowJavaScriptExecutionScope as you mentioned.
,
Jan 22 2018
there's only one place where we check the scope, and it uses a CHECK(), that pretty much makes it a cop, no? We can use the assert scope instead, but not sure what the benefit is? I'd rather get rid of this first: https://cs.chromium.org/chromium/src/.gn?rcl=9b6b4fa5fbc072fa8ba56ebbab5f7b42d21b781b&l=56 |
|||
►
Sign in to add a comment |
|||
Comment 1 by yangguo@chromium.org
, Jan 19 2018