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

content/browser/web_contents/navigation_entry_impl.h

 // 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.
 
 #ifndef CONTENT_BROWSER_WEB_CONTENTS_NAVIGATION_ENTRY_IMPL_H_
 #define CONTENT_BROWSER_WEB_CONTENTS_NAVIGATION_ENTRY_IMPL_H_
 
 #include "base/basictypes.h"
 #include "base/memory/ref_counted.h"
 #include "content/browser/site_instance_impl.h"
@@ skipping 57 matching lines @@
   virtual void SetOriginalRequestURL(const GURL& original_url) OVERRIDE;
   virtual const GURL& GetOriginalRequestURL() const OVERRIDE;
   virtual void SetIsOverridingUserAgent(bool override) OVERRIDE;
   virtual bool GetIsOverridingUserAgent() const OVERRIDE;
   virtual void SetTimestamp(base::Time timestamp) OVERRIDE;
   virtual base::Time GetTimestamp() const OVERRIDE;
   virtual void SetCanLoadLocalResources(bool allow) OVERRIDE;
   virtual bool GetCanLoadLocalResources() const OVERRIDE;
   virtual void SetFrameToNavigate(const std::string& frame_name) OVERRIDE;
   virtual const std::string& GetFrameToNavigate() const OVERRIDE;
+  virtual void SetExtraData(const std::string& key,
+                            const string16& data) OVERRIDE;
+  virtual bool GetExtraData(const std::string& key,
+                            string16* data) const OVERRIDE;
 
   void set_unique_id(int unique_id) {
     unique_id_ = unique_id;
   }
 
   // The SiteInstance tells us how to share sub-processes. This is a reference
   // counted pointer to a shared site instance.
   //
   // Note that the SiteInstance should usually not be changed after it is set,
   // but this may happen if the NavigationEntry was cloned and needs to use a
@@ skipping 170 matching lines @@
   bool should_replace_entry_;
 
   // Set when this entry should be able to access local file:// resources. This
   // value is not needed after the entry commits and is not persisted.
   bool can_load_local_resources_;
 
   // If not empty, the name of the frame to navigate. This field is not
   // persisted, because it is currently only used in tests.
   std::string frame_to_navigate_;
 
+  // Used to store extra data to support browser features.
+  std::map<std::string, string16> extra_data_;

[sky, 2013/01/30 23:32:03]

Do you see the big warning on 182? Do we not need this data persisted?

[sreeram, 2013/01/30 23:39:05]

Yes, hence the changes to chrome/browser/sessions/* in this CL.

However, the warning has something new about Android WebView state_serializer
that wasn't there a month or two ago. @mathp: Though we don't do query terms
extraction for Android, I think we should make those changes now, so that when
Android eventually catches up, we won't forget to.

Also, just to confirm: The c/b/sessions stuff handles TabRestore as well, right?

[sky, 2013/01/31 01:34:15]

That only persists the session terms, not the map.

[sreeram, 2013/01/31 02:14:11]

Right. The map is meant to be a generic bucket for higher level modules
(chrome/...). I don't think we want to assume that everything in the map needs
to be persisted. Stuff that should be persisted should be handled specifically
(since, for example, we'll need to add specific fields to the sync protobufs).
Of course, now that it's easy to add data to this map without modifying the
header, future users of needs-to-persist data may not see the warning, so they
may not realize that they need to take extra steps to persist their data.

Options:
1. Repeat the warning here, to the effect that just adding data to the map won't
automatically persist it.
2. Add a specific field for query terms, instead of this generic map. I think
@jam preferred the map, though.

Your thoughts?

[Mathieu, 2013/01/31 14:45:32]

Option 1 is preferred. I added a warning.

+
   // Copy and assignment is explicitly allowed for this class.
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_WEB_CONTENTS_NAVIGATION_ENTRY_IMPL_H_