Save a little work when fetching the default title by only checking the name...

chrome/common/extensions/extension_unittest.cc

Index: chrome/common/extensions/extension_unittest.cc
===================================================================
--- chrome/common/extensions/extension_unittest.cc	(revision 29521)
+++ chrome/common/extensions/extension_unittest.cc	(working copy)
@@ -300,16 +300,16 @@
   scoped_ptr<ExtensionAction> action;
   DictionaryValue input;
 
-  // First try with an empty dictionary. We should get nothing back.
+  // First try with an empty dictionary.
   ASSERT_TRUE(extension.LoadExtensionActionHelper(
-      &input, &error_msg, ExtensionAction::PAGE_ACTION) == NULL);
-  ASSERT_STRNE("", error_msg.c_str());
+      &input, &error_msg, ExtensionAction::PAGE_ACTION) != NULL);
+  ASSERT_STREQ("", error_msg.c_str());
   error_msg = "";
 
   // Now try the same, but as a browser action. Ensure same results.
   ASSERT_TRUE(extension.LoadExtensionActionHelper(
-      &input, &error_msg, ExtensionAction::BROWSER_ACTION) == NULL);
-  ASSERT_STRNE("", error_msg.c_str());
+      &input, &error_msg, ExtensionAction::BROWSER_ACTION) != NULL);
+  ASSERT_STREQ("", error_msg.c_str());
   error_msg = "";
 
   // Now setup some values to use in the page action.
@@ -332,6 +332,7 @@
   ASSERT_TRUE(NULL != action.get());
   ASSERT_STREQ("", error_msg.c_str());
   ASSERT_STREQ(id.c_str(), action->id().c_str());
+  // No title, so fall back to name.
   ASSERT_STREQ(name.c_str(), action->title().c_str());
   ASSERT_EQ(2u, action->icon_paths().size());
   ASSERT_STREQ(img1.c_str(), action->icon_paths()[0].c_str());
@@ -345,6 +346,7 @@
   ASSERT_STREQ("", error_msg.c_str());
   // Browser actions don't have an id, page actions do.
   ASSERT_STREQ("", action->id().c_str());
+  // No title, so fall back to name.
   ASSERT_STREQ(name.c_str(), action->title().c_str());
   ASSERT_EQ(2u, action->icon_paths().size());
   ASSERT_STREQ(img1.c_str(), action->icon_paths()[0].c_str());
@@ -390,14 +392,14 @@
   ASSERT_STREQ("", error_msg.c_str());
   error_msg = "";
 
-  // Then remove the name key.
+  // Then remove the name key. It's optional, so no error.
   copy.reset(static_cast<DictionaryValue*>(input.DeepCopy()));
   copy->Remove(keys::kName, NULL);
   action.reset(extension.LoadExtensionActionHelper(
       copy.get(), &error_msg, ExtensionAction::PAGE_ACTION));
-  ASSERT_TRUE(NULL == action.get());
-  ASSERT_TRUE(MatchPattern(error_msg.c_str(),
-                           errors::kInvalidPageActionDefaultTitle));
+  ASSERT_TRUE(NULL != action.get());
+  ASSERT_STREQ("", action->title().c_str());
+  ASSERT_STREQ("", error_msg.c_str());
   error_msg = "";
 
   // Same test (name key), but with browser action.
@@ -405,9 +407,9 @@
   copy->Remove(keys::kName, NULL);
   action.reset(extension.LoadExtensionActionHelper(
       copy.get(), &error_msg, ExtensionAction::BROWSER_ACTION));
-  ASSERT_TRUE(NULL == action.get());
-  ASSERT_TRUE(MatchPattern(error_msg.c_str(),
-                           errors::kInvalidPageActionDefaultTitle));
+  ASSERT_TRUE(NULL != action.get());
+  ASSERT_STREQ("", action->title().c_str());
+  ASSERT_STREQ("", error_msg.c_str());
   error_msg = "";
 
   // Then remove the icon paths key.
@@ -418,7 +420,7 @@
   ASSERT_TRUE(NULL != action.get());
   error_msg = "";
 
-  // Same test (name key), but with browser action.
+  // Same test (icon paths key), but with browser action.
   copy.reset(static_cast<DictionaryValue*>(input.DeepCopy()));
   copy->Remove(keys::kPageActionIcons, NULL);
   action.reset(extension.LoadExtensionActionHelper(
@@ -445,6 +447,70 @@
   ASSERT_EQ(1u, action->icon_paths().size());
   ASSERT_EQ(kIcon, action->icon_paths()[0]);
   ASSERT_EQ(ExtensionAction::PAGE_ACTION, action->type());
+
+  // Same test, but with browser action.
+  action.reset(extension.LoadExtensionActionHelper(
+      &input, &error_msg, ExtensionAction::BROWSER_ACTION));
+  ASSERT_TRUE(action.get());
+  ASSERT_STREQ("", error_msg.c_str());
+  ASSERT_EQ(kTitle, action->title());
+  ASSERT_EQ(1u, action->icon_paths().size());
+  ASSERT_EQ(kIcon, action->icon_paths()[0]);
+  ASSERT_EQ(ExtensionAction::BROWSER_ACTION, action->type());
+
+  // Invalid title should give an error even with a valid name.
+  input.Clear();
+  input.SetInteger(keys::kPageActionDefaultTitle, 42);
+  input.SetString(keys::kName, name);
+  action.reset(extension.LoadExtensionActionHelper(
+      &input, &error_msg, ExtensionAction::PAGE_ACTION));
+  ASSERT_TRUE(NULL == action.get());
+  ASSERT_STREQ(errors::kInvalidPageActionDefaultTitle, error_msg.c_str());
+  error_msg = "";
+
+  // Invalid name should give an error only with no title.
+  input.SetString(keys::kPageActionDefaultTitle, kTitle);
+  input.SetInteger(keys::kName, 123);
+  action.reset(extension.LoadExtensionActionHelper(
+      &input, &error_msg, ExtensionAction::PAGE_ACTION));
+  ASSERT_TRUE(NULL != action.get());
+  ASSERT_EQ(kTitle, action->title());
+  ASSERT_STREQ("", error_msg.c_str());
+  error_msg = "";
+
+  input.Remove(keys::kPageActionDefaultTitle, NULL);
+  action.reset(extension.LoadExtensionActionHelper(
+      &input, &error_msg, ExtensionAction::PAGE_ACTION));
+  ASSERT_TRUE(NULL == action.get());
+  ASSERT_STREQ(errors::kInvalidPageActionName, error_msg.c_str());
+  error_msg = "";
+
+  // Same test (invalid title) for browser actions.
+  input.Clear();
+  input.SetInteger(keys::kPageActionDefaultTitle, 42);
+  input.SetString(keys::kName, name);
+  action.reset(extension.LoadExtensionActionHelper(
+      &input, &error_msg, ExtensionAction::BROWSER_ACTION));
+  ASSERT_TRUE(NULL == action.get());
+  ASSERT_STREQ(errors::kInvalidPageActionDefaultTitle, error_msg.c_str());
+  error_msg = "";
+
+  // Same tests (invalid name) for browser actions.
+  input.SetString(keys::kPageActionDefaultTitle, kTitle);
+  input.SetInteger(keys::kName, 123);
+  action.reset(extension.LoadExtensionActionHelper(
+      &input, &error_msg, ExtensionAction::BROWSER_ACTION));
+  ASSERT_TRUE(NULL != action.get());
+  ASSERT_EQ(kTitle, action->title());
+  ASSERT_STREQ("", error_msg.c_str());
+  error_msg = "";
+
+  input.Remove(keys::kPageActionDefaultTitle, NULL);
+  action.reset(extension.LoadExtensionActionHelper(
+      &input, &error_msg, ExtensionAction::BROWSER_ACTION));
+  ASSERT_TRUE(NULL == action.get());
+  ASSERT_STREQ(errors::kInvalidPageActionName, error_msg.c_str());
+  error_msg = "";
 }
 
 TEST(ExtensionTest, IdIsValid) {