Enable query extraction for Android

chrome/browser/search_engines/search_terms_data.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/search_engines/search_terms_data.h"
 
 #include "base/logging.h"
 #include "base/metrics/field_trial.h"
 #include "base/strings/string_number_conversions.h"
 #include "chrome/browser/browser_process.h"
@@ skipping 123 matching lines @@
     uint32 instant_extended_api_version =
         chrome::search::EmbeddedSearchPageVersion(profile_);
     if (instant_extended_api_version == 0 &&
         chrome::BrowserInstantController::IsInstantEnabled(profile_))
       return "ion=1&";
   }
   return std::string();
 }
 
 std::string UIThreadSearchTermsData::InstantExtendedEnabledParam() const {
+#if defined(OS_IOS) || defined(OS_ANDROID)

[Peter Kasting, 2013/03/01 21:44:44]

You're omitting the threading assertion and the profile_ NULL-check, both of
which seem necessary.

[Yusuf, 2013/03/01 22:26:07]

Done.

+  if (chrome::search::IsQueryExtractionEnabled(profile_) &&
+      GURL(GoogleBaseURLValue()).SchemeIsSecure())

[Peter Kasting, 2013/03/01 21:44:44]

Shouldn't we be checking scheme security elsewhere?  Instant extended certainly
doesn't check it here.

[Yusuf, 2013/03/01 22:26:07]

You are right. Apparently it is already checked in getSearchTerms which we will
be using, so removing this from here.

On 2013/03/01 21:44:44, Peter Kasting wrote:

[rohitrao (ping after 24h), 2013/03/01 22:38:26]

We have to be a little careful here, because we don't want to append espv=1 for
HTTP urls.  While getSearchTerms() will check for https before performing terms
extraction, if we blindly add espv=1, then the server might incorrectly return
the 1993 ui for HTTP searches.

+    return std::string(google_util::kInstantExtendedAPIParam) + "=" +
+        std::string(google_util::kInstantExtendedDefaultAPIVersion) + "&";

[Peter Kasting, 2013/03/01 21:44:44]

If you're already modifying
UIThreadSearchTermsData::InstantExtendedEnabledParam() as your way of
accomplishing query extraction, why not modify
chrome::search::EmbeddedSearchPageVersion() to return the version number you
want?  Then you could avoid the #ifs here and, if I'm not mistaken, in the other
places you're touching as well.

[Yusuf, 2013/03/01 22:26:07]

Done.

+#else
   DCHECK(!BrowserThread::IsWellKnownThread(BrowserThread::UI) ||
          BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (profile_) {
     uint32 instant_extended_api_version =
         chrome::search::EmbeddedSearchPageVersion(profile_);
     if (instant_extended_api_version != 0) {
       return std::string(google_util::kInstantExtendedAPIParam) + "=" +
           base::Uint64ToString(instant_extended_api_version) + "&";
     }
   }
+#endif
   return std::string();
 }
 
 // static
 void UIThreadSearchTermsData::SetGoogleBaseURL(const std::string& base_url) {
   delete google_base_url_;
   google_base_url_ = base_url.empty() ? NULL : new std::string(base_url);
 }