[Search] Store and recall search terms using NavigationEntry to improve search term extraction

chrome/browser/ui/search/search.cc

 // Copyright (c) 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/ui/search/search.h"
 
 #include "base/command_line.h"
 #include "base/metrics/field_trial.h"
+#include "base/string16.h"

[sreeram, 2013/01/30 21:41:39]

Nit: No need for this (since it's already included in search.h).

[Mathieu, 2013/01/30 21:55:12]

Done.

 #include "base/string_number_conversions.h"
 #include "base/string_split.h"
 #include "base/string_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/chrome_version_info.h"
+#include "content/public/browser/navigation_entry.h"
 
 #if !defined(OS_ANDROID)
 #include "chrome/browser/themes/theme_service.h"
 #include "chrome/browser/themes/theme_service_factory.h"
 #endif
 
 namespace {
 
 // Configuration options for Embedded Search.
 // InstantExtended field trials are named in such a way that we can parse out
@@ skipping 14 matching lines @@
 
 // If the field trial's group name ends with this string its configuration will
 // be ignored and Instant Extended will not be enabled by default.
 const char kDisablingSuffix[] = "DISABLED";
 
 }  // namespace
 
 namespace chrome {
 namespace search {
 
+// static
+const char kInstantExtendedSearchTermsKey[] = "search_terms";
+
 // Check whether or not the Extended API should be used on the given profile.
 bool IsInstantExtendedAPIEnabled(Profile* profile) {
   return EmbeddedSearchPageVersion(profile) != 0;
 }
 
 // Determine what embedded search page version to request from the user's
 // default search provider.  If 0, the embedded search UI should not be enabled.
 // Note that the profile object here isn't const because we need to determine
 // whether or not the user has a theme installed as part of this check, and
 // that logic requires a non-const profile for whatever reason.
@@ skipping 73 matching lines @@
   CommandLine::ForCurrentProcess()->AppendSwitch(
       switches::kEnableQueryExtraction);
 #else
   // On desktop, query extraction is controlled by the instant-extended-api
   // flag.
   CommandLine::ForCurrentProcess()->AppendSwitch(
       switches::kEnableInstantExtendedAPI);
 #endif
 }
 
+string16 GetSearchTermsFromNavigationEntry(
+    const content::NavigationEntry* entry) {
+  string16 out_value;
+  if (entry->GetExtraData(
+      std::string(kInstantExtendedSearchTermsKey),
+      &out_value))
+    return out_value;
+  return string16();

[sreeram, 2013/01/30 21:41:39]

Nit: If you rewrite this as the following, it will help the compiler perform
NRVO:
  string16 search_terms;
  if (!entry->GetExtraData(std::string(kInstantExtendedSearchTermsKey),
                           &search_terms))
    search_terms.clear();
  return search_terms;

In fact, I'd drop the 'if' check and std::string conversions and just do this:
  string16 search_terms;
  entry->GetExtraData(kInstantExtendedSearchTermsKey, &search_terms);
  return search_terms;

[Mathieu, 2013/01/30 21:55:12]

Done.

+}
+
 bool IsForcedInstantURL(const GURL& url) {
   CommandLine* command_line = CommandLine::ForCurrentProcess();
   if (!command_line->HasSwitch(switches::kInstantURL))
     return false;
 
   GURL instant_url(command_line->GetSwitchValueASCII(switches::kInstantURL));
   return url.scheme() == instant_url.scheme() &&
          url.host() == instant_url.host() &&
          url.port() == instant_url.port() &&
          url.path() == instant_url.path();
@@ skipping 51 matching lines @@
 
 // Given a FieldTrialFlags object, returns the boolean value of the provided
 // flag.
 bool GetBoolValueForFlagWithDefault(
     const std::string& flag, bool default_value, FieldTrialFlags& flags) {
   return !!GetUInt64ValueForFlagWithDefault(flag, default_value ? 1 : 0, flags);
 }
 
 }  // namespace search
 }  // namespace chrome