Instant Extended: Add prominent search term support

chrome/browser/ui/toolbar/toolbar_model_impl.cc

Index: chrome/browser/ui/toolbar/toolbar_model_impl.cc
diff --git a/chrome/browser/ui/toolbar/toolbar_model_impl.cc b/chrome/browser/ui/toolbar/toolbar_model_impl.cc
index fb6d815ac947f7fa577751bcd0bb371de4ca8230..429fc0112953cf4cbbdfdec2f79ac627cf885b80 100644
--- a/chrome/browser/ui/toolbar/toolbar_model_impl.cc
+++ b/chrome/browser/ui/toolbar/toolbar_model_impl.cc
@@ -41,7 +41,8 @@ using content::WebContents;
 
 ToolbarModelImpl::ToolbarModelImpl(ToolbarModelDelegate* delegate)
     : delegate_(delegate),
-      input_in_progress_(false) {
+      input_in_progress_(false),
+      is_prominent_search_term_ui_supported_(false) {
 }
 
 ToolbarModelImpl::~ToolbarModelImpl() {
@@ -87,7 +88,8 @@ ToolbarModel::SecurityLevel ToolbarModelImpl::GetSecurityLevelForWebContents(
 string16 ToolbarModelImpl::GetText(
     bool display_search_urls_as_search_terms) const {
   if (display_search_urls_as_search_terms) {
-    string16 search_terms = GetSearchTerms();
+    string16 search_terms;
+    GetSearchTerms(&search_terms);
     if (!search_terms.empty())
       return search_terms;
   }
@@ -108,7 +110,7 @@ string16 ToolbarModelImpl::GetText(
 }
 
 string16 ToolbarModelImpl::GetCorpusNameForMobile() const {
-  if (!WouldReplaceSearchURLWithSearchTerms())
+  if (GetSearchTermType() == SEARCH_TERM_NONE)
     return string16();
   GURL url(GetURL());
   // If there is a query in the url fragment look for the corpus name there,
@@ -139,8 +141,12 @@ GURL ToolbarModelImpl::GetURL() const {
   return GURL(chrome::kAboutBlankURL);
 }
 
-bool ToolbarModelImpl::WouldReplaceSearchURLWithSearchTerms() const {
-  return !GetSearchTerms().empty();
+ToolbarModel::SearchTermType ToolbarModelImpl::GetSearchTermType() const {
+  return GetSearchTerms(NULL);
+}
+
+void ToolbarModelImpl::SetIsProminentSearchTermUISupported(bool value) {
+  is_prominent_search_term_ui_supported_ = value;
 }
 
 bool ToolbarModelImpl::ShouldDisplayURL() const {
@@ -183,8 +189,13 @@ ToolbarModel::SecurityLevel ToolbarModelImpl::GetSecurityLevel() const {
 }
 
 int ToolbarModelImpl::GetIcon() const {
-  if (WouldReplaceSearchURLWithSearchTerms())
+  SearchTermType search_term_type = GetSearchTermType();
+  SecurityLevel security_level = GetSecurityLevel();
+  bool is_secure = security_level == EV_SECURE || security_level == SECURE;
+  if (search_term_type == SEARCH_TERM_NORMAL ||
+      (search_term_type == SEARCH_TERM_PROMINENT && is_secure))

[Peter Kasting, 2013/04/25 19:42:54]

Presumably these conditions are so we display a lock-with-warning-overlay in the
mixed-content case?  Is that the desired UX?  I didn't see anything about it on
the bug.

[sail, 2013/04/25 22:18:46]

It's linked in the bug. Here's a direct link:
https://docs.google.com/a/google.com/folder/d/0B7Vi6tBJGkUeVW9Sa1lra3dZU0U/ed...

     return IDR_OMNIBOX_SEARCH;
+
   static int icon_ids[NUM_SECURITY_LEVELS] = {
     IDR_LOCATION_BAR_HTTP,
     IDR_OMNIBOX_HTTPS_VALID,
@@ -193,7 +204,7 @@ int ToolbarModelImpl::GetIcon() const {
     IDR_OMNIBOX_HTTPS_INVALID,
   };
   DCHECK(arraysize(icon_ids) == NUM_SECURITY_LEVELS);
-  return icon_ids[GetSecurityLevel()];
+  return icon_ids[security_level];
 }
 
 string16 ToolbarModelImpl::GetEVCertName() const {
@@ -244,23 +255,34 @@ Profile* ToolbarModelImpl::GetProfile() const {
       NULL;
 }
 
-string16 ToolbarModelImpl::GetSearchTerms() const {
-  const WebContents* contents = delegate_->GetActiveWebContents();
-  string16 search_terms = chrome::GetSearchTerms(contents);
-
-  // Don't extract search terms that the omnibox would treat as a navigation.
-  // This might confuse users into believing that the search terms were the
-  // URL of the current page, and could cause problems if users hit enter in
-  // the omnibox expecting to reload the page.
-  if (!search_terms.empty()) {
+ToolbarModel::SearchTermType ToolbarModelImpl::GetSearchTerms(
+    string16* search_terms_out) const {
+  string16 search_terms =

[Peter Kasting, 2013/04/25 19:42:54]

Shorter, simpler implementation.

(1) Replace GetSearchTerms() with a private one-arg function GetSearchTermType()
(in addition to the public, no-arg version).  Implementation follows:

ToolbarModel::SearchTermType ToolbarModelImpl::GetSearchTermType(
    const string16& search_terms) const {
  if (search_terms.empty())
    return SEARCH_TERM_NONE;
  AutocompleteMatch match;
  AutocompleteClassifierFactory::GetForProfile(Profile::FromBrowserContext(
      delegate_->GetActiveWebContents()->GetBrowserContext()))->Classify(
          search_terms, false, false, &match, NULL);
  ToolbarModel::SecurityLevel security_level = GetSecurityLevel();
  if (AutocompleteMatch::IsSearchType(match.type) &&
      ((security_level == SECURE) || (security_level == EV_SECURE)))
    return SEARCH_TERM_NORMAL;
  return is_prominent_search_term_ui_supported_ ?
      SEARCH_TERM_PROMINENT : SEARCH_TERM_NONE;
}

(2) Change GetText() to use this:

string16 ToolbarModelImpl::GetText(
    bool display_search_urls_as_search_terms) const {
  if (display_search_urls_as_search_terms) {
    string16 search_terms(
        chrome::GetSearchTerms(delegate_->GetActiveWebContents()));
    if (GetSearchTermType(search_terms) != SEARCH_TERM_NONE)
      return search_terms;
  }
  ...

(3) Change the public GetSearchTermType() to use this:

ToolbarModel::SearchTermType ToolbarModelImpl::GetSearchTermType() const {
  return GetSearchTermType(
      chrome::GetSearchTerms(delegate_->GetActiveWebContents()));
}

[sail, 2013/04/25 22:18:46]

Done.
Cool, that's even better.

+      chrome::GetSearchTerms(delegate_->GetActiveWebContents());
+  SearchTermType type = SEARCH_TERM_NORMAL;
+
+  ToolbarModel::SecurityLevel security_level = GetSecurityLevel();
+  if (search_terms.empty()) {
+    type = SEARCH_TERM_NONE;
+  } else if (security_level != EV_SECURE && security_level != SECURE) {
+    type = SEARCH_TERM_PROMINENT;
+  } else {
     AutocompleteMatch match;
-    Profile* profile =
-        Profile::FromBrowserContext(contents->GetBrowserContext());
+    Profile* profile = Profile::FromBrowserContext(
+        delegate_->GetActiveWebContents()->GetBrowserContext());
     AutocompleteClassifierFactory::GetForProfile(profile)->Classify(
         search_terms, false, false, &match, NULL);
     if (!AutocompleteMatch::IsSearchType(match.type))
-      search_terms.clear();
+      type = SEARCH_TERM_PROMINENT;
+  }
+
+  if (type == SEARCH_TERM_PROMINENT &&
+      !is_prominent_search_term_ui_supported_) {
+    type = SEARCH_TERM_NONE;
+    search_terms.clear();
   }
 
-  return search_terms;
+  if (search_terms_out)
+    *search_terms_out = search_terms;
+  return type;
 }