Shows the info bubble when the location bar icon is clicked in the origin chip.

chrome/browser/ui/toolbar/toolbar_model_impl.cc

 // Copyright 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/toolbar/toolbar_model_impl.h"
 
 #include "base/command_line.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/autocomplete/autocomplete_classifier.h"
@@ skipping 33 matching lines @@
 using content::SSLStatus;
 using content::WebContents;
 
 ToolbarModelImpl::ToolbarModelImpl(ToolbarModelDelegate* delegate)
     : delegate_(delegate) {
 }
 
 ToolbarModelImpl::~ToolbarModelImpl() {
 }
 
+// static
 ToolbarModel::SecurityLevel ToolbarModelImpl::GetSecurityLevelForWebContents(
       content::WebContents* web_contents) {
   if (!web_contents)
     return NONE;
 
   NavigationEntry* entry = web_contents->GetController().GetVisibleEntry();
   if (!entry)
     return NONE;
 
   const SSLStatus& ssl = entry->GetSSL();
@@ skipping 23 matching lines @@
           content::CertStore::GetInstance()->RetrieveCert(ssl.cert_id, NULL))
         return EV_SECURE;
       return SECURE;
     }
     default:
       NOTREACHED();
       return NONE;
   }
 }
 
+// static
+base::string16 ToolbarModelImpl::GetEVCertName(
+    const net::X509Certificate& cert) {
+  // EV are required to have an organization name and country.
+  if (cert.subject().organization_names.empty() ||
+      cert.subject().country_name.empty()) {
+    NOTREACHED();

[Peter Kasting, 2014/05/09 20:35:40]

Nit: This violates the Chrome style guide rule about not handling DCHECK
failure.  This should just be:

  DCHECK(!cert.subject().organization_names.empty());
  DCHECK(!cert.subject().country_name.empty());

(Unless for some reason these really can be empty, in which case we should
remove the NOTREACHED() instead.  I wouldn't do that unless you know you need
to.)

[macourteau, 2014/05/09 20:53:23]

Isn't the behavior of the two DCHECK's different though? I'm not the author of
this code, but what I see here is that in debug, it will trigger the NOTREACHED,
but in release it will log an error and return an empty string.

With the two DCHECK's, on the other hand, the debug behavior would be the same,
but control would reach the GetStringFUTF16 call in release, which is different
from returning an empty string (and might crash due to accessing index 0 of
organization_name, if it's empty).

[Peter Kasting, 2014/05/09 21:15:47]

NOTREACHED/DCHECK is an assertion that the check in question cannot possibly
fail, ever, in any mode, even for unusual reasons (e.g. disk corruption or
out-of-memory should still not trigger DCHECK failures).

Code is not supposed to speculate about what might happen if the assertion is
violated, and how to handle that.  In other words, and code that says, "but in
release mode, if this fails, then" is wrong, because the entire point of a
NOTREACHED/DCHECK is to say that the conditional literally never fails.

[macourteau, 2014/05/09 22:23:51]

SGTM. Done.

+    return base::string16();
+  }
+
+  return l10n_util::GetStringFUTF16(
+      IDS_SECURE_CONNECTION_EV,
+      base::UTF8ToUTF16(cert.subject().organization_names[0]),
+      base::UTF8ToUTF16(cert.subject().country_name));
+}
+
 // ToolbarModelImpl Implementation.
 base::string16 ToolbarModelImpl::GetText() const {
   base::string16 search_terms(GetSearchTerms(false));
   if (!search_terms.empty())
     return search_terms;
 
   if (WouldOmitURLDueToOriginChip())
     return base::string16();
 
   return GetFormattedURL();
@@ skipping 40 matching lines @@
   const NavigationController* navigation_controller = GetNavigationController();
   if (navigation_controller) {
     const NavigationEntry* entry = navigation_controller->GetVisibleEntry();
     if (entry)
       return ShouldDisplayURL() ? entry->GetVirtualURL() : GURL();
   }
 
   return GURL(content::kAboutBlankURL);
 }
 
-bool ToolbarModelImpl::WouldOmitURLDueToOriginChip() const {
-  const char kInterstitialShownKey[] = "interstitial_shown";
-
-  // When users type URLs and hit enter, continue to show those URLs until
-  // the navigation commits or an interstitial is shown, because having the
-  // omnibox clear immediately feels like the input was ignored.
-  NavigationController* navigation_controller = GetNavigationController();
-  if (navigation_controller) {
-    NavigationEntry* pending_entry = navigation_controller->GetPendingEntry();
-    if (pending_entry) {
-      const NavigationEntry* visible_entry =
-          navigation_controller->GetVisibleEntry();
-      base::string16 unused;
-      // Keep track that we've shown the origin chip on an interstitial so it
-      // can be shown even after the interstitial was dismissed, to avoid
-      // showing the chip, removing it and then showing it again.
-      if (visible_entry &&
-          visible_entry->GetPageType() == content::PAGE_TYPE_INTERSTITIAL &&
-          !pending_entry->GetExtraData(kInterstitialShownKey, &unused))
-        pending_entry->SetExtraData(kInterstitialShownKey, base::string16());
-      const content::PageTransition transition_type =
-          pending_entry->GetTransitionType();
-      if ((transition_type & content::PAGE_TRANSITION_TYPED) != 0 &&
-          !pending_entry->GetExtraData(kInterstitialShownKey, &unused))
-        return false;
-    }
-  }
-
-  bool should_display_origin_chip =
-      chrome::ShouldDisplayOriginChip() || chrome::ShouldDisplayOriginChipV2();
-  return should_display_origin_chip && delegate_->InTabbedBrowser() &&
-      ShouldDisplayURL() && url_replacement_enabled();
-}
-
 bool ToolbarModelImpl::WouldPerformSearchTermReplacement(
     bool ignore_editing) const {
   return !GetSearchTerms(ignore_editing).empty();
 }
 
-bool ToolbarModelImpl::ShouldDisplayURL() const {
-  // Note: The order here is important.
-  // - The WebUI test must come before the extension scheme test because there
-  //   can be WebUIs that have extension schemes (e.g. the bookmark manager). In
-  //   that case, we should prefer what the WebUI instance says.
-  // - The view-source test must come before the NTP test because of the case
-  //   of view-source:chrome://newtab, which should display its URL despite what
-  //   chrome://newtab says.
-  NavigationController* controller = GetNavigationController();
-  NavigationEntry* entry = controller ? controller->GetVisibleEntry() : NULL;
-  if (entry) {
-    if (entry->IsViewSourceMode() ||
-        entry->GetPageType() == content::PAGE_TYPE_INTERSTITIAL) {
-      return true;
-    }
-
-    GURL url = entry->GetURL();
-    GURL virtual_url = entry->GetVirtualURL();
-    if (url.SchemeIs(content::kChromeUIScheme) ||
-        virtual_url.SchemeIs(content::kChromeUIScheme)) {
-      if (!url.SchemeIs(content::kChromeUIScheme))
-        url = virtual_url;
-      return url.host() != chrome::kChromeUINewTabHost;
-    }
-  }
-
-  if (chrome::IsInstantNTP(delegate_->GetActiveWebContents()))
-    return false;
-
-  return true;
-}
-
 ToolbarModel::SecurityLevel ToolbarModelImpl::GetSecurityLevel(
     bool ignore_editing) const {
   // When editing, assume no security style.
   return (input_in_progress() && !ignore_editing) ?
       NONE : GetSecurityLevelForWebContents(delegate_->GetActiveWebContents());
 }
 
 int ToolbarModelImpl::GetIcon() const {
   if (WouldPerformSearchTermReplacement(false)) {
     // The secured version of the search icon is necessary if neither the search
@@ skipping 28 matching lines @@
 base::string16 ToolbarModelImpl::GetEVCertName() const {
   DCHECK_EQ(EV_SECURE, GetSecurityLevel(false));
   scoped_refptr<net::X509Certificate> cert;
   // Note: Navigation controller and active entry are guaranteed non-NULL or
   // the security level would be NONE.
   content::CertStore::GetInstance()->RetrieveCert(
       GetNavigationController()->GetVisibleEntry()->GetSSL().cert_id, &cert);
   return GetEVCertName(*cert.get());
 }
 
-// static
-base::string16 ToolbarModelImpl::GetEVCertName(
-    const net::X509Certificate& cert) {
-  // EV are required to have an organization name and country.
-  if (cert.subject().organization_names.empty() ||
-      cert.subject().country_name.empty()) {
-    NOTREACHED();
-    return base::string16();
+bool ToolbarModelImpl::ShouldDisplayURL() const {
+  // Note: The order here is important.
+  // - The WebUI test must come before the extension scheme test because there
+  //   can be WebUIs that have extension schemes (e.g. the bookmark manager). In
+  //   that case, we should prefer what the WebUI instance says.
+  // - The view-source test must come before the NTP test because of the case
+  //   of view-source:chrome://newtab, which should display its URL despite what
+  //   chrome://newtab says.
+  NavigationController* controller = GetNavigationController();
+  NavigationEntry* entry = controller ? controller->GetVisibleEntry() : NULL;
+  if (entry) {
+    if (entry->IsViewSourceMode() ||
+        entry->GetPageType() == content::PAGE_TYPE_INTERSTITIAL) {
+      return true;
+    }
+
+    GURL url = entry->GetURL();
+    GURL virtual_url = entry->GetVirtualURL();
+    if (url.SchemeIs(content::kChromeUIScheme) ||
+        virtual_url.SchemeIs(content::kChromeUIScheme)) {
+      if (!url.SchemeIs(content::kChromeUIScheme))
+        url = virtual_url;
+      return url.host() != chrome::kChromeUINewTabHost;
+    }
   }
 
-  return l10n_util::GetStringFUTF16(
-      IDS_SECURE_CONNECTION_EV,
-      base::UTF8ToUTF16(cert.subject().organization_names[0]),
-      base::UTF8ToUTF16(cert.subject().country_name));
+  if (chrome::IsInstantNTP(delegate_->GetActiveWebContents()))

[Peter Kasting, 2014/05/09 20:35:40]

Nit: Just:

  return !chrome::IsInstantNTP(delegate_->GetActiveWebContents());

[macourteau, 2014/05/09 20:53:23]

Done.

+    return false;
+
+  return true;
+}
+
+bool ToolbarModelImpl::WouldOmitURLDueToOriginChip() const {
+  const char kInterstitialShownKey[] = "interstitial_shown";
+
+  // When users type URLs and hit enter, continue to show those URLs until
+  // the navigation commits or an interstitial is shown, because having the
+  // omnibox clear immediately feels like the input was ignored.
+  NavigationController* navigation_controller = GetNavigationController();
+  if (navigation_controller) {
+    NavigationEntry* pending_entry = navigation_controller->GetPendingEntry();
+    if (pending_entry) {
+      const NavigationEntry* visible_entry =
+          navigation_controller->GetVisibleEntry();
+      base::string16 unused;
+      // Keep track that we've shown the origin chip on an interstitial so it
+      // can be shown even after the interstitial was dismissed, to avoid
+      // showing the chip, removing it and then showing it again.
+      if (visible_entry &&
+          visible_entry->GetPageType() == content::PAGE_TYPE_INTERSTITIAL &&
+          !pending_entry->GetExtraData(kInterstitialShownKey, &unused))
+        pending_entry->SetExtraData(kInterstitialShownKey, base::string16());
+      const content::PageTransition transition_type =
+          pending_entry->GetTransitionType();
+      if ((transition_type & content::PAGE_TRANSITION_TYPED) != 0 &&
+          !pending_entry->GetExtraData(kInterstitialShownKey, &unused))
+        return false;
+    }
+  }
+
+  bool should_display_origin_chip =
+      chrome::ShouldDisplayOriginChip() || chrome::ShouldDisplayOriginChipV2();
+  return should_display_origin_chip && delegate_->InTabbedBrowser() &&
+      ShouldDisplayURL() && url_replacement_enabled();
 }
 
 NavigationController* ToolbarModelImpl::GetNavigationController() const {
   // This |current_tab| can be NULL during the initialization of the
   // toolbar during window creation (i.e. before any tabs have been added
   // to the window).
   WebContents* current_tab = delegate_->GetActiveWebContents();
   return current_tab ? &current_tab->GetController() : NULL;
 }
 
@@ skipping 32 matching lines @@
   if (entry &&
       google_util::StartsWithCommandLineGoogleBaseURL(entry->GetVirtualURL()))
     return search_terms;
 
   // Otherwise, extract search terms for HTTPS pages that do not have a security
   // error.
   ToolbarModel::SecurityLevel security_level = GetSecurityLevel(ignore_editing);
   return ((security_level == NONE) || (security_level == SECURITY_ERROR)) ?
       base::string16() : search_terms;
 }