Skip to content

Commit

Permalink
[MPArch] Fix a BadMessage crash in RFH::UpdateTargetURL
Browse files Browse the repository at this point in the history
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 98cb31f)

Bug: 1372599
Change-Id: I8529deb2e53857af0b8066959bca7b89959a3e19
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3946400
Commit-Queue: Miyoung Shin <myid.shin@igalia.com>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1061001}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4088534
Commit-Queue: Prudhvikumar Bommana <pbommana@google.com>
Owners-Override: Prudhvikumar Bommana <pbommana@google.com>
Owners-Override: Srinivas Sista <srinivassista@chromium.org>
Commit-Queue: Srinivas Sista <srinivassista@chromium.org>
Reviewed-by: Prudhvikumar Bommana <pbommana@google.com>
Cr-Commit-Position: refs/branch-heads/5359@{#1131}
Cr-Branched-From: 27d3765-refs/heads/main@{#1058933}
  • Loading branch information
MyidShin authored and Chromium LUCI CQ committed Dec 8, 2022
1 parent 97d699d commit c8dbe2b
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 4 deletions.
41 changes: 41 additions & 0 deletions content/browser/preloading/prerender/prerender_browsertest.cc
Expand Up @@ -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
8 changes: 4 additions & 4 deletions content/browser/renderer_host/render_frame_host_impl.cc
Expand Up @@ -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();
}
Expand Down

0 comments on commit c8dbe2b

Please sign in to comment.