Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(915)

Unified Diff: chrome/browser/autocomplete/search_provider.cc

Issue 23621037: Send URLs on non-zero prefix suggest requests also. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Move set_current_page_url Created 7 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/autocomplete/search_provider.cc
diff --git a/chrome/browser/autocomplete/search_provider.cc b/chrome/browser/autocomplete/search_provider.cc
index 1f982e2a2ffbf16f7beda1210549a81f84135033..52ff39986e1fc388195a7b97379a7be28b12a2e0 100644
--- a/chrome/browser/autocomplete/search_provider.cc
+++ b/chrome/browser/autocomplete/search_provider.cc
@@ -25,6 +25,7 @@
#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"
@@ -35,6 +36,8 @@
#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"
@@ -840,6 +843,58 @@ void SearchProvider::ApplyCalculatedNavigationRelevance(
}
}
+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 paragr
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
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 hea
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,
@@ -851,6 +906,11 @@ net::URLFetcher* SearchProvider::CreateSuggestFetcher(
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 (eve
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())

Powered by Google App Engine
This is Rietveld 408576698