New issue
Advanced search Search tips

Issue 775930 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug

Blocking:
issue 696617



Sign in to add a comment

LocalFrameClientImpl should propagate gesture bit consistently for all clients

Project Member Reported by mustaq@chromium.org, Oct 18 2017

Issue description

LocalFrameClientImpl::SetHasReceivedUserGesture currently updates AutofillClient unconditionally w/o checking if |received_previously| is true or false.

However, it updates other clients only if |received_previously| is false.

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/exported/LocalFrameClientImpl.cpp?rcl=2a5a0d8e3bec22edfa275a32264613fc28430ca4&l=1066

(Note that there seems to be only one other client: RenderFrameImpl::SetHasReceivedUserGesture)

The distinction is not clear since this only updates the sticky gesture bit now.

This is important for us because when we will have our simple activation model (Issue 696617), we may make the first case unconditional so that the transient activation bit gets updated appropriately.

 

Comment 1 by mustaq@chromium.org, Oct 18 2017

Blocking: 696617

Comment 2 by mustaq@chromium.org, Oct 18 2017

Description: Show this description
Components: Blink>Input
Labels: UserActivation
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eeed7dfe9a57495020586b071c56dec3590f83fb

commit eeed7dfe9a57495020586b071c56dec3590f83fb
Author: Mustaq Ahmed <mustaq@google.com>
Date: Tue Mar 27 20:16:52 2018

Make activation notification to other processes unconditional

We originally had the notification conditional on whether the current
frame had been activated before or not.  This works fine for the sticky
activation bit but for the transient bit (which can expire or get
consumed), other processes should be notified at every user activation.

Bug:  780556 ,  775930 
Change-Id: I902129651504371bad0b9ab841e65b223bee46b4
Reviewed-on: https://chromium-review.googlesource.com/973507
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546229}
[modify] https://crrev.com/eeed7dfe9a57495020586b071c56dec3590f83fb/third_party/WebKit/Source/core/exported/LocalFrameClientImpl.cpp
[modify] https://crrev.com/eeed7dfe9a57495020586b071c56dec3590f83fb/third_party/WebKit/Source/core/exported/LocalFrameClientImpl.h
[modify] https://crrev.com/eeed7dfe9a57495020586b071c56dec3590f83fb/third_party/WebKit/Source/core/frame/Frame.cpp
[modify] https://crrev.com/eeed7dfe9a57495020586b071c56dec3590f83fb/third_party/WebKit/Source/core/frame/LocalFrameClient.h

Comment 6 by mustaq@chromium.org, Mar 27 2018

Status: Fixed (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 5 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c4cb716e319efaa1c2582565c5f18965dfa39366

commit c4cb716e319efaa1c2582565c5f18965dfa39366
Author: Mustaq Ahmed <mustaq@google.com>
Date: Tue Jun 05 16:28:36 2018

Add browser-side activation state with sync across OOPIFs.

In this CL we added replicated activation state in the
browser process, and fixed propagation in the frame trees
across all renderer processes.  We substituted
the IPCs Frame*Msg_SetHasReceivedUserGesture (used for
notifying activation only) with ones that communicate both
notification & consumption of user activations.

- Lack of browser-side activation states made it
impossible to avoid race conditions with cross-process
postMessages.  All ChromeSitePerProcessTests now pass with
UAv2 (including one that was disabled before).

- Previously, Frame::NotifyUserActivation didn't propagate
the notification across renderers after the first
activation, making the transient bit unavailable after the
first successful consumption during the frame's lifetime.

- Frame::ConsumeTransientUserActivation was incomplete for
OOPIFs, didn't update RemoteFrames in other renderers.
Also fixed a bug (not consuming across the frame tree)
that can be exploited to consume multiple times from
different frames.

Design doc:
https://docs.google.com/document/d/1XL3vCedkqL65ueaGVD-kfB5RnnrnTaxLc7kmU91oerg

Bug:  780556 ,  775930 
Change-Id: I3ea0f5f9d314f6a8bfd88b3bcf8c12700c560cc7
Reviewed-on: https://chromium-review.googlesource.com/967260
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564530}
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/chrome/browser/chrome_site_per_process_browsertest.cc
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/content/browser/frame_host/frame_tree_node.cc
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/content/browser/frame_host/frame_tree_node.h
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/content/browser/frame_host/render_frame_host_manager.cc
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/content/browser/frame_host/render_frame_host_manager.h
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/content/common/frame.mojom
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/content/common/frame_messages.h
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/content/renderer/render_frame_impl.h
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/content/renderer/render_frame_proxy.cc
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/content/renderer/render_frame_proxy.h
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/content/renderer/render_view_impl.cc
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/third_party/blink/common/BUILD.gn
[rename] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/third_party/blink/common/frame/user_activation_state.cc
[add] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/third_party/blink/common/frame/user_activation_state_unittest.cc
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/third_party/blink/public/common/BUILD.gn
[add] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/third_party/blink/public/common/frame/user_activation_state.h
[add] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/third_party/blink/public/common/frame/user_activation_update_type.h
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/third_party/blink/public/web/web_local_frame_client.h
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/third_party/blink/public/web/web_remote_frame.h
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/third_party/blink/renderer/core/exported/local_frame_client_impl.cc
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/third_party/blink/renderer/core/exported/local_frame_client_impl.h
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/third_party/blink/renderer/core/exported/web_remote_frame_impl.cc
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/third_party/blink/renderer/core/exported/web_remote_frame_impl.h
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/third_party/blink/renderer/core/frame/BUILD.gn
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/third_party/blink/renderer/core/frame/frame.cc
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/third_party/blink/renderer/core/frame/frame.h
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/third_party/blink/renderer/core/frame/frame_test.cc
[modify] https://crrev.com/c4cb716e319efaa1c2582565c5f18965dfa39366/third_party/blink/renderer/core/frame/local_frame_client.h
[delete] https://crrev.com/0ae4e766b88f7359be42bc796b9fab36369ab28f/third_party/blink/renderer/core/frame/user_activation_state.h

Sign in to add a comment