From a1a0b1aefdc8969fe71d35c97299503f9650563d Mon Sep 17 00:00:00 2001 From: Pavel Beloborodov <73575789+boocmp@users.noreply.github.com> Date: Mon, 20 Feb 2023 15:03:19 +0700 Subject: [PATCH] Replaced speedreader isolated world id to ISOLATED_WORLD_ID_BRAVE_INTERNAL. (#17268) --- .../speedreader/speedreader_browsertest.cc | 26 ++++++++++--------- browser/speedreader/speedreader_tab_helper.cc | 7 ++--- components/speedreader/common/constants.h | 3 --- .../renderer/speedreader_js_handler.cc | 9 ++++--- .../renderer/speedreader_js_handler.h | 3 ++- .../speedreader_render_frame_observer.cc | 9 ++++--- .../speedreader_render_frame_observer.h | 6 ++--- renderer/brave_content_renderer_client.cc | 3 ++- 8 files changed, 36 insertions(+), 30 deletions(-) diff --git a/browser/speedreader/speedreader_browsertest.cc b/browser/speedreader/speedreader_browsertest.cc index a7ebd519831..1f17bdce318 100644 --- a/browser/speedreader/speedreader_browsertest.cc +++ b/browser/speedreader/speedreader_browsertest.cc @@ -33,6 +33,7 @@ #include "chrome/browser/ui/views/frame/browser_view.h" #include "chrome/browser/ui/views/frame/toolbar_button_provider.h" #include "chrome/browser/ui/views/page_action/page_action_icon_view.h" +#include "chrome/common/chrome_isolated_world_ids.h" #include "chrome/test/base/in_process_browser_test.h" #include "chrome/test/base/ui_test_utils.h" #include "components/keep_alive_registry/keep_alive_types.h" @@ -73,8 +74,9 @@ class SpeedReaderBrowserTest : public InProcessBrowserTest { auto redirector = [](const net::test_server::HttpRequest& request) -> std::unique_ptr { - if (request.GetURL().path_piece() != kTestPageRedirect) + if (request.GetURL().path_piece() != kTestPageRedirect) { return nullptr; + } const std::string dest = base::UnescapeBinaryURLComponent(request.GetURL().query_piece()); @@ -230,15 +232,15 @@ IN_PROC_BROWSER_TEST_F(SpeedReaderBrowserTest, SmokeTest) { // style is injected. EXPECT_LT(0, content::EvalJs(ActiveWebContents(), kGetStyleLength, content::EXECUTE_SCRIPT_DEFAULT_OPTIONS, - speedreader::kIsolatedWorldId) + ISOLATED_WORLD_ID_BRAVE_INTERNAL) .ExtractInt()); EXPECT_TRUE(content::EvalJs(ActiveWebContents(), kGetFontsExists, content::EXECUTE_SCRIPT_DEFAULT_OPTIONS, - speedreader::kIsolatedWorldId) + ISOLATED_WORLD_ID_BRAVE_INTERNAL) .ExtractBool()); EXPECT_GT(17750, content::EvalJs(ActiveWebContents(), kGetContentLength, content::EXECUTE_SCRIPT_DEFAULT_OPTIONS, - speedreader::kIsolatedWorldId) + ISOLATED_WORLD_ID_BRAVE_INTERNAL) .ExtractInt()); EXPECT_TRUE(console_observer.messages().empty()); @@ -248,7 +250,7 @@ IN_PROC_BROWSER_TEST_F(SpeedReaderBrowserTest, SmokeTest) { NavigateToPageSynchronously(kTestPageReadable); EXPECT_LT(106000, content::EvalJs(ActiveWebContents(), kGetContentLength, content::EXECUTE_SCRIPT_DEFAULT_OPTIONS, - speedreader::kIsolatedWorldId) + ISOLATED_WORLD_ID_BRAVE_INTERNAL) .ExtractInt()); } @@ -264,7 +266,7 @@ IN_PROC_BROWSER_TEST_F(SpeedReaderBrowserTest, Redirect) { EXPECT_TRUE(content::EvalJs(ActiveWebContents(), kCheckNoStyle, content::EXECUTE_SCRIPT_DEFAULT_OPTIONS, - speedreader::kIsolatedWorldId) + ISOLATED_WORLD_ID_BRAVE_INTERNAL) .ExtractBool()); } @@ -419,7 +421,7 @@ IN_PROC_BROWSER_TEST_F(SpeedReaderBrowserTest, ShowOriginalPage) { EXPECT_EQ(base::UTF16ToUTF8(title), content::EvalJs(web_contents, kClickLinkAndGetTitle, content::EXECUTE_SCRIPT_DEFAULT_OPTIONS, - speedreader::kIsolatedWorldId) + ISOLATED_WORLD_ID_BRAVE_INTERNAL) .ExtractString()); content::WaitForLoadStop(web_contents); auto* tab_helper = @@ -449,7 +451,7 @@ IN_PROC_BROWSER_TEST_F(SpeedReaderBrowserTest, ShowOriginalPageOnUnreadable) { EXPECT_TRUE(content::EvalJs(web_contents, kCheckNoElement, content::EXECUTE_SCRIPT_DEFAULT_OPTIONS, - speedreader::kIsolatedWorldId) + ISOLATED_WORLD_ID_BRAVE_INTERNAL) .ExtractBool()); constexpr const char kCheckNoApi[] = @@ -459,7 +461,7 @@ IN_PROC_BROWSER_TEST_F(SpeedReaderBrowserTest, ShowOriginalPageOnUnreadable) { EXPECT_TRUE(content::EvalJs(web_contents, kCheckNoApi, content::EXECUTE_SCRIPT_DEFAULT_OPTIONS, - speedreader::kIsolatedWorldId) + ISOLATED_WORLD_ID_BRAVE_INTERNAL) .ExtractBool()); } @@ -490,7 +492,7 @@ IN_PROC_BROWSER_TEST_F(SpeedReaderBrowserTest, SetDataAttributes) { EXPECT_EQ(nullptr, content::EvalJs(contents, GetDataAttribute("data-theme"), content::EXECUTE_SCRIPT_DEFAULT_OPTIONS, - speedreader::kIsolatedWorldId)); + ISOLATED_WORLD_ID_BRAVE_INTERNAL)); auto* tab_helper = speedreader::SpeedreaderTabHelper::FromWebContents(contents); tab_helper->SetTheme(speedreader::mojom::Theme::kDark); @@ -503,7 +505,7 @@ IN_PROC_BROWSER_TEST_F(SpeedReaderBrowserTest, SetDataAttributes) { auto EvalAttr = [&](content::WebContents* contents, const std::string& attr) { return content::EvalJs(contents, GetDataAttribute(attr), content::EXECUTE_SCRIPT_DEFAULT_OPTIONS, - speedreader::kIsolatedWorldId) + ISOLATED_WORLD_ID_BRAVE_INTERNAL) .ExtractString(); }; @@ -546,7 +548,7 @@ IN_PROC_BROWSER_TEST_F(SpeedReaderBrowserTest, RSS) { EXPECT_EQ(nullptr, content::EvalJs(ActiveWebContents(), kNoStyleInjected, content::EXECUTE_SCRIPT_DEFAULT_OPTIONS, - speedreader::kIsolatedWorldId)); + ISOLATED_WORLD_ID_BRAVE_INTERNAL)); } class SpeedReaderBrowserPanelV2Test : public SpeedReaderBrowserTest { diff --git a/browser/speedreader/speedreader_tab_helper.cc b/browser/speedreader/speedreader_tab_helper.cc index 231e409d773..d53fb41540f 100644 --- a/browser/speedreader/speedreader_tab_helper.cc +++ b/browser/speedreader/speedreader_tab_helper.cc @@ -29,6 +29,7 @@ #include "brave/grit/brave_generated_resources.h" #include "chrome/browser/content_settings/host_content_settings_map_factory.h" #include "chrome/browser/profiles/profile.h" +#include "chrome/common/chrome_isolated_world_ids.h" #include "components/content_settings/core/browser/host_content_settings_map.h" #include "components/grit/brave_components_strings.h" #include "components/prefs/pref_change_registrar.h" @@ -530,8 +531,8 @@ void SpeedreaderTabHelper::DOMContentLoaded( const auto script = base::ReplaceStringPlaceholders(kAddShowOriginalPageLink, link_text, nullptr); - render_frame_host->ExecuteJavaScriptInIsolatedWorld(script, base::DoNothing(), - kIsolatedWorldId); + render_frame_host->ExecuteJavaScriptInIsolatedWorld( + script, base::DoNothing(), ISOLATED_WORLD_ID_BRAVE_INTERNAL); } void SpeedreaderTabHelper::OnVisibilityChanged(content::Visibility visibility) { @@ -583,7 +584,7 @@ void SpeedreaderTabHelper::SetDocumentAttribute(const std::string& attribute, nullptr); web_contents()->GetPrimaryMainFrame()->ExecuteJavaScriptInIsolatedWorld( - script, base::DoNothing(), kIsolatedWorldId); + script, base::DoNothing(), ISOLATED_WORLD_ID_BRAVE_INTERNAL); } WEB_CONTENTS_USER_DATA_KEY_IMPL(SpeedreaderTabHelper); diff --git a/components/speedreader/common/constants.h b/components/speedreader/common/constants.h index b2c9938f52d..bb53501274c 100644 --- a/components/speedreader/common/constants.h +++ b/components/speedreader/common/constants.h @@ -12,9 +12,6 @@ namespace speedreader { -constexpr const int kIsolatedWorldId = - content::ISOLATED_WORLD_ID_CONTENT_END + 9; - constexpr webui::LocalizedString kLocalizedStrings[] = { {"braveSpeedreader", IDS_SPEEDREADER_BRAND_LABEL_2}, {"braveSpeedreaderAlwaysLoadLabel", IDS_SPEEDREADER_ALWAYS_LOAD_LABEL}, diff --git a/components/speedreader/renderer/speedreader_js_handler.cc b/components/speedreader/renderer/speedreader_js_handler.cc index 5dfba7e6de0..6bfde1338e1 100644 --- a/components/speedreader/renderer/speedreader_js_handler.cc +++ b/components/speedreader/renderer/speedreader_js_handler.cc @@ -7,7 +7,6 @@ #include -#include "brave/components/speedreader/common/constants.h" #include "brave/components/speedreader/common/speedreader.mojom.h" #include "brave/components/speedreader/renderer/speedreader_render_frame_observer.h" #include "content/public/renderer/render_frame.h" @@ -36,16 +35,18 @@ SpeedreaderJSHandler::~SpeedreaderJSHandler() = default; // static void SpeedreaderJSHandler::Install( - base::WeakPtr owner) { + base::WeakPtr owner, + int32_t isolated_world_id) { DCHECK(owner); v8::Isolate* isolate = blink::MainThreadIsolate(); v8::HandleScope handle_scope(isolate); v8::Local context = owner->render_frame()->GetWebFrame()->GetScriptContextFromWorldId( - isolate, kIsolatedWorldId); - if (context.IsEmpty()) + isolate, isolated_world_id); + if (context.IsEmpty()) { return; + } v8::Context::Scope context_scope(context); v8::Local global = context->Global(); diff --git a/components/speedreader/renderer/speedreader_js_handler.h b/components/speedreader/renderer/speedreader_js_handler.h index 41886c18e0a..adb190bfaee 100644 --- a/components/speedreader/renderer/speedreader_js_handler.h +++ b/components/speedreader/renderer/speedreader_js_handler.h @@ -21,7 +21,8 @@ class SpeedreaderJSHandler final : public gin::Wrappable { SpeedreaderJSHandler(const SpeedreaderJSHandler&) = delete; SpeedreaderJSHandler& operator=(const SpeedreaderJSHandler&) = delete; - static void Install(base::WeakPtr owner); + static void Install(base::WeakPtr owner, + int32_t isolated_world_id); private: explicit SpeedreaderJSHandler( diff --git a/components/speedreader/renderer/speedreader_render_frame_observer.cc b/components/speedreader/renderer/speedreader_render_frame_observer.cc index 3e97eecb9ec..8ae33071bd9 100644 --- a/components/speedreader/renderer/speedreader_render_frame_observer.cc +++ b/components/speedreader/renderer/speedreader_render_frame_observer.cc @@ -12,8 +12,10 @@ namespace speedreader { SpeedreaderRenderFrameObserver::SpeedreaderRenderFrameObserver( - content::RenderFrame* render_frame) - : RenderFrameObserver(render_frame) {} + content::RenderFrame* render_frame, + int32_t isolated_world_id) + : RenderFrameObserver(render_frame), + isolated_world_id_(isolated_world_id) {} SpeedreaderRenderFrameObserver::~SpeedreaderRenderFrameObserver() = default; @@ -26,7 +28,8 @@ void SpeedreaderRenderFrameObserver::DidStartNavigation( void SpeedreaderRenderFrameObserver::DidClearWindowObject() { if (!is_speedreadable_url_ || !render_frame()->IsMainFrame()) return; - SpeedreaderJSHandler::Install(weak_ptr_factory_.GetWeakPtr()); + SpeedreaderJSHandler::Install(weak_ptr_factory_.GetWeakPtr(), + isolated_world_id_); } void SpeedreaderRenderFrameObserver::OnDestruct() { diff --git a/components/speedreader/renderer/speedreader_render_frame_observer.h b/components/speedreader/renderer/speedreader_render_frame_observer.h index c4feabacce6..d17849dda0a 100644 --- a/components/speedreader/renderer/speedreader_render_frame_observer.h +++ b/components/speedreader/renderer/speedreader_render_frame_observer.h @@ -6,8 +6,6 @@ #ifndef BRAVE_COMPONENTS_SPEEDREADER_RENDERER_SPEEDREADER_RENDER_FRAME_OBSERVER_H_ #define BRAVE_COMPONENTS_SPEEDREADER_RENDERER_SPEEDREADER_RENDER_FRAME_OBSERVER_H_ -#include - #include "base/memory/weak_ptr.h" #include "content/public/renderer/render_frame.h" #include "content/public/renderer/render_frame_observer.h" @@ -16,7 +14,8 @@ namespace speedreader { class SpeedreaderRenderFrameObserver : public content::RenderFrameObserver { public: - explicit SpeedreaderRenderFrameObserver(content::RenderFrame* render_frame); + SpeedreaderRenderFrameObserver(content::RenderFrame* render_frame, + int32_t isolated_world_id); SpeedreaderRenderFrameObserver(const SpeedreaderRenderFrameObserver&) = delete; SpeedreaderRenderFrameObserver& operator=( @@ -33,6 +32,7 @@ class SpeedreaderRenderFrameObserver : public content::RenderFrameObserver { // RenderFrameObserver implementation. void OnDestruct() override; + int32_t isolated_world_id_; bool is_speedreadable_url_ = false; base::WeakPtrFactory weak_ptr_factory_{this}; }; diff --git a/renderer/brave_content_renderer_client.cc b/renderer/brave_content_renderer_client.cc index 6f9c28b81b9..b34b120a9ca 100644 --- a/renderer/brave_content_renderer_client.cc +++ b/renderer/brave_content_renderer_client.cc @@ -123,7 +123,8 @@ void BraveContentRendererClient::RenderFrameCreated( #if BUILDFLAG(ENABLE_SPEEDREADER) if (base::FeatureList::IsEnabled(speedreader::kSpeedreaderFeature)) { - new speedreader::SpeedreaderRenderFrameObserver(render_frame); + new speedreader::SpeedreaderRenderFrameObserver( + render_frame, ISOLATED_WORLD_ID_BRAVE_INTERNAL); } #endif