Make ScriptBadgeController record when extensions run scripts on pages yet to

chrome/browser/extensions/script_badge_controller_unittest.cc

Index: chrome/browser/extensions/script_badge_controller_unittest.cc
diff --git a/chrome/browser/extensions/script_badge_controller_unittest.cc b/chrome/browser/extensions/script_badge_controller_unittest.cc
index 2e53cd72f0efb2b16a7a6a2c786fd17ef0f039f1..53a151f74bebd7001ab0185397c70e45252d317e 100644
--- a/chrome/browser/extensions/script_badge_controller_unittest.cc
+++ b/chrome/browser/extensions/script_badge_controller_unittest.cc
@@ -25,6 +25,7 @@
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/test_browser_thread.h"
+#include "content/public/test/test_notification_tracker.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
 using content::BrowserThread;
@@ -60,11 +61,25 @@ class ScriptBadgeControllerTest : public TabContentsTestHarness {
   }
 
  protected:
+  // Gets the current page ID, or -1 if there isn't one yet.
+  int32 GetPageID() {
+    content::NavigationEntry* active_entry =
+        tab_contents()->web_contents()->GetController().GetActiveEntry();
+    return active_entry ? active_entry->GetPageID() : -1;
+  }
+
   // Creates a test extension and adds it to |extension_service_|.
   scoped_refptr<const Extension> AddTestExtension() {
+    return AddTestExtension("");
+  }
+
+  // Creates a test extension with a specific ID and adds it to
+  // |extension_service_|.
+  scoped_refptr<const Extension> AddTestExtension(const std::string& id) {
     scoped_refptr<const Extension> extension = ExtensionBuilder()
+        .SetID(id)
         .SetManifest(DictionaryBuilder()
-            .Set("name", "Extension with page action")
+            .Set("name", "Test extension")
             .Set("version", "1.0.0")
             .Set("manifest_version", 2)
             .Set("permissions", ListBuilder()
@@ -85,52 +100,77 @@ class ScriptBadgeControllerTest : public TabContentsTestHarness {
   content::TestBrowserThread file_thread_;
 };
 
-struct CountingNotificationObserver : public content::NotificationObserver {
-  CountingNotificationObserver() : events(0) {}
+TEST_F(ScriptBadgeControllerTest, ExecutionMakesBadgeVisible) {
+  content::TestNotificationTracker notification_tracker;
+  notification_tracker.ListenFor(

[Jeffrey Yasskin, 2012/07/25 22:22:39]

Hey cool, that's better than my class. :)

[not at google - send to devlin, 2012/07/26 08:52:46]

:) a lucky find. I was looking for where yours was actually, didn't realise it
was in here, found that one instead.

+      chrome::NOTIFICATION_EXTENSION_LOCATION_BAR_UPDATED,
+      content::Source<Profile>(tab_contents()->profile()));
 
-  virtual void Observe(int type,
-                       const content::NotificationSource& source,
-                       const content::NotificationDetails& details) OVERRIDE {
-    events++;
-  }
+  scoped_refptr<const Extension> extension = AddTestExtension();
 
-  int events;
-};
+  EXPECT_THAT(script_badge_controller_->GetCurrentActions(),
+              testing::ElementsAre());
 
-TEST_F(ScriptBadgeControllerTest, ExecutionMakesBadgeVisible) {
-  content::NotificationRegistrar notification_registrar;
+  // Establish a page id.
+  NavigateAndCommit(GURL("http://www.google.com"));
 
+  // Initially, no script badges.
   EXPECT_THAT(script_badge_controller_->GetCurrentActions(),
               testing::ElementsAre());
 
-  scoped_refptr<const Extension> extension = AddTestExtension();
+  // Extension calls executeScript(), should be a script badge for that.
+  {
+    ListValue val;
+    script_badge_controller_->OnExecuteScriptFinished(extension->id(),
+                                                      true,
+                                                      GetPageID(),
+                                                      "",
+                                                      val);
+  }
 
-  // Establish a page id.
-  NavigateAndCommit(GURL("http://www.google.com"));
+  EXPECT_THAT(script_badge_controller_->GetCurrentActions(),
+              testing::ElementsAre(extension->script_badge()));
+  EXPECT_TRUE(notification_tracker.Check1AndReset(
+        chrome::NOTIFICATION_EXTENSION_LOCATION_BAR_UPDATED));
 
-  CountingNotificationObserver location_bar_updated;
-  notification_registrar.Add(
-      &location_bar_updated,
-      chrome::NOTIFICATION_EXTENSION_LOCATION_BAR_UPDATED,
-      content::Source<Profile>(tab_contents()->profile()));
 
-  // Initially, no script badges.
+  // Should clear on navigate.
+  NavigateAndCommit(GURL("http://www.chromium.org"));
+
   EXPECT_THAT(script_badge_controller_->GetCurrentActions(),
               testing::ElementsAre());
+  EXPECT_EQ(0u, notification_tracker.size());
+
+  // Extension calls executeScript() on a page that the browser doesn't

[Jeffrey Yasskin, 2012/07/25 22:22:39]

This seems redundant with the ContentScripts test. Can you either extract the
common code inside ScriptBadgeController so you only need to test it once, or
refactor the tests so that you can run the same code twice with different
notification callbacks?

+  // know about yet.
+  {
+    ListValue val;
+    script_badge_controller_->OnExecuteScriptFinished(extension->id(),
+                                                      true,
+                                                      GetPageID() + 1,
+                                                      "",
+                                                      val);
+  }
+
+  EXPECT_THAT(script_badge_controller_->GetCurrentActions(),
+              testing::ElementsAre());
+  EXPECT_EQ(0u, notification_tracker.size());
+
+  // On navigating to that page, should run script badges then.
+  NavigateAndCommit(GURL("http://www.google.com"));
 
-  ListValue val;
-  script_badge_controller_->OnExecuteScriptFinished(
-      extension->id(), true,
-      tab_contents()->web_contents()->GetController().GetActiveEntry()->
-      GetPageID(),
-      "",
-      val);
   EXPECT_THAT(script_badge_controller_->GetCurrentActions(),
               testing::ElementsAre(extension->script_badge()));
-  EXPECT_THAT(location_bar_updated.events, testing::Gt(0));
+  EXPECT_TRUE(notification_tracker.Check1AndReset(
+        chrome::NOTIFICATION_EXTENSION_LOCATION_BAR_UPDATED));
 };
 
 TEST_F(ScriptBadgeControllerTest, FragmentNavigation) {
+  content::TestNotificationTracker notification_tracker;
+  notification_tracker.ListenFor(
+      chrome::NOTIFICATION_EXTENSION_LOCATION_BAR_UPDATED,
+      content::Source<Profile>(tab_contents()->profile()));
+
   scoped_refptr<const Extension> extension = AddTestExtension();
 
   // Establish a page id.
@@ -138,82 +178,49 @@ TEST_F(ScriptBadgeControllerTest, FragmentNavigation) {
 
   // Run script. Should be a notification and a script badge.
   {
-    content::NotificationRegistrar notification_registrar;
-    CountingNotificationObserver location_bar_updated;
-    notification_registrar.Add(
-        &location_bar_updated,
-        chrome::NOTIFICATION_EXTENSION_LOCATION_BAR_UPDATED,
-        content::Source<Profile>(tab_contents()->profile()));
-
     ListValue val;
-    script_badge_controller_->OnExecuteScriptFinished(
-        extension->id(), true,
-        tab_contents()->web_contents()->GetController().GetActiveEntry()->
-            GetPageID(),
-        "",
-        val);
+    script_badge_controller_->OnExecuteScriptFinished(extension->id(),
+                                                      true,
+                                                      GetPageID(),
+                                                      "",
+                                                      val);
 
     EXPECT_THAT(script_badge_controller_->GetCurrentActions(),
                 testing::ElementsAre(extension->script_badge()));
-    EXPECT_EQ(1, location_bar_updated.events);
+    EXPECT_TRUE(notification_tracker.Check1AndReset(
+        chrome::NOTIFICATION_EXTENSION_LOCATION_BAR_UPDATED));
   }
 
   // Navigate to a hash fragment. Shouldn't change.
   {
-    content::NotificationRegistrar notification_registrar;
-    CountingNotificationObserver location_bar_updated;
-    notification_registrar.Add(
-        &location_bar_updated,
-        chrome::NOTIFICATION_EXTENSION_LOCATION_BAR_UPDATED,
-        content::Source<Profile>(tab_contents()->profile()));
-
     NavigateAndCommit(GURL("http://www.google.com#hash"));
 
     EXPECT_THAT(script_badge_controller_->GetCurrentActions(),
                 testing::ElementsAre(extension->script_badge()));
-    EXPECT_EQ(0, location_bar_updated.events);
+    EXPECT_EQ(0u, notification_tracker.size());
   }
 
   // Refreshing the page should reset the badges.
   {
-    content::NotificationRegistrar notification_registrar;
-    CountingNotificationObserver location_bar_updated;
-    notification_registrar.Add(
-        &location_bar_updated,
-        chrome::NOTIFICATION_EXTENSION_LOCATION_BAR_UPDATED,
-        content::Source<Profile>(tab_contents()->profile()));
-
     Reload();
 
     EXPECT_THAT(script_badge_controller_->GetCurrentActions(),
                 testing::ElementsAre());
-    EXPECT_EQ(0, location_bar_updated.events);
+    EXPECT_EQ(0u, notification_tracker.size());
   }
 }
 
 TEST_F(ScriptBadgeControllerTest, GetAttentionMakesBadgeVisible) {
-  content::NotificationRegistrar notification_registrar;
-
-  scoped_refptr<const Extension> extension =
-      ExtensionBuilder()
-      .SetManifest(DictionaryBuilder()
-                   .Set("name", "Extension")

[Jeffrey Yasskin, 2012/07/25 22:22:39]

I'm not a fan of moving all of the extension creations into a common function.
Doing that makes it unclear which attributes of the single manifest are relevant
to which test.

-                   .Set("version", "1.0.0")
-                   .Set("manifest_version", 2)
-                   .Set("permissions", ListBuilder()
-                        .Append("tabs")))
-      .Build();
-  extension_service_->AddExtension(extension);
+  content::TestNotificationTracker notification_tracker;
+  notification_tracker.ListenFor(
+      chrome::NOTIFICATION_EXTENSION_LOCATION_BAR_UPDATED,
+      content::Source<Profile>(tab_contents()->profile()));
+
+  scoped_refptr<const Extension> extension = AddTestExtension();
 
   // Establish a page id.
   NavigateAndCommit(GURL("http://www.google.com"));
 
-  CountingNotificationObserver initial_badge_display;
-  notification_registrar.Add(
-      &initial_badge_display,
-      chrome::NOTIFICATION_EXTENSION_LOCATION_BAR_UPDATED,
-      content::Source<Profile>(tab_contents()->profile()));
-
   // Initially, no script badges.
   EXPECT_THAT(script_badge_controller_->GetCurrentActions(),
               testing::ElementsAre());
@@ -223,21 +230,89 @@ TEST_F(ScriptBadgeControllerTest, GetAttentionMakesBadgeVisible) {
 
   EXPECT_THAT(script_badge_controller_->GetCurrentActions(),
               testing::ElementsAre(extension->script_badge()));
-  EXPECT_THAT(initial_badge_display.events, testing::Gt(0));
-
-  CountingNotificationObserver subsequent_get_attention_call;
-  notification_registrar.Add(
-      &subsequent_get_attention_call,
-      chrome::NOTIFICATION_EXTENSION_LOCATION_BAR_UPDATED,
-      content::Source<Profile>(tab_contents()->profile()));
+  EXPECT_TRUE(notification_tracker.Check1AndReset(
+      chrome::NOTIFICATION_EXTENSION_LOCATION_BAR_UPDATED));
 
   // Getting attention a second time should have no effect.
   script_badge_controller_->GetAttentionFor(extension->id());
 
   EXPECT_THAT(script_badge_controller_->GetCurrentActions(),
               testing::ElementsAre(extension->script_badge()));
-  EXPECT_EQ(0, subsequent_get_attention_call.events);
+  EXPECT_EQ(0u, notification_tracker.size());
 };
 
+TEST_F(ScriptBadgeControllerTest, ContentScripts) {
+  content::TestNotificationTracker notification_tracker;
+  notification_tracker.ListenFor(
+      chrome::NOTIFICATION_EXTENSION_LOCATION_BAR_UPDATED,
+      content::Source<Profile>(tab_contents()->profile()));
+
+  scoped_refptr<const Extension> extension_a =
+      AddTestExtension("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
+  scoped_refptr<const Extension> extension_b =
+      AddTestExtension("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb");
+  scoped_refptr<const Extension> extension_c =
+      AddTestExtension("cccccccccccccccccccccccccccccccc");
+
+  // Sometimes notifications for content scripts appear earlier than the page
+  // load has happened in the browser.
+  {
+    std::set<std::string> ids;
+    ids.insert(extension_a->id());
+
+    script_badge_controller_->OnContentScriptsExecuting(ids, 0);
+    EXPECT_THAT(script_badge_controller_->GetCurrentActions(),
+                testing::ElementsAre());
+    EXPECT_EQ(0u, notification_tracker.size());
+  }
+
+  // Establish a page id.
+  NavigateAndCommit(GURL("http://www.google.com"));
+
+  EXPECT_THAT(script_badge_controller_->GetCurrentActions(),
+              testing::ElementsAre(extension_a->script_badge()));
+  EXPECT_TRUE(notification_tracker.Check1AndReset(
+      chrome::NOTIFICATION_EXTENSION_LOCATION_BAR_UPDATED));
+
+  // Next page should clear the content-script script badge.
+  NavigateAndCommit(GURL("http://www.chromium.org"));
+
+  EXPECT_THAT(script_badge_controller_->GetCurrentActions(),
+              testing::ElementsAre());
+
+  // Executing a content script on the current page should work.
+  {
+    std::set<std::string> ids;
+    ids.insert(extension_a->id());
+    ids.insert(extension_b->id());
+    script_badge_controller_->OnContentScriptsExecuting(ids, GetPageID());
+
+    EXPECT_THAT(script_badge_controller_->GetCurrentActions(),
+                testing::ElementsAre(extension_a->script_badge(),
+                                     extension_b->script_badge()));
+    EXPECT_TRUE(notification_tracker.Check1AndReset(
+        chrome::NOTIFICATION_EXTENSION_LOCATION_BAR_UPDATED));
+  }
+
+  // Executing a content script on the next page should show up when that
+  // page is navigated to.
+  {
+    std::set<std::string> ids;
+    ids.insert(extension_c->id());
+    script_badge_controller_->OnContentScriptsExecuting(ids, GetPageID() + 1);
+
+    EXPECT_THAT(script_badge_controller_->GetCurrentActions(),
+                testing::ElementsAre(extension_a->script_badge(),
+                                     extension_b->script_badge()));
+    EXPECT_EQ(0u, notification_tracker.size());
+  }
+
+  NavigateAndCommit(GURL("http://www.google.com"));
+  EXPECT_THAT(script_badge_controller_->GetCurrentActions(),
+              testing::ElementsAre(extension_c->script_badge()));
+  EXPECT_TRUE(notification_tracker.Check1AndReset(
+      chrome::NOTIFICATION_EXTENSION_LOCATION_BAR_UPDATED));
+}
+
 }  // namespace
 }  // namespace extensions