Add manage passwords icon to PWA windows

This CL allows PWA title bars to show the manage passwords icon
by moving it from LocationBarView to PageActionIconContainerView.

PWA title bar:
Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=368396&signed_aid=_t4hsG2GFjiEe__8vT-LtQ==&inline=1
After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=368397&signed_aid=q2ASdnVuMGpsPn9GAjrdmg==&inline=1

Location bar (no diff):
Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=368398&signed_aid=hPiZlkK4KcRMeaLejACitg==&inline=1
After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=368399&signed_aid=q0PRCcZ5Fi0BRFv3oqcSQA==&inline=1

Bug: 788051
Change-Id: I54508125c77858bb8c40e80a0971fb6f9ddc9384
Reviewed-on: https://chromium-review.googlesource.com/c/1345672
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611493}

chrome/browser/ui/location_bar/location_bar.h

@@ -53,9 +53,6 @@ class LocationBar {
   // Updates the state of the images showing the content settings status.
   virtual void UpdateContentSettingsIcons() = 0;
 
-  // Updates the password icon and pops up a bubble from the icon if needed.
-  virtual void UpdateManagePasswordsIconAndBubble() = 0;
-
   // Updates the visibility and toggled state of the save credit card icon.
   virtual void UpdateSaveCreditCardIcon() = 0;
 

chrome/browser/ui/page_action/page_action_icon_container.h

@@ -9,11 +9,14 @@ enum class PageActionIconType {
   // TODO(https://crbug.com/788051): Migrate page action icon update methods out
   // of LocationBar to this interface.
   kFind,
+  kManagePasswords,
   kZoom,
 };
 
 class PageActionIconContainer {
  public:
+  virtual ~PageActionIconContainer() {}
+
   // Signals a page action icon to update its visual state if it is present in
   // the browser window.
   virtual void UpdatePageActionIcon(PageActionIconType type) = 0;

chrome/browser/ui/passwords/manage_passwords_ui_controller.cc

@@ -23,6 +23,7 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/chrome_pages.h"
 #include "chrome/browser/ui/location_bar/location_bar.h"
+#include "chrome/browser/ui/page_action/page_action_icon_container.h"
 #include "chrome/browser/ui/passwords/manage_passwords_icon_view.h"
 #include "chrome/browser/ui/passwords/manage_passwords_view_utils.h"
 #include "chrome/browser/ui/passwords/password_dialog_controller_impl.h"
@@ -531,9 +532,8 @@ void ManagePasswordsUIController::UpdateBubbleAndIconVisibility() {
   if (!browser)
     return;
 
-  LocationBar* location_bar = browser->window()->GetLocationBar();
-  DCHECK(location_bar);
-  location_bar->UpdateManagePasswordsIconAndBubble();
+  browser->window()->GetPageActionIconContainer()->UpdatePageActionIcon(
+      PageActionIconType::kManagePasswords);
 }
 
 AccountChooserPrompt* ManagePasswordsUIController::CreateAccountChooser(

chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc

@@ -45,6 +45,7 @@
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/exclusive_access/fullscreen_controller.h"
 #include "chrome/browser/ui/exclusive_access/fullscreen_controller_test.h"
+#include "chrome/browser/ui/passwords/passwords_client_ui_delegate.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/toolbar/browser_actions_bar_browsertest.h"
 #include "chrome/browser/ui/views/bookmarks/bookmark_bar_view.h"
@@ -66,6 +67,7 @@
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/account_id/account_id.h"
+#include "components/autofill/core/common/password_form.h"
 #include "components/keep_alive_registry/keep_alive_types.h"
 #include "components/keep_alive_registry/scoped_keep_alive.h"
 #include "content/public/browser/render_frame_host.h"
@@ -912,6 +914,31 @@ IN_PROC_BROWSER_TEST_P(HostedAppNonClientFrameViewAshTest,
             hosted_app_button_container_);
 }
 
+// Test that the manage passwords icon appears in the title bar for hosted app
+// windows.
+IN_PROC_BROWSER_TEST_P(HostedAppNonClientFrameViewAshTest,
+                       ManagePasswordsIcon) {
+  SetUpHostedApp();
+  content::WebContents* web_contents =
+      app_browser_->tab_strip_model()->GetActiveWebContents();
+  PageActionIconView* manage_passwords_icon =
+      GetPageActionIcon(PageActionIconType::kManagePasswords);
+
+  EXPECT_TRUE(manage_passwords_icon);
+  EXPECT_FALSE(manage_passwords_icon->visible());
+
+  autofill::PasswordForm password_form;
+  password_form.username_value = base::ASCIIToUTF16("test");
+  password_form.origin = GetAppURL().GetOrigin();
+  PasswordsClientUIDelegateFromWebContents(web_contents)
+      ->OnPasswordAutofilled({{password_form.username_value, &password_form}},
+                             password_form.origin, nullptr);
+  chrome::ManagePasswordsForPage(app_browser_);
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_TRUE(manage_passwords_icon->visible());
+}
+
 // Test that the zoom icon appears in the title bar for hosted app windows.
 IN_PROC_BROWSER_TEST_P(HostedAppNonClientFrameViewAshTest, ZoomIcon) {
   SetUpHostedApp();

chrome/browser/ui/views/frame/hosted_app_button_container.cc

@@ -7,6 +7,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/task_runner.h"
 #include "base/threading/sequenced_task_runner_handle.h"
+#include "chrome/browser/ui/browser_command_controller.h"
 #include "chrome/browser/ui/browser_content_setting_bubble_model_delegate.h"
 #include "chrome/browser/ui/content_settings/content_setting_image_model.h"
 #include "chrome/browser/ui/extensions/hosted_app_browser_controller.h"
@@ -184,10 +185,12 @@ HostedAppButtonContainer::HostedAppButtonContainer(
       hosted_app_origin_text_(new HostedAppOriginText(browser_view->browser())),
       content_settings_container_(new ContentSettingsContainer(this)),
       page_action_icon_container_view_(new PageActionIconContainerView(
-          {PageActionIconType::kFind, PageActionIconType::kZoom},
+          {PageActionIconType::kManagePasswords, PageActionIconType::kFind,
+           PageActionIconType::kZoom},
           GetLayoutConstant(HOSTED_APP_PAGE_ACTION_ICON_SIZE),
           HorizontalPaddingBetweenItems(),
           browser_view->browser(),
+          browser_view->browser()->command_controller(),
           this,
           nullptr)),
       browser_actions_container_(
@@ -380,6 +383,10 @@ views::AccessiblePaneView* HostedAppButtonContainer::GetAsAccessiblePaneView() {
   return this;
 }
 
+views::View* HostedAppButtonContainer::GetAnchorView() {
+  return app_menu_button_;
+}
+
 void HostedAppButtonContainer::OnWidgetVisibilityChanged(views::Widget* widget,
                                                          bool visibility) {
   if (!visibility || !pending_widget_visibility_)

chrome/browser/ui/views/frame/hosted_app_button_container.h

@@ -116,6 +116,7 @@ class HostedAppButtonContainer : public views::AccessiblePaneView,
   gfx::Rect GetFindBarBoundingBox(int contents_height) const override;
   void FocusToolbar() override;
   views::AccessiblePaneView* GetAsAccessiblePaneView() override;
+  views::View* GetAnchorView() override;
 
   // views::WidgetObserver:
   void OnWidgetVisibilityChanged(views::Widget* widget, bool visible) override;

chrome/browser/ui/views/frame/toolbar_button_provider.h

@@ -15,6 +15,7 @@ class Rect;
 
 namespace views {
 class AccessiblePaneView;
+class View;
 }
 
 // An interface implemented by a view contains and provides access to toolbar
@@ -41,6 +42,9 @@ class ToolbarButtonProvider {
   // Returns the toolbar as an AccessiblePaneView.
   virtual views::AccessiblePaneView* GetAsAccessiblePaneView() = 0;
 
+  // Returns the toolbar as an anchor point.
+  virtual views::View* GetAnchorView() = 0;
+
   // TODO(calamity): Move other buttons and button actions into here.
  protected:
   virtual ~ToolbarButtonProvider() {}

chrome/browser/ui/views/location_bar/location_bar_view.cc

@@ -213,22 +213,20 @@ void LocationBarView::Init() {
   }
 
   std::vector<PageActionIconType> page_action_icon_types;
+  page_action_icon_types.push_back(PageActionIconType::kManagePasswords);
   // |browser_| may be null when LocationBarView is used for non-Browser windows
   // such as PresentationReceiverWindowView, which do not support page actions.
   if (browser_) {
-    page_action_icon_types = {PageActionIconType::kFind,
-                              PageActionIconType::kZoom};
+    page_action_icon_types.push_back(PageActionIconType::kFind);
+    page_action_icon_types.push_back(PageActionIconType::kZoom);
   }
+
   page_action_icon_container_view_ = new PageActionIconContainerView(
       page_action_icon_types, GetLayoutConstant(LOCATION_BAR_ICON_SIZE), 0,
-      browser_, this, delegate_);
+      browser_, command_updater(), this, delegate_);
   AddChildView(page_action_icon_container_view_);
   page_action_icon_container_view_->SetIconColor(icon_color);
 
-  manage_passwords_icon_view_ =
-      new ManagePasswordsIconViews(command_updater(), this);
-  page_action_icons_.push_back(manage_passwords_icon_view_);
-
   if (browser_) {
     save_credit_card_icon_view_ = new autofill::SaveCardIconView(
         command_updater(), browser_, this, font_list);
@@ -389,7 +387,6 @@ gfx::Size LocationBarView::CalculatePreferredSize() const {
   // Compute width of omnibox-trailing content.
   int trailing_width =
       IncrementalMinimumWidth(translate_icon_view_) +
-      IncrementalMinimumWidth(manage_passwords_icon_view_) +
       IncrementalMinimumWidth(page_action_icon_container_view_);
   if (star_view_)
     trailing_width += IncrementalMinimumWidth(star_view_);
@@ -518,7 +515,6 @@ void LocationBarView::Layout() {
     add_trailing_decoration(save_credit_card_icon_view_);
   if (local_card_migration_icon_view_)
     add_trailing_decoration(local_card_migration_icon_view_);
-  add_trailing_decoration(manage_passwords_icon_view_);
   for (ContentSettingViews::const_reverse_iterator i(
            content_setting_views_.rbegin());
        i != content_setting_views_.rend(); ++i) {
@@ -923,13 +919,6 @@ void LocationBarView::UpdateContentSettingsIcons() {
   }
 }
 
-void LocationBarView::UpdateManagePasswordsIconAndBubble() {
-  if (manage_passwords_icon_view_->Update()) {
-    Layout();
-    SchedulePaint();
-  }
-}
-
 void LocationBarView::UpdateSaveCreditCardIcon() {
   if (save_credit_card_icon_view_->Update()) {
     Layout();

chrome/browser/ui/views/location_bar/location_bar_view.h

@@ -43,7 +43,6 @@ class GURL;
 class IntentPickerView;
 class KeywordHintView;
 class LocationIconView;
-class ManagePasswordsIconViews;
 enum class OmniboxPart;
 class OmniboxPopupView;
 enum class OmniboxTint;
@@ -142,11 +141,6 @@ class LocationBarView : public LocationBar,
   // Returns the delegate.
   Delegate* delegate() const { return delegate_; }
 
-  // The passwords icon. It may not be visible.
-  ManagePasswordsIconViews* manage_passwords_icon_view() {
-    return manage_passwords_icon_view_;
-  }
-
   // Toggles the star on or off.
   void SetStarToggled(bool on);
 
@@ -321,7 +315,6 @@ class LocationBarView : public LocationBar,
   void AcceptInput(base::TimeTicks match_selection_timestamp) override;
   void FocusSearch() override;
   void UpdateContentSettingsIcons() override;
-  void UpdateManagePasswordsIconAndBubble() override;
   void UpdateSaveCreditCardIcon() override;
   void UpdateLocalCardMigrationIcon() override;
   void UpdateBookmarkStarVisibility() override;
@@ -409,9 +402,6 @@ class LocationBarView : public LocationBar,
   // The page action icons.
   PageActionIconContainerView* page_action_icon_container_view_ = nullptr;
 
-  // The manage passwords icon.
-  ManagePasswordsIconViews* manage_passwords_icon_view_ = nullptr;
-
   // The save credit card icon.  It will be null when |browser_| is null.
   autofill::SaveCardIconView* save_credit_card_icon_view_ = nullptr;
 

chrome/browser/ui/views/page_action/page_action_icon_container_view.cc

@@ -8,6 +8,7 @@
 #include "chrome/browser/ui/views/location_bar/find_bar_icon.h"
 #include "chrome/browser/ui/views/location_bar/zoom_bubble_view.h"
 #include "chrome/browser/ui/views/page_action/zoom_view.h"
+#include "chrome/browser/ui/views/passwords/manage_passwords_icon_views.h"
 #include "ui/views/layout/box_layout.h"
 
 PageActionIconContainerView::PageActionIconContainerView(
@@ -15,6 +16,7 @@ PageActionIconContainerView::PageActionIconContainerView(
     int icon_size,
     int between_icon_spacing,
     Browser* browser,
+    CommandUpdater* command_updater,
     PageActionIconView::Delegate* page_action_icon_delegate,
     LocationBarView::Delegate* location_bar_delegate)
     : zoom_observer_(this) {
@@ -30,6 +32,11 @@ PageActionIconContainerView::PageActionIconContainerView(
         find_bar_icon_ = new FindBarIcon(browser, page_action_icon_delegate);
         page_action_icons_.push_back(find_bar_icon_);
         break;
+      case PageActionIconType::kManagePasswords:
+        manage_passwords_icon_ = new ManagePasswordsIconViews(
+            command_updater, page_action_icon_delegate);
+        page_action_icons_.push_back(manage_passwords_icon_);
+        break;
       case PageActionIconType::kZoom:
         zoom_view_ =
             new ZoomView(location_bar_delegate, page_action_icon_delegate);
@@ -61,6 +68,8 @@ PageActionIconView* PageActionIconContainerView::GetPageActionIconView(
   switch (type) {
     case PageActionIconType::kFind:
       return find_bar_icon_;
+    case PageActionIconType::kManagePasswords:
+      return manage_passwords_icon_;
     case PageActionIconType::kZoom:
       return zoom_view_;
   }

chrome/browser/ui/views/page_action/page_action_icon_container_view.h

@@ -15,7 +15,9 @@
 #include "ui/views/view.h"
 
 class Browser;
+class CommandUpdater;
 class FindBarIcon;
+class ManagePasswordsIconViews;
 class ZoomView;
 
 class PageActionIconContainerView : public views::View,
@@ -27,6 +29,7 @@ class PageActionIconContainerView : public views::View,
       int icon_size,
       int between_icon_spacing,
       Browser* browser,
+      CommandUpdater* command_updater,
       PageActionIconView::Delegate* page_action_icon_delegate,
       LocationBarView::Delegate* location_bar_delegate);
   ~PageActionIconContainerView() override;
@@ -60,6 +63,7 @@ class PageActionIconContainerView : public views::View,
 
   ZoomView* zoom_view_ = nullptr;
   FindBarIcon* find_bar_icon_ = nullptr;
+  ManagePasswordsIconViews* manage_passwords_icon_ = nullptr;
   std::vector<PageActionIconView*> page_action_icons_;
 
   ScopedObserver<zoom::ZoomEventManager, zoom::ZoomEventManagerObserver>

chrome/browser/ui/views/passwords/manage_passwords_icon_view_interactive_uitest.cc

@@ -7,6 +7,7 @@
 #include "chrome/browser/ui/passwords/manage_passwords_test.h"
 #include "chrome/browser/ui/passwords/manage_passwords_ui_controller_mock.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/ui/views/page_action/page_action_icon_container_view.h"
 #include "chrome/browser/ui/views/passwords/manage_passwords_icon_views.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/grit/generated_resources.h"
@@ -24,10 +25,13 @@ class ManagePasswordsIconViewTest : public ManagePasswordsTest {
   password_manager::ui::State ViewState() { return GetView()->state_; }
 
   ManagePasswordsIconViews* GetView() {
-    return BrowserView::GetBrowserViewForBrowser(browser())
-        ->toolbar()
-        ->location_bar()
-        ->manage_passwords_icon_view();
+    views::View* view =
+        BrowserView::GetBrowserViewForBrowser(browser())
+            ->toolbar_button_provider()
+            ->GetPageActionIconContainerView()
+            ->GetPageActionIconView(PageActionIconType::kManagePasswords);
+    DCHECK_EQ(view->GetClassName(), ManagePasswordsIconViews::kClassName);
+    return static_cast<ManagePasswordsIconViews*>(view);
   }
 
   base::string16 GetTooltipText() {

chrome/browser/ui/views/passwords/manage_passwords_icon_views.cc

@@ -14,6 +14,8 @@
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 
+const char ManagePasswordsIconViews::kClassName[] = "ManagePasswordsIconViews";
+
 ManagePasswordsIconViews::ManagePasswordsIconViews(
     CommandUpdater* updater,
     PageActionIconView::Delegate* delegate)
@@ -108,3 +110,7 @@ void ManagePasswordsIconViews::AboutToRequestFocusFromTabTraversal(
   if (IsBubbleShowing())
     PasswordBubbleViewBase::ActivateBubble();
 }
+
+const char* ManagePasswordsIconViews::GetClassName() const {
+  return kClassName;
+}

chrome/browser/ui/views/passwords/manage_passwords_icon_views.h

@@ -18,6 +18,8 @@ class CommandUpdater;
 class ManagePasswordsIconViews : public ManagePasswordsIconView,
                                  public PageActionIconView {
  public:
+  static const char kClassName[];
+
   ManagePasswordsIconViews(CommandUpdater* updater,
                            PageActionIconView::Delegate* delegate);
   ~ManagePasswordsIconViews() override;
@@ -36,6 +38,7 @@ class ManagePasswordsIconViews : public ManagePasswordsIconView,
 
   // views::View:
   void AboutToRequestFocusFromTabTraversal(bool reverse) override;
+  const char* GetClassName() const override;
 
  private:
   friend class ManagePasswordsIconViewTest;

chrome/browser/ui/views/passwords/password_bubble_view_base.cc

@@ -8,7 +8,9 @@
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/passwords/passwords_model_delegate.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/ui/views/frame/toolbar_button_provider.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
+#include "chrome/browser/ui/views/page_action/page_action_icon_container_view.h"
 #include "chrome/browser/ui/views/passwords/manage_passwords_icon_views.h"
 #include "chrome/browser/ui/views/passwords/password_auto_sign_in_view.h"
 #include "chrome/browser/ui/views/passwords/password_items_view.h"
@@ -29,7 +31,8 @@ void PasswordBubbleViewBase::ShowBubble(content::WebContents* web_contents,
          !g_manage_passwords_bubble_->GetWidget()->IsVisible());
 
   BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser);
-  views::View* const anchor_view = browser_view->GetLocationBarView();
+  views::View* const anchor_view =
+      browser_view->toolbar_button_provider()->GetAnchorView();
 
   PasswordBubbleViewBase* bubble =
       CreateBubble(web_contents, anchor_view, gfx::Point(), reason);
@@ -38,7 +41,9 @@ void PasswordBubbleViewBase::ShowBubble(content::WebContents* web_contents,
 
   if (anchor_view) {
     g_manage_passwords_bubble_->SetHighlightedButton(
-        browser_view->GetLocationBarView()->manage_passwords_icon_view());
+        browser_view->toolbar_button_provider()
+            ->GetPageActionIconContainerView()
+            ->GetPageActionIconView(PageActionIconType::kManagePasswords));
   } else {
     g_manage_passwords_bubble_->set_parent_window(
         web_contents->GetNativeView());

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

@@ -755,6 +755,10 @@ views::AccessiblePaneView* ToolbarView::GetAsAccessiblePaneView() {
   return this;
 }
 
+views::View* ToolbarView::GetAnchorView() {
+  return location_bar_;
+}
+
 BrowserRootView::DropIndex ToolbarView::GetDropIndex(
     const ui::DropTargetEvent& event) {
   return {browser_->tab_strip_model()->active_index(), false};

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

@@ -214,6 +214,7 @@ class ToolbarView : public views::AccessiblePaneView,
   gfx::Rect GetFindBarBoundingBox(int contents_height) const override;
   void FocusToolbar() override;
   views::AccessiblePaneView* GetAsAccessiblePaneView() override;
+  views::View* GetAnchorView() override;
 
   // BrowserRootView::DropTarget
   BrowserRootView::DropIndex GetDropIndex(

chrome/test/base/test_browser_window.cc

@@ -137,7 +137,7 @@ LocationBar* TestBrowserWindow::GetLocationBar() const {
 }
 
 PageActionIconContainer* TestBrowserWindow::GetPageActionIconContainer() {
-  return nullptr;
+  return &page_action_icon_container_;
 }
 
 ToolbarActionsBar* TestBrowserWindow::GetToolbarActionsBar() {

chrome/test/base/test_browser_window.h

@@ -194,7 +194,6 @@ class TestBrowserWindow : public BrowserWindow {
     void FocusLocation(bool select_all) override {}
     void FocusSearch() override {}
     void UpdateContentSettingsIcons() override {}
-    void UpdateManagePasswordsIconAndBubble() override {}
     void UpdateSaveCreditCardIcon() override {}
     void UpdateLocalCardMigrationIcon() override {}
     void UpdateBookmarkStarVisibility() override {}
@@ -208,8 +207,21 @@ class TestBrowserWindow : public BrowserWindow {
     DISALLOW_COPY_AND_ASSIGN(TestLocationBar);
   };
 
+  class TestPageActionIconContainer : public PageActionIconContainer {
+   public:
+    TestPageActionIconContainer() {}
+    ~TestPageActionIconContainer() override {}
+
+    // PageActionIconContainer:
+    void UpdatePageActionIcon(PageActionIconType type) override {}
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(TestPageActionIconContainer);
+  };
+
   TestDownloadShelf download_shelf_;
   TestLocationBar location_bar_;
+  TestPageActionIconContainer page_action_icon_container_;
 
   DISALLOW_COPY_AND_ASSIGN(TestBrowserWindow);
 };