Implement initial chrome.browserAction.openPopup API.

chrome/browser/extensions/extension_toolbar_model.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_BROWSER_EXTENSIONS_EXTENSION_TOOLBAR_MODEL_H_
 #define CHROME_BROWSER_EXTENSIONS_EXTENSION_TOOLBAR_MODEL_H_
 
 #include "base/compiler_specific.h"
 #include "base/observer_list.h"
 #include "base/prefs/pref_change_registrar.h"
@@ skipping 30 matching lines @@
     virtual void BrowserActionAdded(const extensions::Extension* extension,
                                     int index) {}
 
     // The browser action button for |extension| should no longer show.
     virtual void BrowserActionRemoved(const extensions::Extension* extension) {}
 
     // The browser action button for |extension| has been moved to |index|.
     virtual void BrowserActionMoved(const extensions::Extension* extension,
                                     int index) {}
 
+    // The browser action button for |extension| should show the popup now if it
+    // is in the active window.
+    virtual void BrowserActionShowPopup(const extensions::Extension* extension)
+        {}
+
     // Called when the model has finished loading.
     virtual void ModelLoaded() {}
 
    protected:
     virtual ~Observer() {}
   };
 
   // Functions called by the view.
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
   void MoveBrowserAction(const extensions::Extension* extension, int index);
   // Executes the browser action for an extension and returns the action that
   // the UI should perform in response.
   // |popup_url_out| will be set if the extension should show a popup, with
-  // the URL that should be shown, if non-NULL.
+  // the URL that should be shown, if non-NULL. |should_grant| controls whether
+  // the browser action should grant the page tab permissions. The showPopup API
+  // does not grant permissions.
+  Action ExecuteBrowserAction(const extensions::Extension* extension,
+                              Browser* browser,
+                              GURL* popup_url_out,
+                              bool should_grant);

[Finnur, 2013/10/16 15:01:26]

These kinds of optional params (should_grant) are against the style guide. You
should add this param to the function below and update all the callers.

[justinlin, 2013/10/16 19:31:29]

Hmm.. according to this, this should be the preferred way to add an argument
that can be overriden, but is seldom used:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Defaul...

I believe SimpleFeature does something similar as well. I can change all callers
too, but I think it makes this call more confusing since almost everytime except
through this new extension API you'll want the granting behavior. WDYT?

[Finnur, 2013/10/17 14:58:34]

The public style guide seems to be down (my coworker also tried).

I looked up Default Argument on the internal style guide and I don't see it as
the preferred way.

You have two options: 
1) Add the param to the original function and update all the callers.
2) Add a second function with a different (but similar name) that redirects.

1 is cleaner, I believe, but if we have a ton of callers that all pass in false
except one (which I don't think is the case) then I guess 2 is preferred. Having
said that, you'd probably want ExecuteBrowserAction and
ExecuteBrowserActionAndGrant...etc, so you'd end up updating all the call sites
anyway.

[justinlin, 2013/10/17 18:32:53]

Done.

   Action ExecuteBrowserAction(const extensions::Extension* extension,
                               Browser* browser,
                               GURL* popup_url_out);
   // If count == size(), this will set the visible icon count to -1, meaning
   // "show all actions".
   void SetVisibleIconCount(int count);
   // As above, a return value of -1 represents "show all actions".
   int GetVisibleIconCount() const { return visible_icon_count_; }
 
   bool extensions_initialized() const { return extensions_initialized_; }
 
   const extensions::ExtensionList& toolbar_items() const {
     return toolbar_items_;
   }
 
   // Utility functions for converting between an index into the list of
   // incognito-enabled browser actions, and the list of all browser actions.
   int IncognitoIndexToOriginal(int incognito_index);
   int OriginalIndexToIncognito(int original_index);
 
   void OnExtensionToolbarPrefChange();
+  // Tells observers to display a popup without granting tab permissions.

[Finnur, 2013/10/16 15:01:26]

nit: Line break before this line.

[justinlin, 2013/10/16 19:31:29]

Done.

+  void ShowBrowserActionPopup(const extensions::Extension* extension);
 
  private:
   // content::NotificationObserver implementation.
   virtual void Observe(int type,
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE;
 
   // To be called after the extension service is ready; gets loaded extensions
   // from the extension service and their saved order from the pref service
   // and constructs |toolbar_items_| from these data.
@@ skipping 40 matching lines @@
   // For observing change of toolbar order preference by external entity (sync).
   PrefChangeRegistrar pref_change_registrar_;
   base::Closure pref_change_callback_;
 
   base::WeakPtrFactory<ExtensionToolbarModel> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionToolbarModel);
 };
 
 #endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_TOOLBAR_MODEL_H_