Move ShowPopup logic from BrowserActionsContainer to BrowserActionView

chrome/browser/ui/views/toolbar/toolbar_view.cc

 // Copyright 2013 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/views/toolbar/toolbar_view.h"
 
 #include <algorithm>
 
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
@@ skipping 320 matching lines @@
   ExtensionAction* extension_action =
       extension_manager->GetPageAction(*extension);
   if (extension_action) {
     location_bar_->GetPageActionView(extension_action)->image_view()->
         ExecuteAction(ExtensionPopup::SHOW);
   }
 }
 
 void ToolbarView::ShowBrowserActionPopup(
     const extensions::Extension* extension) {
-  browser_actions_->ShowPopup(extension, true);
+  browser_actions_->ShowPopupForExtension(
+      extension,
+      true,  // Grant active tab permissions.
+      false);  // Don't override the current view.

[Peter Kasting, 2014/07/26 02:33:20]

Nit: I'm not a huge fan of comments on function args.  If the meaning would be
clear enough (possibly with a reference to the function declaration) to the
typical reader, we don't need them.  If not, we may want to consider named enum
values in place of bools.

My normal answer here is to just remove the comments.

[Devlin, 2014/07/29 19:07:15]

I typically view them as providing a reference to the function declaration, to
save any reader from having to look it up (because, as you said, they should be
clear with that).  Tentatively removed the comments, but out of curiosity, would
you feel any better about a shorter "grant_tab_permissions" and "can_override"
(simply documenting which variables each bool is associated with)?

[Peter Kasting, 2014/07/29 19:18:01]

I think those are probably slightly worse than the comments you originally had. 
If we really need to say something because the reader can't look up the function
definition or whatever, it's better to just spell out what we're talking about
than to leave comments that may require the reader to look up the prototype
anyway just to figure them out.

 }
 
 void ToolbarView::ShowAppMenu(bool for_drop) {
   if (wrench_menu_.get() && wrench_menu_->IsShowing())
     return;
 
   int run_flags = 0;
   bool use_new_menu = false;
   // TODO: remove this.
 #if !defined(OS_LINUX) || defined(OS_CHROMEOS)
@@ skipping 448 matching lines @@
 
 void ToolbarView::OnShowHomeButtonChanged() {
   Layout();
   SchedulePaint();
 }
 
 int ToolbarView::content_shadow_height() const {
   return browser_->host_desktop_type() == chrome::HOST_DESKTOP_TYPE_ASH ?
       kContentShadowHeightAsh : kContentShadowHeight;
 }