Skip to content

Commit

Permalink
Don't consume user activation when opening windows in WebView
Browse files Browse the repository at this point in the history
It should in principle be safe to consume user activation here, but there must be checks later on that expect the frame to still have user activation in the renderer. It's not harmful from a security perspective not to consume activation, as the preexisting comment notes.

(cherry picked from commit 4dccf74)

Bug: 1314768
Change-Id: I74862b8e181d9128212d03c89c7a4cf5fce6b4d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3584659
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Mustaq Ahmed <mustaq@chromium.org>
Commit-Queue: Garrett Tanzer <gtanzer@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#992677}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3593255
Owners-Override: Ben Mason <benmason@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Ben Mason <benmason@chromium.org>
Cr-Commit-Position: refs/branch-heads/4951@{#888}
Cr-Branched-From: 27de622-refs/heads/main@{#982481}
  • Loading branch information
Garrett Tanzer authored and Chromium LUCI CQ committed Apr 19, 2022
1 parent 0893dd0 commit 18a9d25
Showing 1 changed file with 9 additions and 9 deletions.
18 changes: 9 additions & 9 deletions content/renderer/render_view_impl.cc
Expand Up @@ -320,15 +320,6 @@ WebView* RenderViewImpl::CreateView(
if (status == mojom::CreateNewWindowStatus::kBlocked)
return nullptr;

// Consume the transient user activation in the current renderer.
consumed_user_gesture = creator->ConsumeTransientUserActivation(
blink::UserActivationUpdateSource::kBrowser);

// If we should ignore the new window (e.g. because of `noopener`), return
// now that user activation was consumed.
if (status == mojom::CreateNewWindowStatus::kIgnore)
return nullptr;

// For Android WebView, we support a pop-up like behavior for window.open()
// even if the embedding app doesn't support multiple windows. In this case,
// window.open() will return "window" and navigate it to whatever URL was
Expand All @@ -341,6 +332,15 @@ WebView* RenderViewImpl::CreateView(
if (status == mojom::CreateNewWindowStatus::kReuse)
return GetWebView();

// Consume the transient user activation in the current renderer.
consumed_user_gesture = creator->ConsumeTransientUserActivation(
blink::UserActivationUpdateSource::kBrowser);

// If we should ignore the new window (e.g. because of `noopener`), return
// now that user activation was consumed.
if (status == mojom::CreateNewWindowStatus::kIgnore)
return nullptr;

DCHECK(reply);
DCHECK_NE(MSG_ROUTING_NONE, reply->route_id);
DCHECK_NE(MSG_ROUTING_NONE, reply->main_frame_route_id);
Expand Down

0 comments on commit 18a9d25

Please sign in to comment.