UserGestureIndicator: remove many unnecessary calls to isMainThread

Most code that calls the UserGestureIndicator knows which thread it is
executing in. Calling isMainThread involves doing a thread id syscall which
can be quite slow, and there are places in the loading code that do this
in a tight loop (e.g. preloading resources).

This patch adds an optional argument to the indicator's static methods which
optionally allow callers to get the gesture state in a thread-safe way.

BUG=348655

Review-Url: https://codereview.chromium.org/2555013004
Cr-Commit-Position: refs/heads/master@{#437941}

extensions/renderer/request_sender.cc

@@ -102,7 +102,7 @@ bool RequestSender::StartRequest(Source* source,
   params.request_id = request_id;
   params.has_callback = has_callback;
   params.user_gesture =
-      blink::WebUserGestureIndicator::isProcessingUserGesture();
+      blink::WebUserGestureIndicator::isProcessingUserGestureThreadSafe();
 
   // Set Service Worker specific params to default values.
   params.worker_thread_id = -1;

third_party/WebKit/Source/core/frame/DOMTimer.cpp

@@ -47,7 +47,7 @@ static const double oneMillisecond = 0.001;
 static const double minimumInterval = 0.004;
 
 static inline bool shouldForwardUserGesture(int interval, int nestingLevel) {
-  return UserGestureIndicator::processingUserGesture() &&
+  return UserGestureIndicator::processingUserGestureThreadSafe() &&
          interval <= maxIntervalForUserGestureForwarding &&
          nestingLevel ==
              1;  // Gestures should not be forwarded to nested timers.
@@ -90,8 +90,11 @@ DOMTimer::DOMTimer(ExecutionContext* context,
       m_nestingLevel(context->timers()->timerNestingLevel() + 1),
       m_action(action) {
   ASSERT(timeoutID > 0);
-  if (shouldForwardUserGesture(interval, m_nestingLevel))
+  if (shouldForwardUserGesture(interval, m_nestingLevel)) {
+    // Thread safe because shouldForwardUserGesture will only return true if
+    // execution is on the the main thread.
     m_userGestureToken = UserGestureIndicator::currentToken();
+  }
 
   InspectorInstrumentation::asyncTaskScheduled(
       context, singleShot ? "setTimeout" : "setInterval", this, !singleShot);

third_party/WebKit/Source/modules/permissions/Permissions.cpp

@@ -156,7 +156,7 @@ ScriptPromise Permissions::request(ScriptState* scriptState,
   service->RequestPermission(
       std::move(descriptor),
       scriptState->getExecutionContext()->getSecurityOrigin(),
-      UserGestureIndicator::processingUserGesture(),
+      UserGestureIndicator::processingUserGestureThreadSafe(),
       convertToBaseCallback(WTF::bind(
           &Permissions::taskComplete, wrapPersistent(this),
           wrapPersistent(resolver), WTF::passed(std::move(descriptorCopy)))));
@@ -247,7 +247,7 @@ ScriptPromise Permissions::requestAll(
   service->RequestPermissions(
       std::move(internalPermissions),
       scriptState->getExecutionContext()->getSecurityOrigin(),
-      UserGestureIndicator::processingUserGesture(),
+      UserGestureIndicator::processingUserGestureThreadSafe(),
       convertToBaseCallback(
           WTF::bind(&Permissions::batchTaskComplete, wrapPersistent(this),
                     wrapPersistent(resolver),

third_party/WebKit/Source/platform/UserGestureIndicator.cpp

@@ -42,7 +42,7 @@ UserGestureToken::UserGestureToken(Status status)
       m_timestamp(WTF::currentTime()),
       m_timeoutPolicy(Default),
       m_usageCallback(nullptr) {
-  if (status == NewGesture || !UserGestureIndicator::currentToken())
+  if (status == NewGesture || !UserGestureIndicator::currentTokenThreadSafe())
     m_consumableGestures++;
 }
 
@@ -151,18 +151,18 @@ bool UserGestureIndicator::utilizeUserGesture() {
 }
 
 bool UserGestureIndicator::processingUserGesture() {
-  if (auto* token = currentToken()) {
-    ASSERT(isMainThread());
+  if (auto* token = currentToken())
     return token->hasGestures();
-  }
-
   return false;
 }
 
+bool UserGestureIndicator::processingUserGestureThreadSafe() {
+  return isMainThread() && processingUserGesture();
+}
+
 // static
 bool UserGestureIndicator::consumeUserGesture() {
   if (auto* token = currentToken()) {
-    ASSERT(isMainThread());
     if (token->consumeGesture()) {
       token->userGestureUtilized();
       return true;
@@ -171,11 +171,17 @@ bool UserGestureIndicator::consumeUserGesture() {
   return false;
 }
 
-// static
+bool UserGestureIndicator::consumeUserGestureThreadSafe() {
+  return isMainThread() && consumeUserGesture();
+}
+
 UserGestureToken* UserGestureIndicator::currentToken() {
-  if (!isMainThread() || !s_rootToken)
-    return nullptr;
+  DCHECK(isMainThread());
   return s_rootToken;
 }
 
+UserGestureToken* UserGestureIndicator::currentTokenThreadSafe() {
+  return isMainThread() ? currentToken() : nullptr;
+}
+
 }  // namespace blink

third_party/WebKit/Source/platform/UserGestureIndicator.h

@@ -94,11 +94,17 @@ class PLATFORM_EXPORT UserGestureIndicator final {
   WTF_MAKE_NONCOPYABLE(UserGestureIndicator);
 
  public:
+  // Note: All *ThreadSafe methods are safe to call from any thread. Their
+  // non-suffixed counterparts *must* be called on the main thread. Consider
+  // always using the non-suffixed one unless the code really
+  // needs to be thread-safe
+
   // Returns whether a user gesture is currently in progress.
   // Does not invoke the UserGestureUtilizedCallback.  Consider calling
   // utilizeUserGesture instead if you know for sure that the return value
   // will have an effect.
   static bool processingUserGesture();
+  static bool processingUserGestureThreadSafe();
 
   // Indicates that a user gesture (if any) is being used, without preventing it
   // from being used again.  Returns whether a user gesture is currently in
@@ -111,8 +117,10 @@ class PLATFORM_EXPORT UserGestureIndicator final {
   // operations like creating a new process.
   // Like utilizeUserGesture, may invoke/clear any UserGestureUtilizedCallback.
   static bool consumeUserGesture();
+  static bool consumeUserGestureThreadSafe();
 
   static UserGestureToken* currentToken();
+  static UserGestureToken* currentTokenThreadSafe();
 
   explicit UserGestureIndicator(PassRefPtr<UserGestureToken>);
   ~UserGestureIndicator();

third_party/WebKit/Source/web/WebUserGestureIndicator.cpp

@@ -40,8 +40,15 @@ bool WebUserGestureIndicator::isProcessingUserGesture() {
   return UserGestureIndicator::processingUserGesture();
 }
 
+bool WebUserGestureIndicator::isProcessingUserGestureThreadSafe() {
+  return UserGestureIndicator::processingUserGestureThreadSafe();
+}
+
+// TODO(csharrison): consumeUserGesture() and currentUserGestureToken() use
+// the thread-safe API, which many callers probably do not need. Consider
+// updating them if they are in any sort of critical path or called often.
 bool WebUserGestureIndicator::consumeUserGesture() {
-  return UserGestureIndicator::consumeUserGesture();
+  return UserGestureIndicator::consumeUserGestureThreadSafe();
 }
 
 bool WebUserGestureIndicator::processedUserGestureSinceLoad(
@@ -51,7 +58,7 @@ bool WebUserGestureIndicator::processedUserGestureSinceLoad(
 }
 
 WebUserGestureToken WebUserGestureIndicator::currentUserGestureToken() {
-  return WebUserGestureToken(UserGestureIndicator::currentToken());
+  return WebUserGestureToken(UserGestureIndicator::currentTokenThreadSafe());
 }
 
 }  // namespace blink

third_party/WebKit/public/web/WebUserGestureIndicator.h

@@ -40,8 +40,13 @@ class WebUserGestureToken;
 
 class WebUserGestureIndicator {
  public:
-  // Returns true if a user gesture is currently being processed.
+  // Returns true if a user gesture is currently being processed. Must be called
+  // on the main thread.
   BLINK_EXPORT static bool isProcessingUserGesture();
+  // Can be called from any thread. Note that this is slower than the non
+  // thread-safe version due to thread id lookups. Prefer the non thread-safe
+  // version for code that will only execute on the main thread.
+  BLINK_EXPORT static bool isProcessingUserGestureThreadSafe();
 
   // Returns true if a consumable gesture exists and has been successfully
   // consumed.