[Extensions Toolbar] Make the ExtensionToolbarModel icon count more stable

chrome/browser/extensions/extension_toolbar_model_unittest.cc

 // Copyright 2014 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 "base/files/file_util.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/extensions/api/extension_action/extension_action_api.h"
@@ skipping 94 matching lines @@
 
 // Creates a new ExtensionToolbarModel for the given |context|.
 KeyedService* BuildToolbarModel(content::BrowserContext* context) {
   return new ExtensionToolbarModel(Profile::FromBrowserContext(context),
                                    ExtensionPrefs::Get(context));
 }
 
 // Given a |profile|, assigns the testing keyed service function to
 // BuildToolbarModel() and uses it to create and initialize a new
 // ExtensionToolbarModel.
-ExtensionToolbarModel* CreateToolbarModelForProfile(Profile* profile) {
+// |wait_for_ready| indicates whether or not to post the ExtensionSystem's
+// ready task.
+ExtensionToolbarModel* CreateToolbarModelForProfile(Profile* profile,
+                                                    bool wait_for_ready) {
   ExtensionToolbarModel* model = ExtensionToolbarModel::Get(profile);
   if (model)
     return model;
 
   // No existing model means it's a new profile (since we, by default, don't
   // create the ToolbarModel in testing).
   ExtensionToolbarModelFactory::GetInstance()->SetTestingFactory(
       profile, &BuildToolbarModel);
   model = ExtensionToolbarModel::Get(profile);
-  // Fake the extension system ready signal.
-  // HACK ALERT! In production, the ready task on ExtensionSystem (and most
-  // everything else on it, too) is shared between incognito and normal
-  // profiles, but a TestExtensionSystem doesn't have the concept of "shared".
-  // Because of this, we have to set any new profile's TestExtensionSystem's
-  // ready task, too.
-  static_cast<TestExtensionSystem*>(ExtensionSystem::Get(profile))->SetReady();
-  // Run tasks posted to TestExtensionSystem.
-  base::RunLoop().RunUntilIdle();
+  if (wait_for_ready) {
+    // Fake the extension system ready signal.
+    // HACK ALERT! In production, the ready task on ExtensionSystem (and most
+    // everything else on it, too) is shared between incognito and normal
+    // profiles, but a TestExtensionSystem doesn't have the concept of "shared".
+    // Because of this, we have to set any new profile's TestExtensionSystem's
+    // ready task, too.
+    static_cast<TestExtensionSystem*>(ExtensionSystem::Get(profile))->
+        SetReady();
+    // Run tasks posted to TestExtensionSystem.
+    base::RunLoop().RunUntilIdle();
+  }
 
   return model;
 }
 
 // Create an extension. If |action_key| is non-NULL, it should point to either
 // kBrowserAction or kPageAction, and the extension will have the associated
 // action.
 scoped_refptr<const Extension> GetActionExtension(const std::string& name,
                                                   const char* action_key) {
   DictionaryBuilder manifest;
@@ skipping 13 matching lines @@
 class ExtensionToolbarModelTestObserver
     : public ExtensionToolbarModel::Observer {
  public:
   explicit ExtensionToolbarModelTestObserver(ExtensionToolbarModel* model);
   ~ExtensionToolbarModelTestObserver() override;
 
   size_t inserted_count() const { return inserted_count_; }
   size_t removed_count() const { return removed_count_; }
   size_t moved_count() const { return moved_count_; }
   int highlight_mode_count() const { return highlight_mode_count_; }
+  size_t initialized_count() const { return initialized_count_; }
   size_t reorder_count() const { return reorder_count_; }
 
  private:
   // ExtensionToolbarModel::Observer:
   void ToolbarExtensionAdded(const Extension* extension, int index) override {
     ++inserted_count_;
   }
 
   void ToolbarExtensionRemoved(const Extension* extension) override {
     ++removed_count_;
@@ skipping 10 matching lines @@
     return false;
   }
 
   void ToolbarVisibleCountChanged() override {}
 
   void ToolbarHighlightModeChanged(bool is_highlighting) override {
     // Add one if highlighting, subtract one if not.
     highlight_mode_count_ += is_highlighting ? 1 : -1;
   }
 
-  void OnToolbarModelInitialized() override {}
+  void OnToolbarModelInitialized() override { ++initialized_count_; }
 
   void OnToolbarReorderNecessary(content::WebContents* web_contents) override {
     ++reorder_count_;
   }
 
   Browser* GetBrowser() override { return NULL; }
 
   ExtensionToolbarModel* model_;
 
   size_t inserted_count_;
   size_t removed_count_;
   size_t moved_count_;
   // Int because it could become negative (if something goes wrong).
   int highlight_mode_count_;
+  size_t initialized_count_;
   size_t reorder_count_;
 };
 
 ExtensionToolbarModelTestObserver::ExtensionToolbarModelTestObserver(
     ExtensionToolbarModel* model) : model_(model),
                                     inserted_count_(0),
                                     removed_count_(0),
                                     moved_count_(0),
                                     highlight_mode_count_(0),
+                                    initialized_count_(0),
                                     reorder_count_(0) {
   model_->AddObserver(this);
 }
 
 ExtensionToolbarModelTestObserver::~ExtensionToolbarModelTestObserver() {
   model_->RemoveObserver(this);
 }
 
 }  // namespace
 
@@ skipping 61 matching lines @@
   scoped_refptr<const Extension> browser_action_c_;
 
   // Sample extensions with different kinds of actions.
   scoped_refptr<const Extension> browser_action_extension_;
   scoped_refptr<const Extension> page_action_extension_;
   scoped_refptr<const Extension> no_action_extension_;
 };
 
 void ExtensionToolbarModelUnitTest::Init() {
   InitializeEmptyExtensionService();
-  toolbar_model_ = CreateToolbarModelForProfile(profile());
+  toolbar_model_ = CreateToolbarModelForProfile(profile(), true);
   model_observer_.reset(new ExtensionToolbarModelTestObserver(toolbar_model_));
 }
 
 void ExtensionToolbarModelUnitTest::TearDown() {
   model_observer_.reset();
   ExtensionServiceTestBase::TearDown();
 }
 
 testing::AssertionResult ExtensionToolbarModelUnitTest::AddExtension(
     const scoped_refptr<const Extension>& extension) {
@@ skipping 567 matching lines @@
   util::SetIsIncognitoEnabled(browser_action_c()->id(), profile(), true);
 
   // Move C to the second index.
   toolbar_model()->MoveExtensionIcon(browser_action_c()->id(), 1u);
   // Set visible count to 2 so that c is overflowed. State is A C [B].
   toolbar_model()->SetVisibleIconCount(2);
   EXPECT_EQ(1u, observer()->moved_count());
 
   // Get an incognito profile and toolbar.
   ExtensionToolbarModel* incognito_model =
-      CreateToolbarModelForProfile(profile()->GetOffTheRecordProfile());
+      CreateToolbarModelForProfile(profile()->GetOffTheRecordProfile(), true);
 
   ExtensionToolbarModelTestObserver incognito_observer(incognito_model);
   EXPECT_EQ(0u, incognito_observer.moved_count());
 
   // We should have two items, C and B, and the order should be preserved from
   // the original model.
   EXPECT_EQ(2u, incognito_model->toolbar_items().size());
   EXPECT_EQ(browser_action_c(), GetExtensionAtIndex(0u, incognito_model));
   EXPECT_EQ(browser_action_b(), GetExtensionAtIndex(1u, incognito_model));
 
@@ skipping 84 matching lines @@
   // The first model should have both extensions visible.
   EXPECT_EQ(2u, toolbar_model()->toolbar_items().size());
   EXPECT_EQ(extension_a, GetExtensionAtIndex(0)->id());
   EXPECT_EQ(extension_b, GetExtensionAtIndex(1)->id());
 
   // Set the model to only show one extension, so the order is A [B].
   toolbar_model()->SetVisibleIconCount(1u);
 
   // Get an incognito profile and toolbar.
   ExtensionToolbarModel* incognito_model =
-      CreateToolbarModelForProfile(profile()->GetOffTheRecordProfile());
+      CreateToolbarModelForProfile(profile()->GetOffTheRecordProfile(), true);
   ExtensionToolbarModelTestObserver incognito_observer(incognito_model);
 
   // Right now, no extensions are enabled in incognito mode.
   EXPECT_EQ(0u, incognito_model->toolbar_items().size());
 
   // Set extension b (which is overflowed) to be enabled in incognito. This
   // results in b reloading, so wait for it.
   {
     TestExtensionRegistryObserver observer(registry(), extension_b);
     util::SetIsIncognitoEnabled(extension_b, profile(), true);
@@ skipping 262 matching lines @@
   extension_action_api->NotifyChange(action, web_contents, profile());
   tab_order = toolbar_model()->GetItemOrderForTab(web_contents);
   ASSERT_EQ(3u, tab_order.size());
   EXPECT_EQ(page_action(), tab_order[0].get());
   EXPECT_EQ(browser_action(), tab_order[1].get());
   EXPECT_EQ(no_action(), tab_order[2].get());
   EXPECT_EQ(1u, toolbar_model()->GetVisibleIconCountForTab(web_contents));
   EXPECT_EQ(3u, observer()->reorder_count());
 }
 
+// Test that observers receive no Added notifications until after the
+// ExtensionSystem has initialized.
+TEST_F(ExtensionToolbarModelUnitTest, ModelWaitsForExtensionSystemReady) {
+  InitializeEmptyExtensionService();
+  ExtensionToolbarModel* toolbar_model =
+       CreateToolbarModelForProfile(profile(), false);
+  ExtensionToolbarModelTestObserver model_observer(toolbar_model);
+  EXPECT_TRUE(AddBrowserActionExtensions());
+
+  // Since the model hasn't been initialized (the ExtensionSystem::ready task
+  // hasn't been run), there should be no insertion notifications.
+  EXPECT_EQ(0u, model_observer.inserted_count());
+  EXPECT_EQ(0u, model_observer.initialized_count());
+  EXPECT_FALSE(toolbar_model->extensions_initialized());
+
+  // Run the ready task.
+  static_cast<TestExtensionSystem*>(ExtensionSystem::Get(profile()))->
+      SetReady();
+  // Run tasks posted to TestExtensionSystem.
+  base::RunLoop().RunUntilIdle();
+
+  // We should still have no insertions, but should have an initialized count.
+  EXPECT_TRUE(toolbar_model->extensions_initialized());
+  EXPECT_EQ(0u, model_observer.inserted_count());
+  EXPECT_EQ(1u, model_observer.initialized_count());
+}
+
 }  // namespace extensions