Do not send JavaScript results if they are not needed.
If large enough, they might OOM the IPC.
BUG=971688,972409
(cherry picked from commit 8e3ebe9c79594a3e0f00e5aac452f33a2ca1f2b6)
Change-Id: Icd25172270b1f111bd70b0d23c655d6d65c429d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1651058
Commit-Queue: Avi Drissman <avi@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#667732}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1653351
Cr-Commit-Position: refs/branch-heads/3809@{#224}
Cr-Branched-From: d82dec1a818f378c464ba307ddd9c92133eac355-refs/heads/master@{#665002}
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index ae45923..b07b5f0b 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -1262,7 +1262,8 @@
DCHECK_CURRENTLY_ON(BrowserThread::UI);
CHECK(CanExecuteJavaScript());
- GetNavigationControl()->JavaScriptExecuteRequest(javascript,
+ const bool wants_result = !callback.is_null();
+ GetNavigationControl()->JavaScriptExecuteRequest(javascript, wants_result,
std::move(callback));
}
@@ -1274,8 +1275,9 @@
DCHECK_GT(world_id, ISOLATED_WORLD_ID_GLOBAL);
DCHECK_LE(world_id, ISOLATED_WORLD_ID_MAX);
+ const bool wants_result = !callback.is_null();
GetNavigationControl()->JavaScriptExecuteRequestInIsolatedWorld(
- javascript, world_id, std::move(callback));
+ javascript, wants_result, world_id, std::move(callback));
}
void RenderFrameHostImpl::ExecuteJavaScriptForTests(
@@ -1284,8 +1286,9 @@
DCHECK_CURRENTLY_ON(BrowserThread::UI);
const bool has_user_gesture = false;
+ const bool wants_result = !callback.is_null();
GetNavigationControl()->JavaScriptExecuteRequestForTests(
- javascript, has_user_gesture, std::move(callback));
+ javascript, wants_result, has_user_gesture, std::move(callback));
}
void RenderFrameHostImpl::ExecuteJavaScriptWithUserGestureForTests(
@@ -1294,7 +1297,7 @@
const bool has_user_gesture = true;
GetNavigationControl()->JavaScriptExecuteRequestForTests(
- javascript, has_user_gesture, base::NullCallback());
+ javascript, false, has_user_gesture, base::NullCallback());
}
void RenderFrameHostImpl::CopyImageAt(int x, int y) {
diff --git a/content/common/frame.mojom b/content/common/frame.mojom
index 82882159..577fd09 100644
--- a/content/common/frame.mojom
+++ b/content/common/frame.mojom
@@ -183,16 +183,22 @@
// |javascript| is the string containing the JavaScript to be executed in the
// target frame's context.
//
+ // |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.String16 javascript) => (mojo_base.mojom.Value result);
+ mojo_base.mojom.String16 javascript,
+ bool wants_result) => (mojo_base.mojom.Value result);
// ONLY FOR TESTS: Same as above but adds a fake UserGestureIndicator around
// execution. (crbug.com/408426)
JavaScriptExecuteRequestForTests(
mojo_base.mojom.String16 javascript,
+ bool wants_result,
bool has_user_gesture)
=> (mojo_base.mojom.Value result);
@@ -200,6 +206,7 @@
// isolated world specified by the fourth parameter.
JavaScriptExecuteRequestInIsolatedWorld(
mojo_base.mojom.String16 javascript,
+ bool wants_result,
int32 world_id) => (mojo_base.mojom.Value result);
// Posts a message from a frame in another process to the current renderer.
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 4d4c8b7..9a03e3b 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -2574,6 +2574,7 @@
void RenderFrameImpl::JavaScriptExecuteRequest(
const base::string16& javascript,
+ bool wants_result,
JavaScriptExecuteRequestCallback callback) {
TRACE_EVENT_INSTANT0("test_tracing", "JavaScriptExecuteRequest",
TRACE_EVENT_SCOPE_THREAD);
@@ -2588,11 +2589,15 @@
if (!weak_this)
return;
- std::move(callback).Run(GetJavaScriptExecutionResult(result));
+ if (wants_result)
+ std::move(callback).Run(GetJavaScriptExecutionResult(result));
+ else
+ std::move(callback).Run({});
}
void RenderFrameImpl::JavaScriptExecuteRequestForTests(
const base::string16& javascript,
+ bool wants_result,
bool has_user_gesture,
JavaScriptExecuteRequestForTestsCallback callback) {
TRACE_EVENT_INSTANT0("test_tracing", "JavaScriptExecuteRequestForTests",
@@ -2614,11 +2619,15 @@
if (!weak_this)
return;
- std::move(callback).Run(GetJavaScriptExecutionResult(result));
+ if (wants_result)
+ std::move(callback).Run(GetJavaScriptExecutionResult(result));
+ else
+ std::move(callback).Run({});
}
void RenderFrameImpl::JavaScriptExecuteRequestInIsolatedWorld(
const base::string16& javascript,
+ bool wants_result,
int32_t world_id,
JavaScriptExecuteRequestInIsolatedWorldCallback callback) {
TRACE_EVENT_INSTANT0("test_tracing",
@@ -2637,15 +2646,18 @@
v8::HandleScope handle_scope(v8::Isolate::GetCurrent());
WebScriptSource script = WebScriptSource(WebString::FromUTF16(javascript));
JavaScriptIsolatedWorldRequest* request = new JavaScriptIsolatedWorldRequest(
- weak_factory_.GetWeakPtr(), std::move(callback));
+ weak_factory_.GetWeakPtr(), wants_result, std::move(callback));
frame_->RequestExecuteScriptInIsolatedWorld(
world_id, &script, 1, false, WebLocalFrame::kSynchronous, request);
}
RenderFrameImpl::JavaScriptIsolatedWorldRequest::JavaScriptIsolatedWorldRequest(
base::WeakPtr<RenderFrameImpl> render_frame_impl,
+ bool wants_result,
JavaScriptExecuteRequestInIsolatedWorldCallback callback)
- : render_frame_impl_(render_frame_impl), callback_(std::move(callback)) {}
+ : render_frame_impl_(render_frame_impl),
+ wants_result_(wants_result),
+ callback_(std::move(callback)) {}
RenderFrameImpl::JavaScriptIsolatedWorldRequest::
~JavaScriptIsolatedWorldRequest() {
@@ -2660,7 +2672,7 @@
}
base::Value value;
- if (!result.empty()) {
+ if (!result.empty() && wants_result_) {
// It's safe to always use the main world context when converting
// here. V8ValueConverterImpl shouldn't actually care about the
// context scope, and it switches to v8::Object's creation context
@@ -3110,7 +3122,7 @@
}
void RenderFrameImpl::ExecuteJavaScript(const base::string16& javascript) {
- JavaScriptExecuteRequest(javascript, base::DoNothing());
+ JavaScriptExecuteRequest(javascript, false, base::DoNothing());
}
void RenderFrameImpl::BindLocalInterface(
diff --git a/content/renderer/render_frame_impl.h b/content/renderer/render_frame_impl.h
index 1d22f81..0119e96 100644
--- a/content/renderer/render_frame_impl.h
+++ b/content/renderer/render_frame_impl.h
@@ -632,13 +632,16 @@
void JavaScriptExecuteRequest(
const base::string16& javascript,
+ bool wants_result,
JavaScriptExecuteRequestCallback callback) override;
void JavaScriptExecuteRequestForTests(
const base::string16& javascript,
+ bool wants_result,
bool has_user_gesture,
JavaScriptExecuteRequestForTestsCallback callback) override;
void JavaScriptExecuteRequestInIsolatedWorld(
const base::string16& javascript,
+ bool wants_result,
int32_t world_id,
JavaScriptExecuteRequestInIsolatedWorldCallback callback) override;
void OnPortalActivated(
@@ -1028,6 +1031,7 @@
public:
JavaScriptIsolatedWorldRequest(
base::WeakPtr<RenderFrameImpl> render_frame_impl,
+ bool wants_result,
JavaScriptExecuteRequestInIsolatedWorldCallback callback);
void Completed(
const blink::WebVector<v8::Local<v8::Value>>& result) override;
@@ -1036,6 +1040,7 @@
~JavaScriptIsolatedWorldRequest() override;
base::WeakPtr<RenderFrameImpl> render_frame_impl_;
+ bool wants_result_;
JavaScriptExecuteRequestInIsolatedWorldCallback callback_;
DISALLOW_COPY_AND_ASSIGN(JavaScriptIsolatedWorldRequest);