Make ToolbarActionsModel own ComponentToolbarActionsFactory

chrome/browser/ui/toolbar/toolbar_actions_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 "chrome/browser/ui/toolbar/toolbar_actions_model.h"
 
 #include <stddef.h>
 
 #include <memory>
 
@@ skipping 105 matching lines @@
 
 }  // namespace
 
 class ToolbarActionsModelUnitTest
     : public extensions::ExtensionServiceTestBase {
  public:
   ToolbarActionsModelUnitTest() {}
   ~ToolbarActionsModelUnitTest() override {}
 
  protected:
-  // Initialize the ExtensionService, ToolbarActionsModel, and
-  // ExtensionSystem.
-  void Init();
-
   enum class MigrationStatus {
     // The feature is enabled without any extension to migrate.
     FEATURE_ENABLED_NO_EXTENSION,
     // The feature is enabled and the user has installed an extension.
     FEATURE_ENABLED_EXTENSION_INSTALLED,
     // Feature is enabled with extension and a pref of false.
     FEATURE_ENABLED_WITH_PREF_FALSE,
     // Feature is enabled with extension and a pref of true.
     FEATURE_ENABLED_WITH_PREF_TRUE,
     // The feature is now disabled after previously being enabled, and the user
     // has a pref reflecting a previous migration.
     FEATURE_DISABLED_WITH_PREF_TRUE,
   };
 
-  // Initialize the ExtensionService, ToolbarActionsModel, and ExtensionSystem,
+  // Initializes the ExtensionService, ToolbarActionsModel, and ExtensionSystem.
+  void Init();
+
+  // Initializes the ExtensionService, ToolbarActionsModel, and ExtensionSystem,
+  // making ToolbarActionsModel use a MockComponentToolbarActionsFactory.
+  void InitWithMockActionsFactory();
+
+  // Initializes the ExtensionService, ToolbarActionsModel, and ExtensionSystem,
   // and an action extension to migrate to a component.  |migration_status|
   // is used to configure the user's initial migration status.
   void InitForMigrationTest(MigrationStatus migration_status);
 
+  // Initializes ExtensionSystem and triggers ToolbarActionsModel::OnReady() to
+  // be called with |mock_actions_factory| in use. The ownership of the actions
+  // factory gets transferred to |toolbar_model_|.
+  void ReadyWithMockActionsFactory(
+      MockComponentToolbarActionsFactory* mock_actions_factory);
+
   void TearDown() override;
 
   // Adds or removes the given |extension| and verify success.
   testing::AssertionResult AddExtension(
       const scoped_refptr<const extensions::Extension>& extension)
       WARN_UNUSED_RESULT;
   testing::AssertionResult RemoveExtension(
       const scoped_refptr<const extensions::Extension>& extension)
       WARN_UNUSED_RESULT;
 
   // Adds three extensions, all with browser actions.
   testing::AssertionResult AddBrowserActionExtensions() WARN_UNUSED_RESULT;
 
   // Adds three extensions, one each for browser action, page action, and no
   // action, and are added in that order.
   testing::AssertionResult AddActionExtensions() WARN_UNUSED_RESULT;
 
   // Creates an extension that is to be migrated to a component action.
-  void CreateMigratedActionExtension();
+  void CreateMigratedActionExtension(
+      MockComponentToolbarActionsFactory* mock_actions_factory);
 
   // Returns the action's id at the given index in the toolbar model, or empty
   // if one does not exist.
   // If |model| is specified, it is used. Otherwise, this defaults to
   // |toolbar_model_|.
   const std::string GetActionIdAtIndex(size_t index,
                                        const ToolbarActionsModel* model) const;
   const std::string GetActionIdAtIndex(size_t index) const;
 
   // Returns true if the |toobar_model_| has an action with the given |id|.
   bool ModelHasActionForId(const std::string& id) const;
 
-  void SetMockActionsFactory(MockComponentToolbarActionsFactory* factory);
-
   ToolbarActionsModel* toolbar_model() { return toolbar_model_; }
 
   const ToolbarActionsModelTestObserver* observer() const {
     return model_observer_.get();
   }
   size_t num_toolbar_items() const {
     return toolbar_model_->toolbar_items().size();
   }
   const extensions::Extension* browser_action_a() const {
     return browser_action_a_.get();
@@ skipping 38 matching lines @@
   scoped_refptr<const extensions::Extension> browser_action_a_;
   scoped_refptr<const extensions::Extension> browser_action_b_;
   scoped_refptr<const extensions::Extension> browser_action_c_;
   scoped_refptr<const extensions::Extension> browser_action_migrated_;
 
   // Sample extensions with different kinds of actions.
   scoped_refptr<const extensions::Extension> browser_action_extension_;
   scoped_refptr<const extensions::Extension> page_action_extension_;
   scoped_refptr<const extensions::Extension> no_action_extension_;
 
-  std::unique_ptr<MockComponentToolbarActionsFactory> mock_actions_factory_;
-
   DISALLOW_COPY_AND_ASSIGN(ToolbarActionsModelUnitTest);
 };
 
 void ToolbarActionsModelUnitTest::Init() {
   InitializeEmptyExtensionService();
   toolbar_model_ =
       extensions::extension_action_test_util::CreateToolbarModelForProfile(
           profile());
   model_observer_.reset(new ToolbarActionsModelTestObserver(toolbar_model_));
 }
 
+void ToolbarActionsModelUnitTest::InitWithMockActionsFactory() {

[msw, 2017/01/10 23:23:59]

nit: this is called once, maybe inline the functionality there?
(or find a way to consolidate some InitForMigrationTest code with this)

[takumif, 2017/02/17 03:22:52]

The test that calls this was already very long, so I didn't want to make it
longer.

+  InitializeEmptyExtensionService();
+  toolbar_model_ = extensions::extension_action_test_util::
+      CreateToolbarModelForProfileWithoutWaitingForReady(profile());
+  MockComponentToolbarActionsFactory* mock_actions_factory =
+      new MockComponentToolbarActionsFactory(profile(), toolbar_model_);

[msw, 2017/01/10 23:23:59]

Use base::MakeUnique

[takumif, 2017/02/17 03:22:52]

Done.

+  ReadyWithMockActionsFactory(mock_actions_factory);
+}
+
+void ToolbarActionsModelUnitTest::ReadyWithMockActionsFactory(
+    MockComponentToolbarActionsFactory* mock_actions_factory) {

[msw, 2017/01/10 23:23:59]

Use unique_ptr for ownership passing

[takumif, 2017/02/17 03:22:52]

Done.

+  mock_actions_factory->RegisterComponentMigrations();
+  toolbar_model_->SetMockActionsFactory(mock_actions_factory);
+
+  // Trigger ToolbarActionsModel::OnReady() after the actions factory has been
+  // swapped out for a mock one.
+  static_cast<extensions::TestExtensionSystem*>(
+      extensions::ExtensionSystem::Get(profile()))
+      ->SetReady();
+  base::RunLoop().RunUntilIdle();
+
+  model_observer_.reset(new ToolbarActionsModelTestObserver(toolbar_model_));
+}
+
 void ToolbarActionsModelUnitTest::InitForMigrationTest(
     MigrationStatus migration_status) {
   InitializeEmptyExtensionService();
-  SetMockActionsFactory(new MockComponentToolbarActionsFactory(nullptr));
-  CreateMigratedActionExtension();
+  toolbar_model_ = extensions::extension_action_test_util::
+      CreateToolbarModelForProfileWithoutWaitingForReady(profile());
+  MockComponentToolbarActionsFactory* mock_actions_factory =
+      new MockComponentToolbarActionsFactory(profile(), toolbar_model_);
+  CreateMigratedActionExtension(mock_actions_factory);
 
   {
     DictionaryPrefUpdate update(profile()->GetPrefs(),
                                 ::prefs::kToolbarMigratedComponentActionStatus);
     switch (migration_status) {
       case MigrationStatus::FEATURE_ENABLED_EXTENSION_INSTALLED:
-        mock_actions_factory_->set_migrated_feature_enabled(true);
+        mock_actions_factory->set_migrated_feature_enabled(true);
         ASSERT_TRUE(AddExtension(browser_action_migrated()));
         break;
       case MigrationStatus::FEATURE_ENABLED_NO_EXTENSION:
-        mock_actions_factory_->set_migrated_feature_enabled(true);
+        mock_actions_factory->set_migrated_feature_enabled(true);
         break;
       case MigrationStatus::FEATURE_ENABLED_WITH_PREF_TRUE:
-        mock_actions_factory_->set_migrated_feature_enabled(true);
+        mock_actions_factory->set_migrated_feature_enabled(true);
         ASSERT_TRUE(AddExtension(browser_action_migrated()));
         update->SetBoolean(component_action_id(), true);
         break;
       case MigrationStatus::FEATURE_ENABLED_WITH_PREF_FALSE:
-        mock_actions_factory_->set_migrated_feature_enabled(true);
+        mock_actions_factory->set_migrated_feature_enabled(true);
         ASSERT_TRUE(AddExtension(browser_action_migrated()));
         update->SetBoolean(component_action_id(), false);
         break;
       case MigrationStatus::FEATURE_DISABLED_WITH_PREF_TRUE:
-        mock_actions_factory_->set_migrated_feature_enabled(false);
+        mock_actions_factory->set_migrated_feature_enabled(false);
         ASSERT_TRUE(AddExtension(browser_action_migrated()));
         update->SetBoolean(component_action_id(), true);
         break;
     }
   }
 
-  toolbar_model_ =
-      extensions::extension_action_test_util::CreateToolbarModelForProfile(
-          profile());
-  model_observer_.reset(new ToolbarActionsModelTestObserver(toolbar_model_));
+  ReadyWithMockActionsFactory(mock_actions_factory);
 }
 
 void ToolbarActionsModelUnitTest::TearDown() {
   model_observer_.reset();
   extensions::ExtensionServiceTestBase::TearDown();
 }
 
 testing::AssertionResult ToolbarActionsModelUnitTest::AddExtension(
     const scoped_refptr<const extensions::Extension>& extension) {
   if (registry()->enabled_extensions().GetByID(extension->id())) {
@@ skipping 59 matching lines @@
           extensions::extension_action_test_util::BROWSER_ACTION);
 
   extensions::ExtensionList extensions;
   extensions.push_back(browser_action_a_);
   extensions.push_back(browser_action_b_);
   extensions.push_back(browser_action_c_);
 
   return AddAndVerifyExtensions(extensions);
 }
 
-void ToolbarActionsModelUnitTest::CreateMigratedActionExtension() {
+void ToolbarActionsModelUnitTest::CreateMigratedActionExtension(
+    MockComponentToolbarActionsFactory* mock_actions_factory) {
   browser_action_migrated_ =
       extensions::extension_action_test_util::CreateActionExtension(
           "browser_actionMigrated",
           extensions::extension_action_test_util::BROWSER_ACTION);
-  mock_actions_factory_->set_migrated_extension_id(
+  mock_actions_factory->set_migrated_extension_id(
       browser_action_migrated_->id());
 }
 
 const std::string ToolbarActionsModelUnitTest::GetActionIdAtIndex(
     size_t index,
     const ToolbarActionsModel* model) const {
   return index < model->toolbar_items().size()
              ? model->toolbar_items()[index].id
              : std::string();
 }
@@ skipping 17 matching lines @@
   for (extensions::ExtensionList::const_iterator iter = extensions.begin();
        iter != extensions.end(); ++iter) {
     if (!AddExtension(*iter)) {
       return testing::AssertionFailure() << "Failed to install extension: "
                                          << (*iter)->name();
     }
   }
   return testing::AssertionSuccess();
 }
 
-void ToolbarActionsModelUnitTest::SetMockActionsFactory(
-    MockComponentToolbarActionsFactory* factory) {
-  mock_actions_factory_.reset(factory);
-}
-
 // A basic test for component actions and extensions with browser actions
 // showing up in the toolbar.
 TEST_F(ToolbarActionsModelUnitTest, BasicToolbarActionsModelTest) {
   Init();
 
   // Load an extension with a browser action.
   scoped_refptr<const extensions::Extension> extension =
       extensions::extension_action_test_util::CreateActionExtension(
           "browser_action",
           extensions::extension_action_test_util::BROWSER_ACTION);
@@ skipping 878 matching lines @@
   EXPECT_EQ(inserted_and_removed_difference,
             observer()->inserted_count() - observer()->removed_count());
 }
 
 // Test various different reorderings, removals, and reinsertions of the
 // toolbar with component actions.
 TEST_F(ToolbarActionsModelUnitTest,
        ActionsToolbarReorderAndReinsertWithSwitchAndComponentActions) {
   extensions::FeatureSwitch::ScopedOverride enable_redesign(
       extensions::FeatureSwitch::extension_action_redesign(), true);
-  SetMockActionsFactory(new MockComponentToolbarActionsFactory(nullptr));
-  Init();
+  InitWithMockActionsFactory();
 
   // One component action was added when the model was initialized.
   EXPECT_EQ(1u, num_toolbar_items());
   EXPECT_EQ(component_action_id(), GetActionIdAtIndex(0u));
   EXPECT_TRUE(toolbar_model()->HasComponentAction(component_action_id()));
 
   // Add the three browser action extensions.
   ASSERT_TRUE(AddBrowserActionExtensions());
 
   // Verify the four actions are in the model in the proper order.
@@ skipping 256 matching lines @@
 }
 
 TEST_F(ToolbarActionsModelUnitTest,
        MigrationToExtensionWithoutExtensionActionRedesign) {
   InitForMigrationTest(MigrationStatus::FEATURE_DISABLED_WITH_PREF_TRUE);
 
   // Initialization re-enables the extension.
   EXPECT_EQ(1u, num_toolbar_items());
   EXPECT_EQ(browser_action_migrated()->id(), GetActionIdAtIndex(0u));
 }