Make ScriptBadgeController record when extensions run scripts on pages yet to

chrome/browser/extensions/script_badge_controller_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <string>
 
 #include "base/command_line.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "base/stringprintf.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/script_badge_controller.h"
 #include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/extensions/test_extension_system.h"
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
 #include "chrome/browser/ui/tab_contents/test_tab_contents.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_builder.h"
 #include "chrome/common/extensions/value_builder.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_registrar.h"
 #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;
 
 namespace extensions {
 namespace {
 
 class ScriptBadgeControllerTest : public TabContentsTestHarness {
  public:
   ScriptBadgeControllerTest()
@@ skipping 15 matching lines @@
     CommandLine command_line(CommandLine::NO_PROGRAM);
     extension_service_ = extension_system->CreateExtensionService(
         &command_line, FilePath(), false);
 
     script_executor_.reset(new ScriptExecutor(web_contents()));
     script_badge_controller_.reset(new ScriptBadgeController(
         tab_contents(), script_executor_.get()));
   }
 
  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()
                 .Append("tabs"))
             .Set("page_action", DictionaryBuilder()
                 .Set("default_title", "Hello")))
         .Build();
     extension_service_->AddExtension(extension);
     return extension;
   }
 
   ExtensionService* extension_service_;
   scoped_ptr<ScriptExecutor> script_executor_;
   scoped_ptr<ScriptBadgeController> script_badge_controller_;
 
  private:
   content::TestBrowserThread ui_thread_;
   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++;
-  }
-
-  int events;
-};
-
-TEST_F(ScriptBadgeControllerTest, ExecutionMakesBadgeVisible) {
-  content::NotificationRegistrar notification_registrar;
+  scoped_refptr<const Extension> extension = AddTestExtension();
 
   EXPECT_THAT(script_badge_controller_->GetCurrentActions(),
               testing::ElementsAre());
 
-  scoped_refptr<const Extension> extension = AddTestExtension();
-
   // Establish a page id.
   NavigateAndCommit(GURL("http://www.google.com"));
 
-  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.
   EXPECT_THAT(script_badge_controller_->GetCurrentActions(),
               testing::ElementsAre());
 
-  ListValue val;
-  script_badge_controller_->OnExecuteScriptFinished(
-      extension->id(), true,
-      tab_contents()->web_contents()->GetController().GetActiveEntry()->
-      GetPageID(),
-      "",
-      val);
+  // Extension calls executeScript(), should be a script badge for that.
+  {
+    ListValue val;
+    script_badge_controller_->OnExecuteScriptFinished(extension->id(),
+                                                      true,
+                                                      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));
+
+
+  // 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"));
+
+  EXPECT_THAT(script_badge_controller_->GetCurrentActions(),
+              testing::ElementsAre(extension->script_badge()));
+  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.
   NavigateAndCommit(GURL("http://www.google.com"));
 
   // 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;
+  content::TestNotificationTracker notification_tracker;
+  notification_tracker.ListenFor(
+      chrome::NOTIFICATION_EXTENSION_LOCATION_BAR_UPDATED,
+      content::Source<Profile>(tab_contents()->profile()));
 
-  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);
+  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());
 
   // Getting attention the first time should display the badge.
   script_badge_controller_->GetAttentionFor(extension->id());
 
   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