Persist the extension omnibox default suggestion to Prefs.

chrome/browser/extensions/api/omnibox/omnibox_api.cc

Index: chrome/browser/extensions/api/omnibox/omnibox_api.cc
diff --git a/chrome/browser/extensions/api/omnibox/omnibox_api.cc b/chrome/browser/extensions/api/omnibox/omnibox_api.cc
index 90dcf698b882ee62eda14128e6a388ca28ad6d04..b7d8f8bed23f52f50e0a8008d358c7bd24611180 100644
--- a/chrome/browser/extensions/api/omnibox/omnibox_api.cc
+++ b/chrome/browser/extensions/api/omnibox/omnibox_api.cc
@@ -11,7 +11,9 @@
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/extension_event_router.h"
+#include "chrome/browser/extensions/extension_prefs.h"
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search_engines/template_url.h"
 #include "chrome/browser/ui/browser.h"
@@ -39,30 +41,11 @@ const char kDescriptionStylesLengthError[] =
 const char kSuggestionContent[] = "content";
 const char kSuggestionDescription[] = "description";
 const char kSuggestionDescriptionStyles[] = "descriptionStyles";
+const char kSuggestionDescriptionStylesRaw[] = "descriptionStylesRaw";
 const char kDescriptionStylesType[] = "type";
 const char kDescriptionStylesOffset[] = "offset";
 const char kDescriptionStylesLength[] = "length";
 
-static base::LazyInstance<base::PropertyAccessor<ExtensionOmniboxSuggestion> >
-    g_extension_omnibox_suggestion_property_accessor =
-        LAZY_INSTANCE_INITIALIZER;
-
-base::PropertyAccessor<ExtensionOmniboxSuggestion>& GetPropertyAccessor() {
-  return g_extension_omnibox_suggestion_property_accessor.Get();
-}
-
-// Returns the suggestion object set by the extension via the
-// omnibox.setDefaultSuggestion call, or NULL if it was never set.
-const ExtensionOmniboxSuggestion* GetDefaultSuggestionForExtension(
-    Profile* profile, const std::string& extension_id) {
-  const Extension* extension =
-      profile->GetExtensionService()->GetExtensionById(extension_id, false);
-  if (!extension)
-    return NULL;
-  return GetPropertyAccessor().GetProperty(
-      profile->GetExtensionService()->GetPropertyBag(extension));
-}
-
 }  // namespace
 
 // static
@@ -128,21 +111,7 @@ bool OmniboxSendSuggestionsFunction::RunImpl() {
     DictionaryValue* suggestion_value;
     EXTENSION_FUNCTION_VALIDATE(suggestions_value->GetDictionary(
         i, &suggestion_value));
-    EXTENSION_FUNCTION_VALIDATE(suggestion_value->GetString(
-        kSuggestionContent, &suggestion.content));
-    EXTENSION_FUNCTION_VALIDATE(suggestion_value->GetString(
-        kSuggestionDescription, &suggestion.description));
-
-    if (suggestion_value->HasKey(kSuggestionDescriptionStyles)) {
-      ListValue* styles;
-      EXTENSION_FUNCTION_VALIDATE(
-          suggestion_value->GetList(kSuggestionDescriptionStyles, &styles));
-      EXTENSION_FUNCTION_VALIDATE(suggestion.ReadStylesFromValue(*styles));
-    } else {
-      suggestion.description_styles.clear();
-      suggestion.description_styles.push_back(
-          ACMatchClassification(0, ACMatchClassification::NONE));
-    }
+    EXTENSION_FUNCTION_VALIDATE(suggestion.Populate(*suggestion_value, true));
   }
 
   content::NotificationService::current()->Notify(
@@ -157,24 +126,12 @@ bool OmniboxSetDefaultSuggestionFunction::RunImpl() {
   ExtensionOmniboxSuggestion suggestion;
   DictionaryValue* suggestion_value;
   EXTENSION_FUNCTION_VALIDATE(args_->GetDictionary(0, &suggestion_value));
-  EXTENSION_FUNCTION_VALIDATE(suggestion_value->GetString(
-      kSuggestionDescription, &suggestion.description));
-
-  if (suggestion_value->HasKey(kSuggestionDescriptionStyles)) {
-    ListValue* styles;
-    EXTENSION_FUNCTION_VALIDATE(
-        suggestion_value->GetList(kSuggestionDescriptionStyles, &styles));
-    EXTENSION_FUNCTION_VALIDATE(suggestion.ReadStylesFromValue(*styles));
-  } else {
-    suggestion.description_styles.clear();
-    suggestion.description_styles.push_back(
-        ACMatchClassification(0, ACMatchClassification::NONE));
-  }
+  EXTENSION_FUNCTION_VALIDATE(suggestion.Populate(*suggestion_value, false));
 
-  // Store the suggestion in the extension's runtime data.
-  GetPropertyAccessor().SetProperty(
-      profile_->GetExtensionService()->GetPropertyBag(GetExtension()),
-      suggestion);
+  ExtensionPrefs* prefs =
+      ExtensionSystem::Get(profile())->extension_service()->extension_prefs();
+  if (prefs)
+    prefs->SetOmniboxDefaultSuggestion(extension_id(), suggestion);
 
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_EXTENSION_OMNIBOX_DEFAULT_SUGGESTION_CHANGED,
@@ -188,6 +145,46 @@ ExtensionOmniboxSuggestion::ExtensionOmniboxSuggestion() {}
 
 ExtensionOmniboxSuggestion::~ExtensionOmniboxSuggestion() {}
 
+bool ExtensionOmniboxSuggestion::Populate(const base::DictionaryValue& value,
+                                          bool require_content) {
+  if (!value.GetString(kSuggestionContent, &content) && require_content)
+    return false;
+
+  if (!value.GetString(kSuggestionDescription, &description))
+    return false;
+
+  description_styles.clear();
+  if (value.HasKey(kSuggestionDescriptionStyles)) {
+    // This version is used by messages from the extension.

[Yoyo Zhou, 2012/05/04 18:50:52]

Would it be clearer to write "comes from" instead of "is used by"? (Also at
165.)

[Matt Perry, 2012/05/04 20:10:12]

Done.

+    ListValue* styles = NULL;
+    if (!value.GetList(kSuggestionDescriptionStyles, &styles) ||
+        !ReadStylesFromValue(*styles)) {
+      return false;
+    }
+  } else if (value.HasKey(kSuggestionDescriptionStylesRaw)) {
+    // This version is the one we use to persist to disk.
+    ListValue* styles = NULL;
+    if (!value.GetList(kSuggestionDescriptionStylesRaw, &styles))
+      return false;

[Yoyo Zhou, 2012/05/04 18:50:52]

Also we should return false if styles is an empty list.

[Matt Perry, 2012/05/04 20:10:12]

The previous code didn't... I think it might be valid to have an empty styles
list.

[Yoyo Zhou, 2012/05/04 20:21:23]

Well, if you have an empty list you'd want to push 0, NONE (which does not
happen in this case).

[Matt Perry, 2012/05/04 20:23:13]

Oh, good call. Thanks. And now that I look more carefully, I think you're right
that the "raw" version should always have a non-empty list.

+    for (size_t i = 0; i < styles->GetSize(); ++i) {
+      base::DictionaryValue* style = NULL;
+      int offset, type;
+      if (!styles->GetDictionary(i, &style))
+        return false;
+      if (!style->GetInteger(kDescriptionStylesType, &type))
+        return false;
+      if (!style->GetInteger(kDescriptionStylesOffset, &offset))
+        return false;
+      description_styles.push_back(ACMatchClassification(offset, type));
+    }
+  } else {
+    description_styles.push_back(
+        ACMatchClassification(0, ACMatchClassification::NONE));
+  }
+
+  return true;
+}
+
 bool ExtensionOmniboxSuggestion::ReadStylesFromValue(
     const ListValue& styles_value) {
   description_styles.clear();
@@ -234,6 +231,27 @@ bool ExtensionOmniboxSuggestion::ReadStylesFromValue(
   return true;
 }
 
+scoped_ptr<base::DictionaryValue> ExtensionOmniboxSuggestion::ToValue() const {
+  scoped_ptr<base::DictionaryValue> value(new base::DictionaryValue());
+
+  value->SetString(kSuggestionContent, content);
+  value->SetString(kSuggestionDescription, description);
+
+  if (description_styles.size() > 0) {
+    base::ListValue* styles_value = new base::ListValue();
+    for (size_t i = 0; i < description_styles.size(); ++i) {
+      base::DictionaryValue* style = new base::DictionaryValue();
+      style->SetInteger(kDescriptionStylesOffset, description_styles[i].offset);
+      style->SetInteger(kDescriptionStylesType, description_styles[i].style);
+      styles_value->Append(style);
+    }
+
+    value->Set(kSuggestionDescriptionStylesRaw, styles_value);
+  }
+
+  return value.Pass();
+}
+
 ExtensionOmniboxSuggestions::ExtensionOmniboxSuggestions() : request_id(0) {}
 
 ExtensionOmniboxSuggestions::~ExtensionOmniboxSuggestions() {}
@@ -244,21 +262,27 @@ void ApplyDefaultSuggestionForExtensionKeyword(
     const string16& remaining_input,
     AutocompleteMatch* match) {
   DCHECK(keyword->IsExtensionKeyword());
-  const ExtensionOmniboxSuggestion* suggestion =
-      GetDefaultSuggestionForExtension(profile, keyword->GetExtensionId());
-  if (!suggestion)
+
+  ExtensionPrefs* prefs =
+      ExtensionSystem::Get(profile)->extension_service()->extension_prefs();
+  if (!prefs)
+    return;
+
+  ExtensionOmniboxSuggestion suggestion =
+      prefs->GetOmniboxDefaultSuggestion(keyword->GetExtensionId());
+  if (suggestion.description.empty())
     return;  // fall back to the universal default
 
   const string16 kPlaceholderText(ASCIIToUTF16("%s"));
   const string16 kReplacementText(ASCIIToUTF16("<input>"));
 
-  string16 description = suggestion->description;
+  string16 description = suggestion.description;
   ACMatchClassifications& description_styles = match->contents_class;
-  description_styles = suggestion->description_styles;
+  description_styles = suggestion.description_styles;
 
   // Replace "%s" with the user's input and adjust the style offsets to the
   // new length of the description.
-  size_t placeholder(suggestion->description.find(kPlaceholderText, 0));
+  size_t placeholder(suggestion.description.find(kPlaceholderText, 0));
   if (placeholder != string16::npos) {
     string16 replacement =
         remaining_input.empty() ? kReplacementText : remaining_input;