Move ExtensionAction from common/ to browser/.

chrome/browser/extensions/extension_action.h

 // 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.
 
-#ifndef CHROME_COMMON_EXTENSIONS_EXTENSION_ACTION_H_
-#define CHROME_COMMON_EXTENSIONS_EXTENSION_ACTION_H_
+#ifndef CHROME_BROWSER_EXTENSIONS_EXTENSION_ACTION_H_

[Aaron Boodman, 2012/09/16 01:42:50]

Wow, neat that you can literally just move the file. I guess this is evidence it
belonged here :).

+#define CHROME_BROWSER_EXTENSIONS_EXTENSION_ACTION_H_
 
 #include <map>
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/memory/linked_ptr.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
+#include "chrome/common/extensions/extension.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/base/animation/linear_animation.h"
 
 class GURL;
+class Profile;
 class SkBitmap;
 class SkDevice;
 
 namespace gfx {
 class Canvas;
 class Image;
 class ImageSkia;
 class Rect;
 class Size;
 }
 
 // ExtensionAction encapsulates the state of a browser action, page action, or
 // script badge.
 // Instances can have both global and per-tab state. If a property does not have
 // a per-tab value, the global value is used instead.
 class ExtensionAction {
  public:
   // Use this ID to indicate the default state for properties that take a tab_id
   // parameter.
   static const int kDefaultTabId;
 
-  // The types of extension actions.
-  enum Type {
-    TYPE_BROWSER,
-    TYPE_PAGE,
-    TYPE_SCRIPT_BADGE,
-  };
-
   enum Appearance {
     // The action icon is hidden.
     INVISIBLE,
     // The action is trying to get the user's attention but isn't yet
     // running on the page.  Currently only used for script badges.
     WANTS_ATTENTION,
     // The action icon is visible with its normal appearance.
     ACTIVE,
   };
 
@@ skipping 50 matching lines @@
     // Device we use to paint icons to.
     mutable scoped_ptr<SkDevice> device_;
 
     ObserverList<Observer> observers_;
 
     base::WeakPtrFactory<IconAnimation> weak_ptr_factory_;
 
     DISALLOW_COPY_AND_ASSIGN(IconAnimation);
   };
 
-  ExtensionAction(const std::string& extension_id, Type action_type);
+  ExtensionAction(const std::string& extension_id,
+                  const extensions::Extension::ActionInfo& manifest_info);
   ~ExtensionAction();
 
   // Gets a copy of this, ownership passed to caller.
   // It doesn't make sense to copy of an ExtensionAction except in tests.
   scoped_ptr<ExtensionAction> CopyForTest() const;
 
   // extension id
   const std::string& extension_id() const { return extension_id_; }
 
   // What kind of action is this?
-  Type action_type() const { return action_type_; }
+  extensions::Extension::ActionInfo::Type action_type() const {
+    return action_type_;
+  }
 
   // action id -- only used with legacy page actions API
   std::string id() const { return id_; }
   void set_id(const std::string& id) { id_ = id; }
 
   // static icon paths from manifest -- only used with legacy page actions API.
   std::vector<std::string>* icon_paths() { return &icon_paths_; }
   const std::vector<std::string>* icon_paths() const { return &icon_paths_; }
 
   bool has_changed() const { return has_changed_; }
@@ skipping 155 matching lines @@
     } else {
       iter = map->find(kDefaultTabId);
       return iter != map->end() ? iter->second : ValueTraits<T>::CreateEmpty();
     }
   }
 
   // The id for the extension this action belongs to (as defined in the
   // extension manifest).
   const std::string extension_id_;
 
-  const Type action_type_;
+  const extensions::Extension::ActionInfo::Type action_type_;
 
   // Each of these data items can have both a global state (stored with the key
   // kDefaultTabId), or tab-specific state (stored with the tab_id as the key).
   std::map<int, GURL> popup_url_;
   std::map<int, std::string> title_;
   std::map<int, gfx::ImageSkia> icon_;
   std::map<int, int> icon_index_;  // index into icon_paths_
   std::map<int, std::string> badge_text_;
   std::map<int, SkColor> badge_background_color_;
   std::map<int, SkColor> badge_text_color_;
@@ skipping 26 matching lines @@
   DISALLOW_COPY_AND_ASSIGN(ExtensionAction);
 };
 
 template<>
 struct ExtensionAction::ValueTraits<int> {
   static int CreateEmpty() {
     return -1;
   }
 };
 
-#endif  // CHROME_COMMON_EXTENSIONS_EXTENSION_ACTION_H_
+namespace extensions {
+ExtensionAction* GetBrowserAction(Profile* profile, const Extension& extension);

[Jeffrey Yasskin, 2012/09/14 19:18:33]

Functions along these lines are important to shorten code that needs to retrieve
ExtensionActions, but they could live in several places. Aaron, do you have an
opinion on where?

Relatedly, should we put ExtensionAction in the extensions namespace in this
change? I don't think removing the Extension prefix makes sense even if we put
it in the namespace.

[Aaron Boodman, 2012/09/16 01:42:50]

namespace: yes, keep 'Extension' prefix: yes

[Aaron Boodman, 2012/09/16 01:42:50]

The downside of free functions like this is that it's not immediately clear when
reading the code where they come from.

I'd typically prefer them as static members of ExtensionAction, but then I'm not
sure if it's enough of a typing savings.

[not at google - send to devlin, 2012/09/27 00:46:10]

I also do not like free/utility functions like this.

Even if they do live as static functions on ExtensionAction, it slightly
obfuscates where the ExtensionActions come from, with the only advantage being
saving a bit of typing. It also means that somebody cargo-culting code might
think they need a Profile when really all they need is an
ExtensionActionManager, which leads to dependency bloat and testing
difficulties.

Aside: I get upset every time I need to find the header file where
chrome::GetActiveWebContents lives. Grr! Y U NOT ON BROWSER.

Yennyway, the question is just who owns the ExtensionActions now. And the answer
is (I hear) ExtensionSystem (or some object which hangs off it like an
ExtensionActionManager -- which I think is a good idea, keep dependencies
small). If a class repeatedly gets page actions and wants to save typing, it can
cache a reference to the ExtensionActionManager (though more likely it's just
going to cache a reference to the ExtensionAction itself).

+ExtensionAction* GetPageAction(Profile* profile, const Extension& extension);
+ExtensionAction* GetScriptBadge(Profile* profile, const Extension& extension);
+}
+
+#endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_ACTION_H_