Make ToolbarActionsModel own ComponentToolbarActionsFactory

chrome/browser/ui/toolbar/toolbar_actions_model.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 "chrome/browser/ui/toolbar/toolbar_actions_model.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/location.h"
 #include "base/metrics/histogram_base.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/single_thread_task_runner.h"
 #include "base/stl_util.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/component_migration_helper.h"
 #include "chrome/browser/extensions/extension_action_manager.h"
 #include "chrome/browser/extensions/extension_message_bubble_controller.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/extensions/extension_action_view_controller.h"
 #include "chrome/browser/ui/extensions/extension_message_bubble_factory.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
-#include "chrome/browser/ui/toolbar/component_toolbar_actions_factory.h"
 #include "chrome/browser/ui/toolbar/toolbar_action_view_controller.h"
 #include "chrome/browser/ui/toolbar/toolbar_actions_bar.h"
 #include "chrome/browser/ui/toolbar/toolbar_actions_model_factory.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/web_contents.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/pref_names.h"
 #include "extensions/common/extension_set.h"
 #include "extensions/common/feature_switch.h"
 #include "extensions/common/manifest_constants.h"
 #include "extensions/common/one_shot_event.h"
 
 ToolbarActionsModel::ToolbarActionsModel(
     Profile* profile,
     extensions::ExtensionPrefs* extension_prefs)
     : profile_(profile),
       extension_prefs_(extension_prefs),
       prefs_(profile_->GetPrefs()),
       extension_action_api_(extensions::ExtensionActionAPI::Get(profile_)),
       extension_registry_(extensions::ExtensionRegistry::Get(profile_)),
       extension_action_manager_(
           extensions::ExtensionActionManager::Get(profile_)),
-      component_migration_helper_(
-          new extensions::ComponentMigrationHelper(profile_, this)),
+      component_actions_factory_(
+          base::MakeUnique<ComponentToolbarActionsFactory>(profile_, this)),
       actions_initialized_(false),
       use_redesign_(
           extensions::FeatureSwitch::extension_action_redesign()->IsEnabled()),
       highlight_type_(HIGHLIGHT_NONE),
       has_active_bubble_(false),
       extension_action_observer_(this),
       extension_registry_observer_(this),
       weak_ptr_factory_(this) {
-  ComponentToolbarActionsFactory::GetInstance()->RegisterComponentMigrations(
-      component_migration_helper_.get());
+  component_actions_factory_->RegisterComponentMigrations();
   extensions::ExtensionSystem::Get(profile_)->ready().Post(
       FROM_HERE, base::Bind(&ToolbarActionsModel::OnReady,
                             weak_ptr_factory_.GetWeakPtr()));
   visible_icon_count_ =
       prefs_->GetInteger(extensions::pref_names::kToolbarSize);
 
   // We only care about watching the prefs if not in incognito mode.
   if (!profile_->IsOffTheRecord()) {
     pref_change_registrar_.Init(prefs_);
     pref_change_callback_ =
@@ skipping 67 matching lines @@
   // don't store anything in incognito.
   if (!is_highlighting() && !profile_->IsOffTheRecord()) {
     prefs_->SetInteger(extensions::pref_names::kToolbarSize,
                        visible_icon_count_);
   }
 
   for (Observer& observer : observers_)
     observer.OnToolbarVisibleCountChanged();
 }
 
+extensions::ComponentMigrationHelper*
+ToolbarActionsModel::GetComponentMigrationHelper() {
+  return component_actions_factory_->migration_helper();
+}
+
 void ToolbarActionsModel::OnExtensionActionUpdated(
     ExtensionAction* extension_action,
     content::WebContents* web_contents,
     content::BrowserContext* browser_context) {
   // Notify observers if the extension exists and is in the model.
   if (HasItem(
           ToolbarItem(extension_action->extension_id(), EXTENSION_ACTION))) {
     for (Observer& observer : observers_)
       observer.OnToolbarActionUpdated(extension_action->extension_id());
   }
@@ skipping 25 matching lines @@
       DCHECK(extension);
 
       // Create and add an ExtensionActionViewController for the extension.
       result.reset(new ExtensionActionViewController(
           extension, browser,
           extension_action_manager_->GetExtensionAction(*extension), bar));
       break;
     }
     case COMPONENT_ACTION: {
       DCHECK(use_redesign_);
-      result = ComponentToolbarActionsFactory::GetInstance()
-                   ->GetComponentToolbarActionForId(item.id, browser, bar);
+      result = component_actions_factory_->GetComponentToolbarActionForId(
+          item.id, browser, bar);
       break;
     }
     case UNKNOWN_ACTION:
       NOTREACHED();  // Should never have an UNKNOWN_ACTION in toolbar_items.
       break;
   }
   return result;
 }
 
 void ToolbarActionsModel::OnExtensionActionVisibilityChanged(
@@ skipping 61 matching lines @@
   extension_action_observer_.Add(extension_action_api_);
 
   actions_initialized_ = true;
   for (Observer& observer : observers_)
     observer.OnToolbarModelInitialized();
 
   if (use_redesign_) {
     // Handle component action migrations.  We must make sure that observers are
     // notified of initialization first, so that the associated widgets are
     // created.
-    ComponentToolbarActionsFactory::GetInstance()->HandleComponentMigrations(
-        component_migration_helper_.get(), profile_);
+    component_actions_factory_->HandleComponentMigrations();
   }
 }
 
 size_t ToolbarActionsModel::FindNewPositionFromLastKnownGood(
     const ToolbarItem& action) {
   // See if we have last known good position for this action.
   size_t new_index = 0;
   // Loop through the ID list of known positions, to count the number of
   // visible action icons preceding |action|'s id.
   for (const std::string& last_pos_id : last_known_positions_) {
@@ skipping 147 matching lines @@
 ToolbarActionsModel::GetExtensionMessageBubbleController(Browser* browser) {
   std::unique_ptr<extensions::ExtensionMessageBubbleController> controller;
   if (has_active_bubble())
     return controller;
   controller = ExtensionMessageBubbleFactory(browser).GetController();
   if (controller)
     controller->SetIsActiveBubble();
   return controller;
 }
 
+void ToolbarActionsModel::SetMockActionsFactory(
+    ComponentToolbarActionsFactory* mock_factory) {
+  component_actions_factory_.reset(mock_factory);
+}
+
 void ToolbarActionsModel::RemoveExtension(
     const extensions::Extension* extension) {
   RemoveItem(ToolbarItem(extension->id(), EXTENSION_ACTION));
 }
 
 // Combine the currently enabled extensions that have browser actions (which
 // we get from the ExtensionRegistry) and component actions (which we get from
 // ComponentToolbarActionsFactory) with the ordering we get from the pref
 // service. For robustness we use a somewhat inefficient process:
 // 1. Create a vector of actions sorted by their pref values. This vector may
@@ skipping 33 matching lines @@
       if (!extension_action_api_->GetBrowserActionVisibility(extension->id()))
         ++hidden;
       continue;
     }
 
     all_actions.push_back(ToolbarItem(extension->id(), EXTENSION_ACTION));
   }
 
   // Next, add the component action ids.
   std::set<std::string> component_ids =
-      ComponentToolbarActionsFactory::GetInstance()->GetInitialComponentIds(
-          profile_);
+      component_actions_factory_->GetInitialComponentIds();
   for (const std::string& id : component_ids)
     all_actions.push_back(ToolbarItem(id, COMPONENT_ACTION));
 
   // Add each action id to the appropriate list. Since the |sorted| list is
   // created with enough room for each id in |positions| (which helps with
   // proper order insertion), holes can be present if there isn't an action
   // for each id. This is handled below when we add the actions to
   // |toolbar_items_| to ensure that there are never any holes in
   // |toolbar_items_| itself (or, relatedly, CreateActions()).
   for (const ToolbarItem& action : all_actions) {
@@ skipping 109 matching lines @@
   return base::ContainsValue(toolbar_items_, item);
 }
 
 bool ToolbarActionsModel::HasComponentAction(
     const std::string& action_id) const {
   DCHECK(use_redesign_);
   return HasItem(ToolbarItem(action_id, COMPONENT_ACTION));
 }
 
 void ToolbarActionsModel::AddComponentAction(const std::string& action_id) {
+  DCHECK(use_redesign_);
   if (!actions_initialized_) {
-    // TODO(crbug.com/660972): Add these component actions at initialization.
+    component_actions_factory_->OnAddComponentActionBeforeInit(action_id);

[Devlin, 2017/01/10 23:07:11]

I'm not entirely sure this is less convoluted than before... it seems like we
still have this strange triangle of
                       ToolbarActionsModel
ComponentToolbarActionsFactory       ComponentMigrationHelper

where each has to know about the other in some way, shape, or form (though they
might be hidden behind delegate interfaces).  I think I'd prefer it if we could
make these a little more unidirectional.

What if instead, we have ComponentToolbarActionsFactory own the
ComponentMigrationHelper (like you have), but also implement its delegate
interface?  And then have the factory call directly into ToolbarActionsModel to
add component actions, and have the model have the caching mechanism.  Then, we
have a little more orderly flow of knowledge.  Does that make sense?  WDYT? 
(I'm brainstorming right now, so lemme know if I'm totally off base :))

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

I think things are a bit less complex after the removal of
CompomentMigrationHelper. ToolbarActionsModel owns
ComponentToolbarActionsFactory, and ComponentToolbarActionsFactory has a
reference to ToolbarActionsModel through ComponentActionDelegate interface.

     return;
   }
 
-  DCHECK(use_redesign_);
   ToolbarItem component_item(action_id, COMPONENT_ACTION);
   DCHECK(!HasItem(component_item));
   AddItem(component_item);
 }
 
 void ToolbarActionsModel::RemoveComponentAction(const std::string& action_id) {
   DCHECK(use_redesign_);
+  if (!actions_initialized_) {
+    component_actions_factory_->OnRemoveComponentActionBeforeInit(action_id);
+    return;
+  }
   // If the action was visible and there are overflowed actions, we reduce the
   // visible count so that we don't pop out a previously-hidden action.
   if (IsActionVisible(action_id) && !all_icons_visible())
     SetVisibleIconCount(visible_icon_count() - 1);
 
   ToolbarItem component_item(action_id, COMPONENT_ACTION);
   DCHECK(HasItem(component_item));
   RemoveItem(component_item);
   RemovePref(component_item);
 }
 
 void ToolbarActionsModel::IncognitoPopulate() {
   DCHECK(profile_->IsOffTheRecord());
   const ToolbarActionsModel* original_model =
       ToolbarActionsModel::Get(profile_->GetOriginalProfile());
 
   // Find the absolute value of the original model's count.
   int original_visible = original_model->visible_icon_count();
 
   // In incognito mode, we show only those actions that are incognito-enabled
   // Further, any actions that were overflowed in regular mode are still
   // overflowed. Order is the same as in regular mode.
   visible_icon_count_ = 0;
 
   std::set<std::string> component_ids =
-      ComponentToolbarActionsFactory::GetInstance()->GetInitialComponentIds(
-          profile_);
+      component_actions_factory_->GetInitialComponentIds();
   for (std::vector<ToolbarItem>::const_iterator iter =
            original_model->toolbar_items_.begin();
        iter != original_model->toolbar_items_.end(); ++iter) {
     // The extension might not be shown in incognito mode.
     // We may also disable certain component actions in incognito mode.
     bool should_add = false;
     switch (iter->type) {
       case EXTENSION_ACTION:
         should_add = ShouldAddExtension(GetExtensionById(iter->id));
         break;
@@ skipping 175 matching lines @@
   return extension_registry_->enabled_extensions().GetByID(id);
 }
 
 bool ToolbarActionsModel::IsActionVisible(const std::string& action_id) const {
   size_t index = 0u;
   while (toolbar_items().size() > index &&
          toolbar_items()[index].id != action_id)
     ++index;
   return index < visible_icon_count();
 }