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

content/browser/web_contents/navigation_entry_impl.h

Index: content/browser/web_contents/navigation_entry_impl.h
diff --git a/content/browser/web_contents/navigation_entry_impl.h b/content/browser/web_contents/navigation_entry_impl.h
index c65bdb32b75a9d9dea2f212e344bb4000ae06810..da22ba9ffba27ec472ed6526a205065d31221b2c 100644
--- a/content/browser/web_contents/navigation_entry_impl.h
+++ b/content/browser/web_contents/navigation_entry_impl.h
@@ -75,6 +75,10 @@ class CONTENT_EXPORT NavigationEntryImpl
   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;
@@ -265,6 +269,9 @@ class CONTENT_EXPORT NavigationEntryImpl
   // 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.
 };