Skip to content

Commit

Permalink
Blink: Move isolated world related calls out of WebLocalFrame.
Browse files Browse the repository at this point in the history
Isolated worlds are per renderer. Hence the functions to set
IsolatedWorldInfo and query information about an isolated world don't
belong on a WebLocalFrame. This CL moves them out of the WebLocalFrame.

In WebLocalFrameImpl::SetIsolatedWorldInfo, currently we use the
corresponding dom window's agent cluster ID for the isolated world
origin. This is probably incorrect since the isolated world security
origin is per renderer. To accommodate agent cluster ID,
DOMWrapperWorld::IsolatedWorldSecurityOrigin is modified to take in a
cluster ID from the client.

BUG=961448, 896041

Change-Id: Ice72c3f4f3f99372093422ea581ba0545375122c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2255746
Auto-Submit: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#788942}
  • Loading branch information
Karandeep Bhatia authored and Commit Bot committed Jul 16, 2020
1 parent b4592b5 commit 4a8b435
Show file tree
Hide file tree
Showing 21 changed files with 174 additions and 150 deletions.
2 changes: 2 additions & 0 deletions components/translate/content/renderer/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import("//build/config/features.gni")

static_library("renderer") {
sources = [
"isolated_world_util.cc",
"isolated_world_util.h",
"per_frame_translate_agent.cc",
"per_frame_translate_agent.h",
"translate_agent.cc",
Expand Down
36 changes: 36 additions & 0 deletions components/translate/content/renderer/isolated_world_util.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "components/translate/content/renderer/isolated_world_util.h"

#include "base/check_op.h"
#include "base/optional.h"
#include "components/translate/core/common/translate_util.h"
#include "third_party/blink/public/platform/web_isolated_world_info.h"
#include "third_party/blink/public/platform/web_url.h"

namespace translate {

void EnsureIsolatedWorldInitialized(int world_id) {
static base::Optional<int> last_used_world_id;
if (last_used_world_id) {
// Early return since the isolated world info. is already initialized.
DCHECK_EQ(*last_used_world_id, world_id)
<< "EnsureIsolatedWorldInitialized should always be called with the "
"same |world_id|";
return;
}

last_used_world_id = world_id;
constexpr char kContentSecurityPolicy[] = "script-src 'self' 'unsafe-eval'";

blink::WebIsolatedWorldInfo info;
info.security_origin =
blink::WebSecurityOrigin::Create(GetTranslateSecurityOrigin());
info.content_security_policy =
blink::WebString::FromUTF8(kContentSecurityPolicy);
blink::SetIsolatedWorldInfo(world_id, info);
}

} // namespace translate
15 changes: 15 additions & 0 deletions components/translate/content/renderer/isolated_world_util.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef COMPONENTS_TRANSLATE_CONTENT_RENDERER_ISOLATED_WORLD_UTIL_H_
#define COMPONENTS_TRANSLATE_CONTENT_RENDERER_ISOLATED_WORLD_UTIL_H_

namespace translate {

// Ensures the isolated world information for |world_id| is initialized.
void EnsureIsolatedWorldInitialized(int world_id);

} // namespace translate

#endif // COMPONENTS_TRANSLATE_CONTENT_RENDERER_ISOLATED_WORLD_UTIL_H_
40 changes: 7 additions & 33 deletions components/translate/content/renderer/per_frame_translate_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "base/strings/string16.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "components/translate/content/renderer/isolated_world_util.h"
#include "components/translate/core/common/translate_constants.h"
#include "components/translate/core/common/translate_metrics.h"
#include "components/translate/core/common/translate_util.h"
Expand All @@ -23,7 +24,6 @@
#include "content/public/renderer/render_frame.h"
#include "services/service_manager/public/cpp/interface_provider.h"
#include "third_party/blink/public/common/browser_interface_broker_proxy.h"
#include "third_party/blink/public/platform/web_isolated_world_info.h"
#include "third_party/blink/public/web/web_document.h"
#include "third_party/blink/public/web/web_language_detection_details.h"
#include "third_party/blink/public/web/web_local_frame.h"
Expand All @@ -35,7 +35,6 @@ using blink::WebDocument;
using blink::WebLanguageDetectionDetails;
using blink::WebLocalFrame;
using blink::WebScriptSource;
using blink::WebSecurityOrigin;
using blink::WebString;
using blink::WebVector;

Expand All @@ -61,9 +60,6 @@ const int kMaxTranslateStatusCheckAttempts = 10;
// Language name passed to the Translate element for it to detect the language.
const char kAutoDetectionLanguage[] = "auto";

// Isolated world sets following content-security-policy.
const char kContentSecurityPolicy[] = "script-src 'self' 'unsafe-eval'";

} // namespace

namespace translate {
Expand Down Expand Up @@ -131,16 +127,7 @@ void PerFrameTranslateAgent::TranslateFrame(const std::string& translate_script,
GURL url(render_frame()->GetWebFrame()->GetDocument().Url());
ReportPageScheme(url.scheme());

if (!isolated_world_initialized_) {
// Set up v8 isolated world with proper content-security-policy and
// security-origin.
blink::WebIsolatedWorldInfo info;
info.security_origin =
WebSecurityOrigin::Create(GetTranslateSecurityOrigin());
info.content_security_policy = WebString::FromUTF8(kContentSecurityPolicy);
render_frame()->GetWebFrame()->SetIsolatedWorldInfo(world_id_, info);
isolated_world_initialized_ = true;
}
EnsureIsolatedWorldInitialized(world_id_);

if (!IsTranslateLibAvailable()) {
// Evaluate the script to add the translation related method to the global
Expand Down Expand Up @@ -206,7 +193,7 @@ base::TimeDelta PerFrameTranslateAgent::AdjustDelay(int delay_in_milliseconds) {
}

void PerFrameTranslateAgent::ExecuteScript(const std::string& script) {
EnsureIsolatedWorldInitialized();
EnsureIsolatedWorldInitialized(world_id_);
WebLocalFrame* local_frame = render_frame()->GetWebFrame();
if (!local_frame)
return;
Expand All @@ -218,7 +205,7 @@ void PerFrameTranslateAgent::ExecuteScript(const std::string& script) {
bool PerFrameTranslateAgent::ExecuteScriptAndGetBoolResult(
const std::string& script,
bool fallback) {
EnsureIsolatedWorldInitialized();
EnsureIsolatedWorldInitialized(world_id_);
WebLocalFrame* local_frame = render_frame()->GetWebFrame();
if (!local_frame)
return fallback;
Expand All @@ -235,7 +222,7 @@ bool PerFrameTranslateAgent::ExecuteScriptAndGetBoolResult(

std::string PerFrameTranslateAgent::ExecuteScriptAndGetStringResult(
const std::string& script) {
EnsureIsolatedWorldInitialized();
EnsureIsolatedWorldInitialized(world_id_);
WebLocalFrame* local_frame = render_frame()->GetWebFrame();
if (!local_frame)
return std::string();
Expand All @@ -257,7 +244,7 @@ std::string PerFrameTranslateAgent::ExecuteScriptAndGetStringResult(

double PerFrameTranslateAgent::ExecuteScriptAndGetDoubleResult(
const std::string& script) {
EnsureIsolatedWorldInitialized();
EnsureIsolatedWorldInitialized(world_id_);
WebLocalFrame* local_frame = render_frame()->GetWebFrame();
if (!local_frame)
return 0.0;
Expand All @@ -274,7 +261,7 @@ double PerFrameTranslateAgent::ExecuteScriptAndGetDoubleResult(

int64_t PerFrameTranslateAgent::ExecuteScriptAndGetIntegerResult(
const std::string& script) {
EnsureIsolatedWorldInitialized();
EnsureIsolatedWorldInitialized(world_id_);
WebLocalFrame* local_frame = render_frame()->GetWebFrame();
if (!local_frame)
return 0;
Expand Down Expand Up @@ -422,19 +409,6 @@ void PerFrameTranslateAgent::CancelPendingTranslation() {
target_lang_.clear();
}

void PerFrameTranslateAgent::EnsureIsolatedWorldInitialized() {
if (!isolated_world_initialized_) {
// Set up v8 isolated world with proper content-security-policy and
// security-origin.
blink::WebIsolatedWorldInfo info;
info.security_origin =
WebSecurityOrigin::Create(GetTranslateSecurityOrigin());
info.content_security_policy = WebString::FromUTF8(kContentSecurityPolicy);
render_frame()->GetWebFrame()->SetIsolatedWorldInfo(world_id_, info);
isolated_world_initialized_ = true;
}
}

void PerFrameTranslateAgent::BindReceiver(
mojo::PendingAssociatedReceiver<mojom::TranslateAgent> receiver) {
receiver_.Bind(std::move(receiver));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,6 @@ class PerFrameTranslateAgent : public content::RenderFrameObserver,
// Converts language code to the one used in server supporting list.
static void ConvertLanguageCodeSynonym(std::string* code);

// Ensures that the isolated world for executing translation JS.
void EnsureIsolatedWorldInitialized();

// Sets receiver for translate messages from browser process.
void BindReceiver(
mojo::PendingAssociatedReceiver<mojom::TranslateAgent> receiver);
Expand Down Expand Up @@ -148,9 +145,6 @@ class PerFrameTranslateAgent : public content::RenderFrameObserver,
// The world ID to use for script execution.
int world_id_;

// Whether the isolated world info has been initialized.
bool isolated_world_initialized_ = false;

mojo::AssociatedReceiver<mojom::TranslateAgent> receiver_{this};

// Method factory used to make calls to TranslateFrameImpl.
Expand Down
16 changes: 3 additions & 13 deletions components/translate/content/renderer/translate_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "base/strings/string16.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "components/translate/content/renderer/isolated_world_util.h"
#include "components/translate/core/common/translate_constants.h"
#include "components/translate/core/common/translate_metrics.h"
#include "components/translate/core/common/translate_util.h"
Expand All @@ -26,7 +27,6 @@
#include "content/public/renderer/render_frame.h"
#include "content/public/renderer/render_thread.h"
#include "third_party/blink/public/common/browser_interface_broker_proxy.h"
#include "third_party/blink/public/platform/web_isolated_world_info.h"
#include "third_party/blink/public/web/web_document.h"
#include "third_party/blink/public/web/web_language_detection_details.h"
#include "third_party/blink/public/web/web_local_frame.h"
Expand All @@ -38,7 +38,6 @@ using blink::WebDocument;
using blink::WebLanguageDetectionDetails;
using blink::WebLocalFrame;
using blink::WebScriptSource;
using blink::WebSecurityOrigin;
using blink::WebString;
using blink::WebVector;

Expand All @@ -58,10 +57,6 @@ const int kTranslateStatusCheckDelayMs = 400;

// Language name passed to the Translate element for it to detect the language.
const char kAutoDetectionLanguage[] = "auto";

// Isolated world sets following content-security-policy.
const char kContentSecurityPolicy[] = "script-src 'self' 'unsafe-eval'";

} // namespace

namespace translate {
Expand Down Expand Up @@ -320,13 +315,8 @@ void TranslateAgent::TranslateFrame(const std::string& translate_script,
GURL url(main_frame->GetDocument().Url());
ReportPageScheme(url.scheme());

// Set up v8 isolated world with proper content-security-policy and
// security-origin.
blink::WebIsolatedWorldInfo info;
info.security_origin =
WebSecurityOrigin::Create(GetTranslateSecurityOrigin());
info.content_security_policy = WebString::FromUTF8(kContentSecurityPolicy);
main_frame->SetIsolatedWorldInfo(world_id_, info);
// Set up v8 isolated world.
EnsureIsolatedWorldInitialized(world_id_);

if (!IsTranslateLibAvailable()) {
// Evaluate the script to add the translation related method to the global
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "base/containers/flat_map.h"
#include "content/public/common/isolated_world_ids.h"
#include "mojo/public/cpp/bindings/self_owned_receiver.h"
#include "third_party/blink/public/platform/web_isolated_world_info.h"
#include "third_party/blink/public/web/web_local_frame.h"
#include "third_party/blink/public/web/web_local_frame_client.h"
#include "v8/include/v8.h"
Expand Down Expand Up @@ -87,9 +88,9 @@ class FrameAssociatedMeasurementDelegate : public v8::MeasureMemoryDelegate {

if (world_id != content::ISOLATED_WORLD_ID_GLOBAL) {
isolated_world_usage->stable_id =
frame->GetIsolatedWorldStableId(context).Utf8();
blink::GetIsolatedWorldStableId(context).Utf8();
isolated_world_usage->human_readable_name =
frame->GetIsolatedWorldHumanReadableName(context).Utf8();
blink::GetIsolatedWorldHumanReadableName(context).Utf8();
}

DCHECK(
Expand Down
2 changes: 1 addition & 1 deletion content/shell/renderer/web_test/test_runner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1108,7 +1108,7 @@ void TestRunnerBindings::SetIsolatedWorldInfo(
// Clear the document->isolated world CSP mapping.
GetWebFrame()->ClearIsolatedWorldCSPForTesting(world_id);

GetWebFrame()->SetIsolatedWorldInfo(world_id, info);
blink::SetIsolatedWorldInfo(world_id, info);
}

void TestRunnerBindings::AddOriginAccessAllowListEntry(
Expand Down
24 changes: 11 additions & 13 deletions extensions/renderer/script_injection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,10 @@ const int64_t kInvalidRequestId = -1;
// The id of the next pending injection.
int64_t g_next_pending_id = 0;

// Gets the isolated world ID to use for the given |injection_host|
// in the given |frame|. If no isolated world has been created for that
// |injection_host| one will be created and initialized.
int GetIsolatedWorldIdForInstance(const InjectionHost* injection_host,
blink::WebLocalFrame* frame) {
// Gets the isolated world ID to use for the given |injection_host|. If no
// isolated world has been created for that |injection_host| one will be created
// and initialized.
int GetIsolatedWorldIdForInstance(const InjectionHost* injection_host) {
static int g_next_isolated_world_id =
ExtensionsRendererClient::Get()->GetLowestIsolatedWorldId();

Expand All @@ -67,9 +66,6 @@ int GetIsolatedWorldIdForInstance(const InjectionHost* injection_host,
isolated_worlds[key] = id;
}

// We need to set the isolated world origin and CSP even if it's not a new
// world since these are stored per frame, and we might not have used this
// isolated world in this frame before.
blink::WebIsolatedWorldInfo info;
info.security_origin =
blink::WebSecurityOrigin::Create(injection_host->url());
Expand All @@ -80,7 +76,10 @@ int GetIsolatedWorldIdForInstance(const InjectionHost* injection_host,
if (csp)
info.content_security_policy = blink::WebString::FromUTF8(*csp);

frame->SetIsolatedWorldInfo(id, info);
// Even though there may be an existing world for this |injection_host|'s key,
// the properties may have changed (e.g. due to an extension update).
// Overwrite any existing entries.
blink::SetIsolatedWorldInfo(id, info);

return id;
}
Expand Down Expand Up @@ -285,12 +284,10 @@ ScriptInjection::InjectionResult ScriptInjection::Inject(
void ScriptInjection::InjectJs(std::set<std::string>* executing_scripts,
size_t* num_injected_js_scripts) {
DCHECK(!did_inject_js_);
blink::WebLocalFrame* web_frame = render_frame_->GetWebFrame();
std::vector<blink::WebScriptSource> sources = injector_->GetJsSources(
run_location_, executing_scripts, num_injected_js_scripts);
DCHECK(!sources.empty());
int world_id =
GetIsolatedWorldIdForInstance(injection_host_.get(), web_frame);
int world_id = GetIsolatedWorldIdForInstance(injection_host_.get());
bool is_user_gesture = injector_->IsUserGesture();

std::unique_ptr<blink::WebScriptExecutionCallback> callback(
Expand All @@ -315,7 +312,8 @@ void ScriptInjection::InjectJs(std::set<std::string>* executing_scripts,
should_execute_asynchronously
? blink::WebLocalFrame::kAsynchronousBlockingOnload
: blink::WebLocalFrame::kSynchronous;
web_frame->RequestExecuteScriptInIsolatedWorld(

render_frame_->GetWebFrame()->RequestExecuteScriptInIsolatedWorld(
world_id, &sources.front(), sources.size(), is_user_gesture,
execution_option, callback.release());
}
Expand Down
15 changes: 15 additions & 0 deletions third_party/blink/public/platform/web_isolated_world_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "third_party/blink/public/platform/web_common.h"
#include "third_party/blink/public/platform/web_security_origin.h"
#include "third_party/blink/public/platform/web_string.h"
#include "v8/include/v8.h"

#ifndef THIRD_PARTY_BLINK_PUBLIC_PLATFORM_WEB_ISOLATED_WORLD_INFO_H_
#define THIRD_PARTY_BLINK_PUBLIC_PLATFORM_WEB_ISOLATED_WORLD_INFO_H_
Expand Down Expand Up @@ -49,6 +51,19 @@ struct WebIsolatedWorldInfo {
WebString stable_id;
};

// Sets up an isolated world by associating a |world_id| with |info|.
// worldID must be > 0 (as 0 represents the main world).
// worldID must be < kEmbedderWorldIdLimit, high number used internally.
BLINK_EXPORT void SetIsolatedWorldInfo(int32_t world_id,
const WebIsolatedWorldInfo& info);

// Returns the stable ID that was set with SetIsolatedWorldInfo.
BLINK_EXPORT WebString GetIsolatedWorldStableId(v8::Local<v8::Context>);

// Returns the human readable name that was set with SetIsolatedWorldInfo.
BLINK_EXPORT WebString
GetIsolatedWorldHumanReadableName(v8::Local<v8::Context>);

} // namespace blink

#endif // THIRD_PARTY_BLINK_PUBLIC_PLATFORM_WEB_ISOLATED_WORLD_INFO_H_
15 changes: 0 additions & 15 deletions third_party/blink/public/web/web_local_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -316,21 +316,6 @@ class WebLocalFrame : public WebFrame {
// Document.
virtual void ClearIsolatedWorldCSPForTesting(int32_t world_id) = 0;

// Sets up an isolated world by associating a |world_id| with |info|.
// worldID must be > 0 (as 0 represents the main world).
// worldID must be < kEmbedderWorldIdLimit, high number used internally.
// TODO(karandeepb): This modifies the global isolated world info and hence
// should ideally be moved out of WebLocalFrame.
virtual void SetIsolatedWorldInfo(int32_t world_id,
const WebIsolatedWorldInfo& info) = 0;

// Returns the stable ID that was set with SetIsolatedWorldInfo.
virtual WebString GetIsolatedWorldStableId(v8::Local<v8::Context>) const = 0;

// Returns the human readable name that was set with SetIsolatedWorldInfo.
virtual WebString GetIsolatedWorldHumanReadableName(
v8::Local<v8::Context>) const = 0;

// Executes script in the context of the current page and returns the value
// that the script evaluated to.
// DEPRECATED: Use WebLocalFrame::requestExecuteScriptAndReturnValue.
Expand Down

0 comments on commit 4a8b435

Please sign in to comment.