New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 667070 link

Starred by 2 users

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Inconsistent user gesture tracking during lifetime of a navigation

Project Member Reported by bmcquade@chromium.org, Nov 19 2016

Issue description

When a navigation is opened in a new tab, the user gesture state is not tracked consistently during the lifetime of a navigation.

To repro:
1. navigate to news.google.com
2. click on any link that opens a story in a new tab
3. observe that NavigationHandleImpl::WillStartRequest receives a has_user_gesture param set to true, while the later call to
NavigationHandleImpl::DidCommitNavigation receives a FrameHostMsg_DidCommitProvisionalLoad_Params with a gesture value of NavigationGestureAuto

Expected:
The user gesture should be consistent between WillStartRequest and DidCommitNavigation
 
The root cause seems to be:

1. In FrameLoader::load, we create a ResourceRequest and annotate it as having a user gesture:

#1 0x7f8c797284c0 blink::ResourceRequest::setHasUserGesture()
#2 0x7f8c718bd3a6 blink::FrameLoader::load()
#3 0x7f8c7148390d blink::HTMLAnchorElement::handleClick()

This ResourceRequest is sent to the browser process via ResourceHostMsg_RequestResource, which triggers an invocation of WillStartRequest with has_user_gesture set to true

2. Later in FrameLoader::load, blink::createWindowForRequest is invoked, which eventually consumes the current user gesture:

#1 0x7f8c795d0966 blink::UserGestureToken::consumeGesture()
#2 0x7f8c795d0f36 blink::UserGestureIndicator::consumeUserGesture()
#3 0x7f8c7d4eb6cc content::RenderViewImpl::createView()
#4 0x7f8c7933070a blink::ChromeClientImpl::createWindow()
#5 0x7f8c718f1c7a blink::createWindowHelper()
#6 0x7f8c718f223b blink::createWindowForRequest()
#7 0x7f8c718bda81 blink::FrameLoader::load()

3. Later during createWindowForRequest, we eventually cause an invocation of RenderFrameImpl::didStartProvisionalLoad, which stores the user gesture value on RenderViewImpl:

#1 0x7f8c7d4ba08d content::RenderViewImpl::set_navigation_gesture()
#2 0x7f8c7d4b9a7e content::RenderFrameImpl::didStartProvisionalLoad()
#3 0x7f8c7934e3c8 blink::FrameLoaderClientImpl::dispatchDidStartProvisionalLoad()
#4 0x7f8c718c155e blink::FrameLoader::startLoad()
#5 0x7f8c718bd8f5 blink::FrameLoader::load()
#6 0x7f8c718f23a1 blink::createWindowForRequest()
#7 0x7f8c718bda81 blink::FrameLoader::load()

however, since the user gesture was already consumed earlier, the value set on RenderViewImpl is incorrect, indicating that the navigation was not started via user gesture

4. When the navigation commits, RenderFrameImpl sends a FrameHostMsg_DidCommitProvisionalLoad, with the user gesture value stored earlier in step 3.

5. The FrameHostMsg_DidCommitProvisionalLoad eventually triggers an invocation of NavigationHandleImpl::DidCommitNavigation, and the navigation handle's user gesture value is updated to false, which is an incorrect value, after having been correctly set to true in step 1.

It seems like user gesture tracking is very fragile and broken in various ways due to the subtle ordering dependencies between blink and content. This would probably benefit from someone diving in and owning the end to end gesture tracking logic across blink and content.
Cc: clamy@chromium.org nasko@chromium.org
Labels: UserActivation
Cc: mustaq@chromium.org

Sign in to add a comment