Skip to content

Commit

Permalink
Move JavaScriptExecuteRequest into blink
Browse files Browse the repository at this point in the history
This CL moves JavaScriptExecuteRequest from
content::mojom::Frame to blink::mojom::LocalFrame.

Bug: 1192244
Change-Id: I10e890d719405865b7974316ded172b253e0e22b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2831152
Commit-Queue: Julie Kim <jkim@igalia.com>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#873729}
  • Loading branch information
jkim-julie authored and Chromium LUCI CQ committed Apr 19, 2021
1 parent c04d324 commit 36194bc
Show file tree
Hide file tree
Showing 9 changed files with 60 additions and 48 deletions.
2 changes: 1 addition & 1 deletion content/browser/renderer_host/render_frame_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1929,7 +1929,7 @@ void RenderFrameHostImpl::ExecuteJavaScript(const std::u16string& javascript,
CHECK(CanExecuteJavaScript());

const bool wants_result = !callback.is_null();
GetMojomFrameInRenderer()->JavaScriptExecuteRequest(javascript, wants_result,
GetAssociatedLocalFrame()->JavaScriptExecuteRequest(javascript, wants_result,
std::move(callback));
}

Expand Down
19 changes: 0 additions & 19 deletions content/common/frame.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -341,25 +341,6 @@ interface Frame {
UpdateSubresourceLoaderFactories(
blink.mojom.URLLoaderFactoryBundle subresource_loader_factories);

// Request for the renderer to execute JavaScript in the frame's context.
//
// |javascript| is the string containing the JavaScript to be executed in the
// target frame's context. Note that this uses BigString16 rather than
// String16 as this is used in contexts, like DevTools, where the contents of
// the JavaScript script is user-provided, and therefore we can't guarantee
// the size of the script.
//
// |wants_result| is true if the result of this execution is required by the
// caller. If it is false, a reply is still required by Mojo, but a null value
// should be returned to avoid issues serializing a large, unwanted reply.
//
// TODO(hajimehoshi): This requires navigate association to keep the message
// order with other navigation-related messages. Fix this and move this to a
// non-navigate-related interface if possible.
JavaScriptExecuteRequest(
mojo_base.mojom.BigString16 javascript,
bool wants_result) => (mojo_base.mojom.Value result);

// ONLY FOR TESTS: Same as above but this can optionally trigger a fake user
// activation notification to test functionalities that are gated by user
// activation.
Expand Down
5 changes: 5 additions & 0 deletions content/public/test/fake_local_frame.cc
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,11 @@ void FakeLocalFrame::JavaScriptMethodExecuteRequest(
bool wants_result,
JavaScriptMethodExecuteRequestCallback callback) {}

void FakeLocalFrame::JavaScriptExecuteRequest(
const std::u16string& javascript,
bool wants_result,
JavaScriptExecuteRequestCallback callback) {}

void FakeLocalFrame::GetSavableResourceLinks(
GetSavableResourceLinksCallback callback) {}

Expand Down
4 changes: 4 additions & 0 deletions content/public/test/fake_local_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ class FakeLocalFrame : public blink::mojom::LocalFrame {
base::Value arguments,
bool wants_result,
JavaScriptMethodExecuteRequestCallback callback) override;
void JavaScriptExecuteRequest(
const std::u16string& javascript,
bool wants_result,
JavaScriptExecuteRequestCallback callback) override;
void GetSavableResourceLinks(
GetSavableResourceLinksCallback callback) override;
#if defined(OS_MAC)
Expand Down
26 changes: 2 additions & 24 deletions content/renderer/render_frame_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2211,29 +2211,6 @@ void RenderFrameImpl::Delete(mojom::FrameDeleteIntention intent) {
frame_->Detach();
}

void RenderFrameImpl::JavaScriptExecuteRequest(
const std::u16string& javascript,
bool wants_result,
JavaScriptExecuteRequestCallback callback) {
TRACE_EVENT_INSTANT0("test_tracing", "JavaScriptExecuteRequest",
TRACE_EVENT_SCOPE_THREAD);

// Note that ExecuteScriptAndReturnValue may end up killing this object.
base::WeakPtr<RenderFrameImpl> weak_this = weak_factory_.GetWeakPtr();

v8::HandleScope handle_scope(v8::Isolate::GetCurrent());
v8::Local<v8::Value> result = frame_->ExecuteScriptAndReturnValue(
WebScriptSource(WebString::FromUTF16(javascript)));

if (!weak_this)
return;

if (wants_result)
std::move(callback).Run(GetJavaScriptExecutionResult(result));
else
std::move(callback).Run({});
}

void RenderFrameImpl::JavaScriptExecuteRequestForTests(
const std::u16string& javascript,
bool wants_result,
Expand Down Expand Up @@ -2470,7 +2447,8 @@ blink::WebPlugin* RenderFrameImpl::CreatePlugin(
}

void RenderFrameImpl::ExecuteJavaScript(const std::u16string& javascript) {
JavaScriptExecuteRequest(javascript, false, base::DoNothing());
v8::HandleScope handle_scope(v8::Isolate::GetCurrent());
frame_->ExecuteScript(WebScriptSource(WebString::FromUTF16(javascript)));
}

void RenderFrameImpl::BindLocalInterface(
Expand Down
4 changes: 0 additions & 4 deletions content/renderer/render_frame_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -808,10 +808,6 @@ class CONTENT_EXPORT RenderFrameImpl
void UpdateSubresourceLoaderFactories(
std::unique_ptr<blink::PendingURLLoaderFactoryBundle>
subresource_loader_factories) override;
void JavaScriptExecuteRequest(
const std::u16string& javascript,
bool wants_result,
JavaScriptExecuteRequestCallback callback) override;
void JavaScriptExecuteRequestForTests(
const std::u16string& javascript,
bool wants_result,
Expand Down
16 changes: 16 additions & 0 deletions third_party/blink/public/mojom/frame/frame.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -713,6 +713,22 @@ interface LocalFrame {
mojo_base.mojom.ListValue arguments,
bool wants_result) => (mojo_base.mojom.Value result);

// Request for the renderer to execute JavaScript in the frame's context.
//
// |javascript| is the string containing the JavaScript to be executed in the
// target frame's context. Note that this uses BigString16 rather than
// String16 as this is used in contexts, like DevTools, where the contents of
// the JavaScript script is user-provided, and therefore we can't guarantee
// the size of the script.
//
// |wants_result| is true if the result of this execution is required by the
// caller. If it is false, a reply is still required by Mojo, but a null value
// should be returned to avoid issues serializing a large, unwanted reply.
//
JavaScriptExecuteRequest(
mojo_base.mojom.BigString16 javascript,
bool wants_result) => (mojo_base.mojom.Value result);

// Requests the index of a character in the frame's text stream at the given
// point. The point is in the viewport coordinate space. Replies using
// TextInputHost.
Expand Down
28 changes: 28 additions & 0 deletions third_party/blink/renderer/core/frame/local_frame.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@

#include "base/metrics/histogram_functions.h"
#include "base/unguessable_token.h"
#include "base/values.h"
#include "mojo/public/cpp/bindings/self_owned_receiver.h"
#include "mojo/public/cpp/system/message_pipe.h"
#include "services/data_decoder/public/mojom/resource_snapshot_for_web_bundle.mojom-blink.h"
Expand Down Expand Up @@ -73,6 +74,7 @@
#include "third_party/blink/public/web/web_plugin.h"
#include "third_party/blink/renderer/bindings/core/v8/script_controller.h"
#include "third_party/blink/renderer/bindings/core/v8/source_location.h"
#include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_core.h"
#include "third_party/blink/renderer/core/content_capture/content_capture_manager.h"
#include "third_party/blink/renderer/core/core_initializer.h"
#include "third_party/blink/renderer/core/core_probe_sink.h"
Expand Down Expand Up @@ -169,6 +171,7 @@
#include "third_party/blink/renderer/core/paint/paint_layer_scrollable_area.h"
#include "third_party/blink/renderer/core/paint/paint_timing.h"
#include "third_party/blink/renderer/core/probe/core_probes.h"
#include "third_party/blink/renderer/core/script/classic_script.h"
#include "third_party/blink/renderer/core/scroll/smooth_scroll_sequencer.h"
#include "third_party/blink/renderer/core/svg/svg_document_extensions.h"
#include "third_party/blink/renderer/platform/back_forward_cache_utils.h"
Expand Down Expand Up @@ -3614,6 +3617,31 @@ void LocalFrame::JavaScriptMethodExecuteRequest(
}
}

void LocalFrame::JavaScriptExecuteRequest(
const String& javascript,
bool wants_result,
JavaScriptExecuteRequestCallback callback) {
TRACE_EVENT_INSTANT0("test_tracing", "JavaScriptExecuteRequest",
TRACE_EVENT_SCOPE_THREAD);

v8::HandleScope handle_scope(v8::Isolate::GetCurrent());
v8::Local<v8::Value> result =
ClassicScript::CreateUnspecifiedScript(javascript)
->RunScriptAndReturnValue(DomWindow());

if (wants_result) {
std::unique_ptr<WebV8ValueConverter> converter =
Platform::Current()->CreateWebV8ValueConverter();
converter->SetDateAllowed(true);
converter->SetRegExpAllowed(true);

std::move(callback).Run(
GetJavaScriptExecutionResult(result, this, converter.get()));
} else {
std::move(callback).Run({});
}
}

void LocalFrame::BindReportingObserver(
mojo::PendingReceiver<mojom::blink::ReportingObserver> receiver) {
ReportingContext::From(DomWindow())->Bind(std::move(receiver));
Expand Down
4 changes: 4 additions & 0 deletions third_party/blink/renderer/core/frame/local_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,10 @@ class CORE_EXPORT LocalFrame final
base::Value arguments,
bool wants_result,
JavaScriptMethodExecuteRequestCallback callback) final;
void JavaScriptExecuteRequest(
const String& javascript,
bool wants_result,
JavaScriptExecuteRequestCallback callback) final;
void BindReportingObserver(
mojo::PendingReceiver<mojom::blink::ReportingObserver> receiver) final;
void UpdateOpener(
Expand Down

0 comments on commit 36194bc

Please sign in to comment.