Send URLs on non-zero prefix suggest requests also.

chrome/browser/autocomplete/search_provider.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/autocomplete/search_provider.h"
 
 #include <algorithm>
 #include <cmath>
 
 #include "base/callback.h"
 #include "base/i18n/break_iterator.h"
 #include "base/i18n/case_conversion.h"
 #include "base/i18n/icu_string_conversions.h"
 #include "base/json/json_string_value_serializer.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/autocomplete/autocomplete_classifier.h"
 #include "chrome/browser/autocomplete/autocomplete_classifier_factory.h"
 #include "chrome/browser/autocomplete/autocomplete_match.h"
 #include "chrome/browser/autocomplete/autocomplete_provider_listener.h"
 #include "chrome/browser/autocomplete/autocomplete_result.h"
 #include "chrome/browser/autocomplete/keyword_provider.h"
 #include "chrome/browser/autocomplete/url_prefix.h"
+#include "chrome/browser/google/google_util.h"
 #include "chrome/browser/history/history_service.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/history/in_memory_database.h"
 #include "chrome/browser/metrics/variations/variations_http_header_provider.h"
 #include "chrome/browser/omnibox/omnibox_field_trial.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/search_engines/template_url_prepopulate_data.h"
 #include "chrome/browser/search_engines/template_url_service.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
+#include "chrome/browser/sync/profile_sync_service.h"
+#include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_instant_controller.h"
 #include "chrome/browser/ui/search/instant_controller.h"
 #include "chrome/common/net/url_fixer_upper.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "grit/generated_resources.h"
 #include "net/base/escape.h"
 #include "net/base/load_flags.h"
@@ skipping 785 matching lines @@
     NavigationResults* list) {
   for (size_t i = 0; i < list->size(); ++i) {
     NavigationResult& result = (*list)[i];
     result.set_relevance(
         result.CalculateRelevance(input_, providers_.has_keyword_provider()) +
         (list->size() - i - 1));
     result.set_relevance_from_server(false);
   }
 }
 
+bool SearchProvider::CanSendURL(const GURL& url,
+                                const TemplateURL* template_url,
+                                Profile* profile){
+  if (!url.is_valid())
+    return false;
+
+  // Only allow HTTP URLs or Google HTTPS URLs (including Google search
+  // result pages).  For the latter case, Google was already sent the HTTPS
+  // URLs when requesting the page, so the information is just re-sent.
+  if ((url.scheme() != content::kHttpScheme) &&
+      !google_util::IsGoogleDomainUrl(url, google_util::ALLOW_SUBDOMAIN,
+                                      google_util::ALLOW_NON_STANDARD_PORTS))
+    return false;
+
+  // Don't run if there's no profile or in incognito mode.
+  if (profile == NULL || profile->IsOffTheRecord())
+    return false;
+
+  // Don't run if we can't get preferences or search suggest is not enabled.
+  PrefService* prefs = profile->GetPrefs();
+  if (!prefs->GetBoolean(prefs::kSearchSuggestEnabled))
+    return false;
+
+  // TODO(hfung): Generalize detection of if the provider supports zero suggest.
+  // User permission to send URLs to non-Google providers also need to be
+  // determined.

[Peter Kasting, 2013/10/08 01:10:25]

Nit: This sentence seems redundant with the paragraph below and should probably
be removed.

[H Fung, 2013/10/30 23:56:15]

Done.

+  // Only make the request if we know that the provider supports zero suggest
+  // (currently only the prepopulated Google provider).
+  if (template_url == NULL || !template_url->SupportsReplacement() ||
+      template_url->prepopulate_id() != 1)

[Peter Kasting, 2013/10/08 01:10:25]

Never check prepopulate IDs in order to determine anything.  (2 places)

Check if GetEngineType() on the TemplateURL returns SEARCH_ENGINE_GOOGLE. 
That's the best way to check "is this Google".  Yes, this can succeed for some
non-prepopulated Google entries, I don't see why that's really problematic
though.

[H Fung, 2013/10/30 23:56:15]

Done.

+    return false;
+
+  ProfileSyncService* service =
+      ProfileSyncServiceFactory::GetInstance()->GetForProfile(profile);
+  browser_sync::SyncPrefs sync_prefs(prefs);
+  // ZeroSuggest requires sending the current URL to the suggest provider, so we
+  // only want to enable it if the user is willing to have this data sent.
+  // Because tab sync involves sending the same data, we currently use
+  // "tab sync is enabled and tab sync data is unencrypted" as a proxy for
+  // "the user is OK with sending this data".  We might someday want to change
+  // this to a standalone setting or part of some other explicit general opt-in.

[Peter Kasting, 2013/10/08 01:10:25]

This comment paragraph probably belongs in the header, since it's an explanation
of why we're doing what we're doing.

[H Fung, 2013/10/30 23:56:15]

Done.

+  if (!OmniboxFieldTrial::InZeroSuggestFieldTrial() ||
+      service == NULL ||
+      !service->IsSyncEnabledAndLoggedIn() ||
+      !sync_prefs.GetPreferredDataTypes(syncer::UserTypes()).Has(
+          syncer::PROXY_TABS) ||
+      service->GetEncryptedDataTypes().Has(syncer::SESSIONS)) {
+    return false;
+  }
+  return true;
+}
+
 net::URLFetcher* SearchProvider::CreateSuggestFetcher(
     int id,
     const TemplateURL* template_url,
     const AutocompleteInput& input) {
   if (!template_url || template_url->suggestions_url().empty())
     return NULL;
 
   // Bail if the suggestion URL is invalid with the given replacements.
   TemplateURLRef::SearchTermsArgs search_term_args(input.text());
   search_term_args.cursor_position = input.cursor_position();
   search_term_args.page_classification = input.current_page_classification();
+  // Send the current page URL if

[Peter Kasting, 2013/10/08 01:10:25]

This comment is incomplete.

[H Fung, 2013/10/30 23:56:15]

Done.

+  if (CanSendURL(current_page_url_, template_url, profile_) &&
+      OmniboxFieldTrial::InZeroSuggestAfterTypingFieldTrial() &&
+      template_url->prepopulate_id() == 1)

[Peter Kasting, 2013/10/08 01:10:25]

Why do we need to re-check the prepopulate ID (even if checking such a thing
were OK in the first place)?

[H Fung, 2013/10/30 23:56:15]

Done.

+    search_term_args.current_page_url = current_page_url_.spec();
   GURL suggest_url(template_url->suggestions_url_ref().ReplaceSearchTerms(
       search_term_args));
   if (!suggest_url.is_valid())
     return NULL;
 
   suggest_results_pending_++;
   LogOmniboxSuggestRequest(REQUEST_SENT);
 
   net::URLFetcher* fetcher =
       net::URLFetcher::Create(id, suggest_url, net::URLFetcher::GET, this);
@@ skipping 754 matching lines @@
     it->set_relevance(max_query_relevance);
     it->set_relevance_from_server(relevance_from_server);
   }
 }
 
 void SearchProvider::UpdateDone() {
   // We're done when the timer isn't running, there are no suggest queries
   // pending, and we're not waiting on Instant.
   done_ = !timer_.IsRunning() && (suggest_results_pending_ == 0);
 }