[Search] No longer reverting omnibox text on instant search

android_webview/native/state_serializer.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 "android_webview/native/state_serializer.h"
 
 #include <string>
 
 #include "base/memory/scoped_vector.h"
 #include "base/pickle.h"
@@ skipping 146 matching lines @@
 
   if (!pickle->WriteString(entry.GetBaseURLForDataURL().spec()))
     return false;
 
   if (!pickle->WriteBool(static_cast<int>(entry.GetIsOverridingUserAgent())))
     return false;
 
   if (!pickle->WriteInt64(entry.GetTimestamp().ToInternalValue()))
     return false;
 
+  if (!pickle->WriteString16(entry.GetSearchTerms()))
+    return false;

[joth, 2012/12/13 02:11:48]

fwiw the java-side of the webview API has no way to read/write search terms, so
there's not much point us persisting these. (AIUI the content/ layer would never
spontaneously push values into this field? It's purely up to the embedder to
populate this field, and android_webview/ never would).

but equally, doesn't hurt to have this here.

[boliu, 2012/12/13 02:26:46]

Right, agree with joth here that android webview won't be using this field, so
there is no point in persisting it. Also save/restore is probably a hot path for
android, but not that a single empty string will make a difference.

+
   // Please update AW_STATE_VERSION if serialization format is changed.

[joth, 2012/12/13 02:11:48]

guess you didn't see this comment? Maybe put some bold flashing ascii art on it?

 
   return true;
 }
 
 bool RestoreNavigationEntryFromPickle(PickleIterator* iterator,
                                       content::NavigationEntry* entry) {
   {
     string url;
     if (!iterator->ReadString(&url))
       return false;
@@ skipping 64 matching lines @@
     entry->SetIsOverridingUserAgent(is_overriding_user_agent);
   }
 
   {
     int64 timestamp;
     if (!iterator->ReadInt64(&timestamp))
       return false;
     entry->SetTimestamp(base::Time::FromInternalValue(timestamp));
   }
 
+  {
+    string16 search_terms;
+    if (!iterator->ReadString16(&search_terms))
+      search_terms = string16();

[joth, 2012/12/13 02:11:48]

this doesn't make sense? you're modifying a local, but it never makes it into
entry->SetSearchTerms(search_terms). the tests should catch this.

Following pattern above, it should be return false if the read failed.

+  }
+
   return true;
 }
 
 }  // namespace internal
 
 }  // namespace android_webview