New issue
Advanced search Search tips

Issue 752158 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner:
Closed: Nov 6
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug

Blocking:
issue 696617



Sign in to add a comment

Stored UserGestureTokens seem either unnecessary or unpredictable

Project Member Reported by mustaq@chromium.org, Aug 3 2017

Issue description

We have ~20 cases in our code where current UserGestureToken is saved at one point in time, and then later used in an UserGestureIndicator.  
This created a two-level state maintenance mechanism without a clear benefit as explained below.

But more importantly, this unnecessary complexity makes it very hard (if not impossible) to add an alternate implementation (for the simple model, Issue 696617) such that we can switch between them for testing.

[Why saving a token locally is either unnecessary or unpredictable]

UGIs maintain a single effective state through a static root_token which is updated through the UGI ctor as follows:
  [A] if the passed token == root_token (incl. both nulls):
      root_token remains unchanged
  [B] else (the passed token != root_token):
      root_token gets a fresh start (new gesture, new expiry deadline)

Now saved tokens have two usage patterns:
- For mouse events, a UGI is created before saving the root_token at mousedown, then the UGI gets out of scope.  The saved token is then reused to create a new UGI at mouseup; [B] implies this is like using a brand new token.  This is done in EventHandler, WebViewImpl, WebFrameWidget{Impl,Base}.

- The rest of the use-cases (DOMTimer, PostMessageTimer in LocalDOMWindow, ScheduledNavigation, SuspedableScriptExecutor) saves current root_token w/o creating one (so could be null), then reuses the saved token later on to create a new UGI.  The outcome depends on what happened to the old UGI in between: if it was destructed (with or w/o consumption), the new UGI is like a new gesture [B]; otherwise the new token is ignored [A].  This seems unpredictable.

A simple solution: don't save token, but instead save the active gesture state (if there is any unconsumed gesture), and use the state to create a UGI later on.

So we won't need any UGT!
 
Description: Show this description
Another option seems to be not even storing tokens (nor the active gesture state), because UMA stats suggests that:
- In 99% of the cases, the passed UGT is useless:
  https://uma.googleplex.com/p/chrome/timeline_v2?sid=1940c21ad1561cfea5b7a88ee651e2df
- Token merging affects only ~0.013% of page visits:
  https://uma.googleplex.com/p/chrome/timeline_v2?sid=c5f90682ad7a8cc55b5c2296c47e1af6


Comment 3 by mustaq@chromium.org, Sep 28 2017

Cc: jochen@chromium.org
Looks like my comment "w/o any clear benefit" in the original post is not quite correct :(

But the original question remains the same: given the UMA stats, can't we just save the boolean state "have_an_unconsumed_gesture" instead of UserGestureTokens?

I guess the answer doesn't depend on whether or not we remove mousedown gesture (Issue 769796), right?

Comment 4 by jochen@chromium.org, Sep 29 2017

I would be interested in learning in what cases we merge tokens. Can we maybe add some code that records the creation stack trace in the token, and uploads a minidump if we merge tokens (there's base::debug::DumpWithoutCrashing, I'd use base::debug::StackTrace to record the creation stacktrace, and then put it on the stack using alloca before invoking DumpWithoutCrashing)
Labels: UserActivation

Comment 6 by ojan@chromium.org, May 8 2018

Cc: -ojan@chromium.org
Status: WontFix (was: Assigned)
Don't want to waste time on v1 tokens which will become redundant with UAv2.

Sign in to add a comment