Commit 8232a090 authored by Sam Sebree's avatar Sam Sebree Committed by Commit Bot

[SyntheticModules] Address follow-up comments from "Implements...

[SyntheticModules] Address follow-up comments from "Implements ValueWrapperSyntheticModuleScript::Create" CL

The following CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1694604
Contains a few comments left by nhiroki@chromium.org which didn't make it into the CL until after it was merged. This is a follow-up CL designed to address these comments.

The comments are:

third_party/blink/renderer/core/script/value_wrapper_synthetic_module_script.cc:
Line 22:
"Non-member variables shouldn't have trailing `_`, so this should be `options`."

Line 55:
"According to the spec, the base URL and fetch options are null, while in this CL they are set to non-null things. Is this intentional?"

Bug: https://bugs.chromium.org/p/chromium/issues/detail?id=967018
Change-Id: Ie579d0f0d0429e32304b5f01e2e9454ee4cad044
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1727519Reviewed-by: default avatarHiroshige Hayashizaki <hiroshige@chromium.org>
Commit-Queue: Sam Sebree <sasebree@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#683349}
parent 20d90979
...@@ -18,8 +18,7 @@ namespace blink { ...@@ -18,8 +18,7 @@ namespace blink {
ValueWrapperSyntheticModuleScript* ValueWrapperSyntheticModuleScript*
ValueWrapperSyntheticModuleScript::CreateJSONWrapperSyntheticModuleScript( ValueWrapperSyntheticModuleScript::CreateJSONWrapperSyntheticModuleScript(
const base::Optional<ModuleScriptCreationParams>& params, const base::Optional<ModuleScriptCreationParams>& params,
Modulator* settings_object, Modulator* settings_object) {
const ScriptFetchOptions options_) {
DCHECK(settings_object->HasValidContext()); DCHECK(settings_object->HasValidContext());
ScriptState::Scope scope(settings_object->GetScriptState()); ScriptState::Scope scope(settings_object->GetScriptState());
v8::Local<v8::Context> context = v8::Local<v8::Context> context =
...@@ -51,12 +50,12 @@ ValueWrapperSyntheticModuleScript::CreateJSONWrapperSyntheticModuleScript( ...@@ -51,12 +50,12 @@ ValueWrapperSyntheticModuleScript::CreateJSONWrapperSyntheticModuleScript(
v8::Local<v8::Value> error = exception_state.GetException(); v8::Local<v8::Value> error = exception_state.GetException();
exception_state.ClearException(); exception_state.ClearException();
return ValueWrapperSyntheticModuleScript::CreateWithError( return ValueWrapperSyntheticModuleScript::CreateWithError(
parsed_json, settings_object, params->GetResponseUrl(), parsed_json, settings_object, params->GetResponseUrl(), KURL(),
params->GetResponseUrl(), options_, error); ScriptFetchOptions(), error);
} else { } else {
return ValueWrapperSyntheticModuleScript::CreateWithDefaultExport( return ValueWrapperSyntheticModuleScript::CreateWithDefaultExport(
parsed_json, settings_object, params->GetResponseUrl(), parsed_json, settings_object, params->GetResponseUrl(), KURL(),
params->GetResponseUrl(), options_); ScriptFetchOptions());
} }
} }
......
...@@ -28,8 +28,7 @@ class CORE_EXPORT ValueWrapperSyntheticModuleScript final ...@@ -28,8 +28,7 @@ class CORE_EXPORT ValueWrapperSyntheticModuleScript final
static ValueWrapperSyntheticModuleScript* static ValueWrapperSyntheticModuleScript*
CreateJSONWrapperSyntheticModuleScript( CreateJSONWrapperSyntheticModuleScript(
const base::Optional<ModuleScriptCreationParams>& params, const base::Optional<ModuleScriptCreationParams>& params,
Modulator* settings_object, Modulator* settings_object);
const ScriptFetchOptions options_);
static ValueWrapperSyntheticModuleScript* CreateWithDefaultExport( static ValueWrapperSyntheticModuleScript* CreateWithDefaultExport(
v8::Local<v8::Value> value, v8::Local<v8::Value> value,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment