From c8dbe2bd4b8b1cdcd340500c988d349da81a0c5b Mon Sep 17 00:00:00 2001 From: Miyoung Shin Date: Thu, 8 Dec 2022 19:45:38 +0000 Subject: [PATCH] [MPArch] Fix a BadMessage crash in RFH::UpdateTargetURL The crash occurs by the bad message added at crrev.com/3807142, and the UpdateTargetURL Mojo message can be sent from the pre-rendered page's renderer when an element having the URL is focused. This CL removes the bad message and changes to ignore this MOJO message in prerendered pages. (cherry picked from commit 98cb31f9b5a67b651ca3a51116bc7f321ee1a904) Bug: 1372599 Change-Id: I8529deb2e53857af0b8066959bca7b89959a3e19 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3946400 Commit-Queue: Miyoung Shin Reviewed-by: Alexander Timin Cr-Original-Commit-Position: refs/heads/main@{#1061001} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4088534 Commit-Queue: Prudhvikumar Bommana Owners-Override: Prudhvikumar Bommana Owners-Override: Srinivas Sista Commit-Queue: Srinivas Sista Reviewed-by: Prudhvikumar Bommana Cr-Commit-Position: refs/branch-heads/5359@{#1131} Cr-Branched-From: 27d3765d341b09369006d030f83f582a29eb57ae-refs/heads/main@{#1058933} --- .../prerender/prerender_browsertest.cc | 41 +++++++++++++++++++ .../renderer_host/render_frame_host_impl.cc | 8 ++-- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/content/browser/preloading/prerender/prerender_browsertest.cc b/content/browser/preloading/prerender/prerender_browsertest.cc index ab929fe151b31..62984a89bb1ef 100644 --- a/content/browser/preloading/prerender/prerender_browsertest.cc +++ b/content/browser/preloading/prerender/prerender_browsertest.cc @@ -8698,4 +8698,45 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, ExpectFinalStatusForSpeculationRule(PrerenderHost::FinalStatus::kActivated); } +class UpdateTargetURLDelegate : public WebContentsDelegate { + public: + explicit UpdateTargetURLDelegate(WebContents* web_contents) { + web_contents->SetDelegate(this); + } + + UpdateTargetURLDelegate(const UpdateTargetURLDelegate&) = delete; + UpdateTargetURLDelegate& operator=(const UpdateTargetURLDelegate&) = delete; + + bool is_updated_target_url() { return is_updated_target_url_; } + + private: + void UpdateTargetURL(WebContents* source, const GURL& url) override { + is_updated_target_url_ = true; + } + + bool is_updated_target_url_ = false; +}; + +// Tests that text autosizer works per page. +IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, FocusChangeInPrerenderedPage) { + const GURL kInitialUrl = GetUrl("/empty.html"); + const GURL kPrerenderingUrl = GetUrl("/simple_links.html"); + + // Navigate to an initial page. + ASSERT_TRUE(NavigateToURL(shell(), kInitialUrl)); + + int host_id = AddPrerender(kPrerenderingUrl); + RenderFrameHostImpl* prerender_frame_host = + GetPrerenderedMainFrameHost(host_id); + + UpdateTargetURLDelegate delegate(shell()->web_contents()); + + // No crash. + EXPECT_TRUE(ExecJs(prerender_frame_host, + "document.getElementById('same_site_link').focus();")); + + // The prerendered page should not update the target url. + EXPECT_FALSE(delegate.is_updated_target_url()); +} + } // namespace content diff --git a/content/browser/renderer_host/render_frame_host_impl.cc b/content/browser/renderer_host/render_frame_host_impl.cc index e4e16ccdcd207..81ee9846e14c1 100644 --- a/content/browser/renderer_host/render_frame_host_impl.cc +++ b/content/browser/renderer_host/render_frame_host_impl.cc @@ -5491,12 +5491,12 @@ void RenderFrameHostImpl::TakeFocus(bool reverse) { void RenderFrameHostImpl::UpdateTargetURL( const GURL& url, blink::mojom::LocalMainFrameHost::UpdateTargetURLCallback callback) { - // Prerendering pages should not reach this code since the renderer only calls - // this when the mouse over the URL or keyboard focuses the URL. - if (lifecycle_state_ == LifecycleStateImpl::kPrerendering) { - mojo::ReportBadMessage("Unexpected UpdateTargetURL from renderer"); + // An inactive document should ignore to update the target url. + if (!IsActive()) { + std::move(callback).Run(); return; } + delegate_->UpdateTargetURL(this, url); std::move(callback).Run(); }