[OriginChip] Show the page info bubble on the location bar icon.

chrome/browser/ui/views/location_bar/location_icon_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_icon_view.h"
 
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
 #include "grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 
 LocationIconView::LocationIconView(LocationBarView* location_bar)
-    : page_info_helper_(this, location_bar) {
+    : page_info_helper_(this, location_bar),
+      location_bar_(location_bar) {
   SetTooltipText(l10n_util::GetStringUTF16(IDS_TOOLTIP_LOCATION_ICON));
   LocationBarView::InitTouchableLocationBarChildView(this);
 }
 
 LocationIconView::~LocationIconView() {
 }
 
 bool LocationIconView::OnMousePressed(const ui::MouseEvent& event) {
   // We want to show the dialog on mouse release; that is the standard behavior
   // for buttons.
   return true;
 }
 
 void LocationIconView::OnMouseReleased(const ui::MouseEvent& event) {
-  if (!chrome::ShouldDisplayOriginChip() &&
-      !chrome::ShouldDisplayOriginChipV2())
-    page_info_helper_.ProcessEvent(event);
+  // Do not show page info if the user has been editing the location bar or the
+  // location bar is at the NTP.  Also skip showing the page info if the
+  // toolbar-based origin chip is being shown because it is responsible for
+  // showing the page info instead.
+  if (location_bar_->GetOmniboxView()->IsEditingOrEmpty() ||

[groby-ooo-7-16, 2014/03/12 19:53:46]

Random drive-by-question: Why not handle this in |page_info_helper_|?

[Peter Kasting, 2014/03/12 21:11:27]

Yeah, it's not clear to me why the code like this got moved from one class to
the other.

[Justin Donnelly, 2014/03/14 15:27:51]

PageInfoHelper is used in 3 places:
- The location icon in the location bar.
- The location icon in the origin chip.
- The EV bubble in the location bar.

The latter two don't care whether the omnibox is empty or being edited.  When
they appear they always represent the current content.  Having the check in
PageInfoHelper didn't allow for 1. always succeeding for the new origin chip and
2. conditionally succeeeding for the omnibox location icon, depending on
IsEditingOrEmpty().

So I moved the check here, which is the code path used by (and only by) the
location omnibox location icon.

+      chrome::ShouldDisplayOriginChip())
+    return;
+
+  page_info_helper_.ProcessEvent(event);
 }
 
 void LocationIconView::OnGestureEvent(ui::GestureEvent* event) {
-  if (!chrome::ShouldDisplayOriginChip() &&
-      !chrome::ShouldDisplayOriginChipV2() &&
-      (event->type() == ui::ET_GESTURE_TAP)) {
-    page_info_helper_.ProcessEvent(*event);
-    event->SetHandled();
-  }
+  // Do not show page info if the user has been editing the location bar or the
+  // location bar is at the NTP.  Also skip showing the page info if the
+  // toolbar-based origin chip is being shown because it is responsible for
+  // showing the page info instead.
+  if (location_bar_->GetOmniboxView()->IsEditingOrEmpty() ||
+      chrome::ShouldDisplayOriginChip() ||
+      (event->type() != ui::ET_GESTURE_TAP))
+    return;
+
+  page_info_helper_.ProcessEvent(*event);
+  event->SetHandled();
 }
 
 void LocationIconView::ShowTooltip(bool show) {
   SetTooltipText(show ?
       l10n_util::GetStringUTF16(IDS_TOOLTIP_LOCATION_ICON) : base::string16());
 }