Stored UserGestureTokens seem either unnecessary or unpredictable |
|||||
Issue descriptionWe 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!
,
Aug 3 2017
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
,
Sep 28 2017
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?
,
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)
,
Nov 1 2017
,
May 8 2018
,
Nov 6
Don't want to waste time on v1 tokens which will become redundant with UAv2. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by mustaq@chromium.org
, Aug 3 2017