Close bubbles not on UpdateToolbar but when the activated tab is changed

chrome/browser/ui/views/location_bar/location_bar_view.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/views/location_bar/location_bar_view.h"
 
 #include <algorithm>
 #include <map>
 
 #include "base/i18n/rtl.h"
@@ skipping 953 matching lines @@
 ////////////////////////////////////////////////////////////////////////////////
 // LocationBarView, public OmniboxEditController implementation:
 
 void LocationBarView::Update(const WebContents* contents) {
   mic_search_view_->SetVisible(
       !GetToolbarModel()->input_in_progress() && browser_ &&
       browser_->search_model()->voice_search_supported());
   RefreshContentSettingViews();
   generated_credit_card_view_->Update();
   ZoomBubbleView::CloseBubble();
-  TranslateBubbleView::CloseBubble();

[Peter Kasting, 2014/11/19 09:18:12]

What about the other statements here, e.g. ZoomBubbleView::CloseBubble()? 
Wouldn't that be wrong too?  Can the other updating functions here do the wrong
things as well?

[hajimehoshi, 2014/11/19 10:02:08]

As to the zoom bubble, maybe yes, and I can update this. I'm not sure about the
other functions.

I said 'maybe' because you said this was the right place to close the zoom
bubble before (https://codereview.chromium.org/425223003). But if my findings
are right and new to you, we should move updating the zoom bubble.

BTW, being different from the Translate bubble, the zoom bubble is closed after
several seconds automatically in some cases, so I suppose users don't notice
that the zoom bubble is closed by non-navigation events.

   RefreshZoomView();
   RefreshPageActionViews();
   RefreshTranslateIcon();
   RefreshManagePasswordsIconView();
   content::WebContents* web_contents_for_sub_views =
       GetToolbarModel()->input_in_progress() ? NULL : GetWebContents();
   open_pdf_in_reader_view_->Update(web_contents_for_sub_views);
 
   if (star_view_)
     UpdateBookmarkStarVisibility();
@@ skipping 161 matching lines @@
 
   WebContents* web_contents = GetWebContents();
   if (!web_contents)
     return;
   translate::LanguageState& language_state =
       ChromeTranslateClient::FromWebContents(web_contents)->GetLanguageState();
   bool enabled = language_state.translate_enabled();
   command_updater()->UpdateCommandEnabled(IDC_TRANSLATE_PAGE, enabled);
   translate_icon_view_->SetVisible(enabled);
   translate_icon_view_->SetToggled(language_state.IsPageTranslated());
+
+  if (!enabled)
+    TranslateBubbleView::CloseBubble();

[Peter Kasting, 2014/11/19 09:18:12]

Does this close the bubble when the main frame is navigated?  (I presume yes,
otherwise main frame navigations wouldn't close the bubble at all, which seems
wrong)

[hajimehoshi, 2014/11/19 10:02:08]

Yes. When the main frame is navigated, |enabled| should be false.

 }
 
 bool LocationBarView::RefreshManagePasswordsIconView() {
   DCHECK(manage_passwords_icon_view_);
   WebContents* web_contents = GetWebContents();
   if (!web_contents)
     return false;
   const bool was_visible = manage_passwords_icon_view_->visible();
   ManagePasswordsUIController::FromWebContents(
       web_contents)->UpdateIconAndBubbleState(manage_passwords_icon_view_);
@@ skipping 490 matching lines @@
 
 void LocationBarView::ModelChanged(const SearchModel::State& old_state,
                                    const SearchModel::State& new_state) {
   const bool visible = !GetToolbarModel()->input_in_progress() &&
       new_state.voice_search_supported;
   if (mic_search_view_->visible() != visible) {
     mic_search_view_->SetVisible(visible);
     Layout();
   }
 }