Make ToolbarActionsModel own ComponentToolbarActionsFactory

chrome/browser/ui/toolbar/component_toolbar_actions_browsertest.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 <memory>
 
 #include "base/macros.h"
 #include "chrome/browser/extensions/browser_action_test_util.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/toolbar/component_toolbar_actions_factory.h"
 #include "chrome/browser/ui/toolbar/mock_component_toolbar_actions_factory.h"
 #include "chrome/browser/ui/toolbar/test_toolbar_action_view_controller.h"
 #include "chrome/browser/ui/toolbar/toolbar_action_view_controller.h"
 #include "chrome/browser/ui/toolbar/toolbar_actions_bar.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "extensions/common/feature_switch.h"
 
 class ComponentToolbarActionsBrowserTest : public InProcessBrowserTest {
  protected:
   ComponentToolbarActionsBrowserTest() {}
   ~ComponentToolbarActionsBrowserTest() override {}
 
-  void SetUpCommandLine(base::CommandLine* command_line) override {
-    InProcessBrowserTest::SetUpCommandLine(command_line);
-    enable_redesign_.reset(new extensions::FeatureSwitch::ScopedOverride(
-        extensions::FeatureSwitch::extension_action_redesign(), true));
-    mock_actions_factory_.reset(new MockComponentToolbarActionsFactory(
-        browser()));
-  }
+  void SetUpOnMainThread() override {
+    InProcessBrowserTest::SetUpOnMainThread();
 
-  MockComponentToolbarActionsFactory* mock_factory() {
-    return mock_actions_factory_.get();
+    // Swap out the actions factory for a mock one.
+    ToolbarActionsModel::Get(browser()->profile())

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

optional nit: cache a local profile pointer

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

Done.

+        ->SetMockActionsFactory(new MockComponentToolbarActionsFactory(
+            browser()->profile(),
+            ToolbarActionsModel::Get(browser()->profile())));
   }
 
  private:
-  std::unique_ptr<extensions::FeatureSwitch::ScopedOverride> enable_redesign_;

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

Is this no longer needed?

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

No, removing.

-  std::unique_ptr<MockComponentToolbarActionsFactory> mock_actions_factory_;
-
   DISALLOW_COPY_AND_ASSIGN(ComponentToolbarActionsBrowserTest);
 };
 
 // Test that Component Toolbar Actions appear in the browser actions container
 // and can receive click events properly.
 IN_PROC_BROWSER_TEST_F(ComponentToolbarActionsBrowserTest,
                        ComponentToolbarActionsShowUpAndRespondToClicks) {
   BrowserActionTestUtil browser_actions_bar(browser());
 
+  ToolbarActionsModel::Get(browser()->profile())
+      ->AddComponentAction(
+          MockComponentToolbarActionsFactory::kActionIdForTesting);
+
   // There should be only one component action view.
   ASSERT_EQ(1, browser_actions_bar.NumberOfBrowserActions());
 
   // Even though the method says "ExtensionId", this actually refers to any id
   // for the action.
   EXPECT_EQ(MockComponentToolbarActionsFactory::kActionIdForTesting,
             browser_actions_bar.GetExtensionId(0));
 
-  // There should only have been one created component action.
-  EXPECT_EQ(1u, ComponentToolbarActionsFactory::GetInstance()

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

Should GetInitialComponentIds still be checked in this test?

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

This was just to make sure that the MockComponentToolbarActionsFactory returns a
test action as expected. I've changed to explicitly adding the action by calling
AddComponentAction() above, so this is no longer necessary.

-                    ->GetInitialComponentIds(browser()->profile())
-                    .size());
-
   const std::vector<ToolbarActionViewController*>& actions =
       browser_actions_bar.GetToolbarActionsBar()->GetActions();
   TestToolbarActionViewController* mock_component_action =
       static_cast<TestToolbarActionViewController* const>(actions[0]);
   ASSERT_TRUE(mock_component_action);
 
   // Test that clicking on the component action works.
   EXPECT_EQ(0, mock_component_action->execute_action_count());
   browser_actions_bar.Press(0);
   EXPECT_EQ(1, mock_component_action->execute_action_count());
 }