diff --git a/components/search_engines/search_terms_data.cc b/components/search_engines/search_terms_data.cc index e8abc03d36dd4..be897db754e21 100644 --- a/components/search_engines/search_terms_data.cc +++ b/components/search_engines/search_terms_data.cc @@ -76,6 +76,10 @@ std::string SearchTermsData::NTPIsThemedParam() const { return std::string(); } +std::string SearchTermsData::IOSWebViewTypeParam() const { + return std::string(); +} + std::string SearchTermsData::GoogleImageSearchSource() const { return std::string(); } diff --git a/components/search_engines/search_terms_data.h b/components/search_engines/search_terms_data.h index 2dcdd414eb45e..96c8854899363 100644 --- a/components/search_engines/search_terms_data.h +++ b/components/search_engines/search_terms_data.h @@ -70,6 +70,11 @@ class SearchTermsData { // non-empty for UIThreadSearchTermsData. virtual std::string NTPIsThemedParam() const; + // Returns a string indicating which webview is currently in use on iOS, + // suitable for adding as a query string param to search requests. Returns an + // empty string if no parameter should be passed along with search requests. + virtual std::string IOSWebViewTypeParam() const; + // Returns the value to use for replacements of type // GOOGLE_IMAGE_SEARCH_SOURCE. virtual std::string GoogleImageSearchSource() const; diff --git a/ios/chrome/browser/experimental_flags.h b/ios/chrome/browser/experimental_flags.h index a284e187517c4..0869146cb1e4f 100644 --- a/ios/chrome/browser/experimental_flags.h +++ b/ios/chrome/browser/experimental_flags.h @@ -5,6 +5,8 @@ #ifndef IOS_CHROME_BROWSER_EXPERIMENTAL_FLAGS_H_ #define IOS_CHROME_BROWSER_EXPERIMENTAL_FLAGS_H_ +#include + // This file can be empty. Its purpose is to contain the relatively short lived // declarations required for experimental flags. @@ -30,6 +32,10 @@ bool IsLRUSnapshotCacheEnabled(); // The returned value will not change within a given session. bool IsWKWebViewEnabled(); +// Returns a string containing extra params that should be sent along with +// omnibox search requests. The returned value contains a leading "&". +std::string GetWKWebViewSearchParams(); + // Whether keyboard commands are supported. bool AreKeyboardCommandsEnabled(); diff --git a/ios/chrome/browser/experimental_flags.mm b/ios/chrome/browser/experimental_flags.mm index e6714a400a1bf..da6805f663c18 100644 --- a/ios/chrome/browser/experimental_flags.mm +++ b/ios/chrome/browser/experimental_flags.mm @@ -7,6 +7,7 @@ #include "ios/chrome/browser/experimental_flags.h" +#include #import #include @@ -70,31 +71,64 @@ bool IsLRUSnapshotCacheEnabled() { base::CompareCase::INSENSITIVE_ASCII); } -bool IsWKWebViewEnabled() { - // If g_wkwebview_trial_eligibility hasn't been set, default it to - // ineligibile. This ensures future calls to try to set it will DCHECK. - if (g_wkwebview_trial_eligibility == WKWebViewEligibility::UNSET) { - g_wkwebview_trial_eligibility = WKWebViewEligibility::INELIGIBLE; - } +// Helper method that returns true if it is safe to check the finch group for +// the IOSUseWKWebView experiment. Some users are ineligible to be in the +// trial, so for those users, this method returns false. If this method returns +// false, do not check for the current finch group, as doing so will incorrectly +// mark the current user as being in the experiment. +bool CanCheckWKWebViewExperiment() { + // True if this user is eligible for the WKWebView experiment and it is ok to + // check the experiment group. + static bool ok_to_check_finch = false; + static dispatch_once_t once; + dispatch_once(&once, ^{ + // If g_wkwebview_trial_eligibility hasn't been set, default it to + // ineligible. This ensures future calls to try to set it will DCHECK. + if (g_wkwebview_trial_eligibility == WKWebViewEligibility::UNSET) { + g_wkwebview_trial_eligibility = WKWebViewEligibility::INELIGIBLE; + } + + // If WKWebView isn't supported, don't activate the experiment at all. This + // avoids someone being slotted into the WKWebView bucket (and thus + // reporting as WKWebView), but actually running UIWebView. + if (!web::IsWKWebViewSupported()) { + ok_to_check_finch = false; + return; + } + + // Check for a flag forcing a specific group. Even ineligible users can be + // opted into WKWebView if an override flag is set. + base::CommandLine* command_line = base::CommandLine::ForCurrentProcess(); + bool trial_overridden = + command_line->HasSwitch(switches::kEnableIOSWKWebView) || + command_line->HasSwitch(switches::kDisableIOSWKWebView); + + // If the user isn't eligible for the trial (i.e., their state is such that + // they should not be automatically selected for the group), and there's no + // explicit override, don't check the group (again, to avoid having them + // report as part of a group at all). + if (g_wkwebview_trial_eligibility == WKWebViewEligibility::INELIGIBLE && + !trial_overridden) { + ok_to_check_finch = false; + return; + } + + ok_to_check_finch = true; + }); + + return ok_to_check_finch; +} - // If WKWebView isn't supported, don't activate the experiment at all. This - // avoids someone being slotted into the WKWebView bucket (and thus reporting - // as WKWebView), but actually running UIWebView. - if (!web::IsWKWebViewSupported()) +bool IsWKWebViewEnabled() { + if (!CanCheckWKWebViewExperiment()) { return false; + } - // Check for a flag forcing a specific group. + // Check if the experimental flag is turned on. base::CommandLine* command_line = base::CommandLine::ForCurrentProcess(); - bool force_enable = command_line->HasSwitch(switches::kEnableIOSWKWebView); - bool force_disable = command_line->HasSwitch(switches::kDisableIOSWKWebView); - bool trial_overridden = force_enable || force_disable; - - // If the user isn't eligible for the trial (i.e., their state is such that - // they should not be automatically selected for the group), and there's no - // explicit override, don't check the group (again, to avoid having them - // report as part of a group at all). - if (g_wkwebview_trial_eligibility == WKWebViewEligibility::INELIGIBLE && - !trial_overridden) + if (command_line->HasSwitch(switches::kEnableIOSWKWebView)) + return true; + else if (command_line->HasSwitch(switches::kDisableIOSWKWebView)) return false; // Now that it's been established that user is a candidate, set up the trial @@ -102,17 +136,19 @@ bool IsWKWebViewEnabled() { std::string group_name = base::FieldTrialList::FindFullName("IOSUseWKWebView"); - // Check if the experimental flag is turned on. - if (force_enable) - return true; - else if (force_disable) - return false; - // Check if the finch experiment is turned on. return base::StartsWith(group_name, "Enabled", base::CompareCase::INSENSITIVE_ASCII); } +std::string GetWKWebViewSearchParams() { + if (!CanCheckWKWebViewExperiment()) { + return std::string(); + } + + return variations::GetVariationParamValue("IOSUseWKWebView", "esrch"); +} + bool AreKeyboardCommandsEnabled() { return !base::CommandLine::ForCurrentProcess()->HasSwitch( switches::kDisableKeyboardCommands); diff --git a/ios/chrome/browser/search_engines/ui_thread_search_terms_data.cc b/ios/chrome/browser/search_engines/ui_thread_search_terms_data.cc index e09d36f08c6ba..d6e92f6a68603 100644 --- a/ios/chrome/browser/search_engines/ui_thread_search_terms_data.cc +++ b/ios/chrome/browser/search_engines/ui_thread_search_terms_data.cc @@ -12,10 +12,12 @@ #include "components/search/search.h" #include "components/version_info/version_info.h" #include "ios/chrome/browser/application_context.h" +#include "ios/chrome/browser/experimental_flags.h" #include "ios/chrome/browser/google/google_brand.h" #include "ios/chrome/browser/google/google_url_tracker_factory.h" #include "ios/chrome/common/channel_info.h" #include "ios/web/public/web_thread.h" +#include "net/base/escape.h" #include "url/gurl.h" #if defined(ENABLE_RLZ) @@ -119,6 +121,16 @@ std::string UIThreadSearchTermsData::NTPIsThemedParam() const { return std::string(); } +std::string UIThreadSearchTermsData::IOSWebViewTypeParam() const { + DCHECK(thread_checker_.CalledOnValidThread()); + std::string param = experimental_flags::GetWKWebViewSearchParams(); + if (param.empty()) { + return std::string(); + } + + return "&esrch=" + net::EscapeQueryParamValue(param, true); +} + std::string UIThreadSearchTermsData::GoogleImageSearchSource() const { DCHECK(thread_checker_.CalledOnValidThread()); std::string version(version_info::GetProductName() + " " + diff --git a/ios/chrome/browser/search_engines/ui_thread_search_terms_data.h b/ios/chrome/browser/search_engines/ui_thread_search_terms_data.h index 7e7e01bee447b..0c8a2c29b335d 100644 --- a/ios/chrome/browser/search_engines/ui_thread_search_terms_data.h +++ b/ios/chrome/browser/search_engines/ui_thread_search_terms_data.h @@ -31,6 +31,7 @@ class UIThreadSearchTermsData : public SearchTermsData { std::string ForceInstantResultsParam(bool for_prerender) const override; int OmniboxStartMargin() const override; std::string NTPIsThemedParam() const override; + std::string IOSWebViewTypeParam() const override; std::string GoogleImageSearchSource() const override; private: