UserGestureIndicator is a mess. Clean it up.

third_party/WebKit/Source/platform/UserGestureIndicatorTest.cpp

Index: third_party/WebKit/Source/platform/UserGestureIndicatorTest.cpp
diff --git a/third_party/WebKit/Source/platform/UserGestureIndicatorTest.cpp b/third_party/WebKit/Source/platform/UserGestureIndicatorTest.cpp
index f60284c7010e9e8cf91fa21724fd9bc177f68635..5fb0e9d32a0a455c1a649e61304719ddb808e887 100644
--- a/third_party/WebKit/Source/platform/UserGestureIndicatorTest.cpp
+++ b/third_party/WebKit/Source/platform/UserGestureIndicatorTest.cpp
@@ -18,7 +18,8 @@ TEST(UserGestureIndicatorTest, InitialState) {
 
 TEST(UserGestureIndicatorTest, ConstructedWithNewUserGesture) {
   UserGestureIndicator::clearProcessedUserGestureSinceLoad();
-  UserGestureIndicator userGestureScope(DefinitelyProcessingNewUserGesture);
+  UserGestureIndicator userGestureScope(
+      UserGestureToken::create(UserGestureToken::NewGesture));
 
   EXPECT_TRUE(UserGestureIndicator::utilizeUserGesture());
   EXPECT_TRUE(UserGestureIndicator::processedUserGestureSinceLoad());
@@ -29,7 +30,7 @@ TEST(UserGestureIndicatorTest, ConstructedWithNewUserGesture) {
 
 TEST(UserGestureIndicatorTest, ConstructedWithUserGesture) {
   UserGestureIndicator::clearProcessedUserGestureSinceLoad();
-  UserGestureIndicator userGestureScope(DefinitelyProcessingUserGesture);
+  UserGestureIndicator userGestureScope(UserGestureToken::create());
 
   EXPECT_TRUE(UserGestureIndicator::utilizeUserGesture());
   EXPECT_TRUE(UserGestureIndicator::processedUserGestureSinceLoad());
@@ -40,11 +41,11 @@ TEST(UserGestureIndicatorTest, ConstructedWithUserGesture) {
 
 TEST(UserGestureIndicatorTest, ConstructedWithNoUserGesture) {
   UserGestureIndicator::clearProcessedUserGestureSinceLoad();
-  UserGestureIndicator userGestureScope(DefinitelyNotProcessingUserGesture);
+  UserGestureIndicator userGestureScope(nullptr);
 
   EXPECT_FALSE(UserGestureIndicator::utilizeUserGesture());
   EXPECT_FALSE(UserGestureIndicator::processedUserGestureSinceLoad());
-  EXPECT_NE(nullptr, UserGestureIndicator::currentToken());
+  EXPECT_EQ(nullptr, UserGestureIndicator::currentToken());
 
   EXPECT_FALSE(UserGestureIndicator::consumeUserGesture());
 }
@@ -52,7 +53,7 @@ TEST(UserGestureIndicatorTest, ConstructedWithNoUserGesture) {
 // Check that after UserGestureIndicator destruction state will be cleared.
 TEST(UserGestureIndicatorTest, DestructUserGestureIndicator) {
   {
-    UserGestureIndicator userGestureScope(DefinitelyProcessingUserGesture);
+    UserGestureIndicator userGestureScope(UserGestureToken::create());
 
     EXPECT_TRUE(UserGestureIndicator::utilizeUserGesture());
     EXPECT_TRUE(UserGestureIndicator::processedUserGestureSinceLoad());
@@ -67,7 +68,8 @@ TEST(UserGestureIndicatorTest, DestructUserGestureIndicator) {
 // Tests creation of scoped UserGestureIndicator objects.
 TEST(UserGestureIndicatorTest, ScopedNewUserGestureIndicators) {
   // Root GestureIndicator and GestureToken.
-  UserGestureIndicator userGestureScope(DefinitelyProcessingNewUserGesture);
+  UserGestureIndicator userGestureScope(
+      UserGestureToken::create(UserGestureToken::NewGesture));
 
   EXPECT_TRUE(UserGestureIndicator::utilizeUserGesture());
   EXPECT_TRUE(UserGestureIndicator::processedUserGestureSinceLoad());
@@ -75,7 +77,8 @@ TEST(UserGestureIndicatorTest, ScopedNewUserGestureIndicators) {
   {
     // Construct inner UserGestureIndicator.
     // It should share GestureToken with the root indicator.
-    UserGestureIndicator innerUserGesture(DefinitelyProcessingNewUserGesture);
+    UserGestureIndicator innerUserGesture(
+        UserGestureToken::create(UserGestureToken::NewGesture));
 
     EXPECT_TRUE(UserGestureIndicator::utilizeUserGesture());
     EXPECT_NE(nullptr, UserGestureIndicator::currentToken());
@@ -114,7 +117,9 @@ TEST(UserGestureIndicatorTest, Callback) {
   UsedCallback cb;
 
   {
-    UserGestureIndicator userGestureScope(DefinitelyProcessingUserGesture, &cb);
+    RefPtr<UserGestureToken> token = UserGestureToken::create();
+    token->setUserGestureUtilizedCallback(&cb);
+    UserGestureIndicator userGestureScope(token);
     EXPECT_EQ(0u, cb.getAndResetUsedCount());
 
     // Untracked doesn't invoke the callback
@@ -133,7 +138,9 @@ TEST(UserGestureIndicatorTest, Callback) {
   EXPECT_EQ(0u, cb.getAndResetUsedCount());
 
   {
-    UserGestureIndicator userGestureScope(DefinitelyProcessingUserGesture, &cb);
+    RefPtr<UserGestureToken> token = UserGestureToken::create();
+    token->setUserGestureUtilizedCallback(&cb);
+    UserGestureIndicator userGestureScope(token);
 
     // Consume also invokes the callback
     EXPECT_TRUE(UserGestureIndicator::consumeUserGesture());
@@ -145,15 +152,6 @@ TEST(UserGestureIndicatorTest, Callback) {
     EXPECT_EQ(0u, cb.getAndResetUsedCount());
   }
 
-  {
-    UserGestureIndicator userGestureScope(DefinitelyNotProcessingUserGesture,
-                                          &cb);
-
-    // Callback not invoked when there isn't actually a user gesture
-    EXPECT_FALSE(UserGestureIndicator::processingUserGesture());
-    EXPECT_EQ(0u, cb.getAndResetUsedCount());
-  }
-
   // The callback isn't invoked outside the scope of the UGI

[Rick Byers, 2016/10/11 18:00:19]

With the separation from the token, this is probably the test which should
validate the callback lifetime semantics.  I probably should have written this
test using a nested UGI, then it would have failed as a result of this
refactoring.

[Nate Chapin, 2016/10/11 19:17:55]

Done (with a unique_ptr instead of with nested scopes).

   EXPECT_FALSE(UserGestureIndicator::utilizeUserGesture());
   EXPECT_EQ(0u, cb.getAndResetUsedCount());