Remove the limit of number of commands per extension, but keep the limit of 4 shortcuts per extensi…

chrome/common/extensions/command_unittest.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 "chrome/common/extensions/command.h"
 
 #include "base/memory/scoped_ptr.h"
 #include "base/string_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/utf_string_conversions.h"
@@ skipping 23 matching lines @@
 
   const struct {
     bool expected_result;
     ui::Accelerator accelerator;
     const char* command_name;
     const char* key;
     const char* description;
   } kTests[] = {
     // Negative test (one or more missing required fields). We don't need to
     // test |command_name| being blank as it is used as a key in the manifest,
-    // so it can't be blank (and we CHECK() when it is).
+    // so it can't be blank (and we CHECK() when it is). A blank shortcut is

[Yoyo Zhou, 2013/03/27 00:44:46]

You should mention this in the CL description too.

[Finnur, 2013/03/27 13:00:39]

Done.

+    // permitted.
     { false, none, "command", "",       "" },
     { false, none, "command", "Ctrl+f", "" },
-    { false, none, "command", "",       "description" },
     // Ctrl+Alt is not permitted, see MSDN link in comments in Parse function.
     { false, none, "command", "Ctrl+Alt+F", "description" },
     // Unsupported shortcuts/too many, or missing modifier.
     { false, none, "command", "A",                "description" },
     { false, none, "command", "F10",              "description" },
     { false, none, "command", "Ctrl+F+G",         "description" },
     { false, none, "command", "Ctrl+Alt+Shift+G", "description" },
     // Shift on its own is not supported.
     { false, shift_f, "command", "Shift+F",       "description" },
     { false, shift_f, "command", "F+Shift",       "description" },
     // Basic tests.
+    { true, none,         "command", "",             "description" },
     { true, ctrl_f,       "command", "Ctrl+F",       "description" },
     { true, alt_f,        "command", "Alt+F",        "description" },
     { true, ctrl_shift_f, "command", "Ctrl+Shift+F", "description" },
     { true, alt_shift_f,  "command", "Alt+Shift+F",  "description" },
     { true, ctrl_1,       "command", "Ctrl+1",       "description" },
     // Shortcut token order tests.
     { true, ctrl_f,       "command", "F+Ctrl",       "description" },
     { true, alt_f,        "command", "F+Alt",        "description" },
     { true, ctrl_shift_f, "command", "F+Ctrl+Shift", "description" },
     { true, ctrl_shift_f, "command", "F+Shift+Ctrl", "description" },
     { true, alt_shift_f,  "command", "F+Alt+Shift",  "description" },
     { true, alt_shift_f,  "command", "F+Shift+Alt",  "description" },
     // Case insensitivity is not OK.
     { false, ctrl_f, "command", "Ctrl+f", "description" },
     { false, ctrl_f, "command", "cTrL+F", "description" },
     // Skipping description is OK for browser- and pageActions.
     { true, ctrl_f, "_execute_browser_action", "Ctrl+F", "" },
-    { true, ctrl_f, "_execute_page_action",    "Ctrl+F", "" },
+    { true, ctrl_f, "_execute_page_action",    "Ctrl+ F", "" },

[Yoyo Zhou, 2013/03/27 00:44:46]

Is this intentional?

[Finnur, 2013/03/27 13:00:39]

Intentional? Nope. Removed.

   };
 
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kTests); ++i) {
     // First parse the command as a simple string.
     scoped_ptr<DictionaryValue> input(new DictionaryValue);
     input->SetString("suggested_key", kTests[i].key);
     input->SetString("description", kTests[i].description);
 
     SCOPED_TRACE(std::string("Command name: |") + kTests[i].command_name +
                  "| key: |" + kTests[i].key +
                  "| description: |" + kTests[i].description +
                  "| index: " + base::IntToString(i));
 
     extensions::Command command;
     string16 error;
     bool result =
         command.Parse(input.get(), kTests[i].command_name, i, &error);
 
     EXPECT_EQ(kTests[i].expected_result, result);
     if (result) {
       EXPECT_STREQ(kTests[i].description,
                    UTF16ToASCII(command.description()).c_str());
       EXPECT_STREQ(kTests[i].command_name, command.command_name().c_str());
       EXPECT_EQ(kTests[i].accelerator, command.accelerator());
     }
 
     // Now parse the command as a dictionary of multiple values.
-    input.reset(new DictionaryValue);
-    DictionaryValue* key_dict = new DictionaryValue();
-    key_dict->SetString("default", kTests[i].key);
-    key_dict->SetString("windows", kTests[i].key);
-    key_dict->SetString("mac", kTests[i].key);
-    input->Set("suggested_key", key_dict);
-    input->SetString("description", kTests[i].description);
+    if (kTests[i].key[0] != 0) {

[Yoyo Zhou, 2013/03/27 00:44:46]

'\0' would make it a little clearer here that it's a char comparison.

+      input.reset(new DictionaryValue);
+      DictionaryValue* key_dict = new DictionaryValue();
+      key_dict->SetString("default", kTests[i].key);
+      key_dict->SetString("windows", kTests[i].key);
+      key_dict->SetString("mac", kTests[i].key);
+      input->Set("suggested_key", key_dict);
+      input->SetString("description", kTests[i].description);
 
-    result = command.Parse(input.get(), kTests[i].command_name, i, &error);
+      result = command.Parse(input.get(), kTests[i].command_name, i, &error);
 
-    EXPECT_EQ(kTests[i].expected_result, result);
-    if (result) {
-      EXPECT_STREQ(kTests[i].description,
-                   UTF16ToASCII(command.description()).c_str());
-      EXPECT_STREQ(kTests[i].command_name, command.command_name().c_str());
-      EXPECT_EQ(kTests[i].accelerator, command.accelerator());
+      EXPECT_EQ(kTests[i].expected_result, result);
+      if (result) {
+        EXPECT_STREQ(kTests[i].description,
+                     UTF16ToASCII(command.description()).c_str());
+        EXPECT_STREQ(kTests[i].command_name, command.command_name().c_str());
+        EXPECT_EQ(kTests[i].accelerator, command.accelerator());
+      }
     }
   }
 }
 
 TEST(CommandTest, ExtensionCommandParsingFallback) {
   std::string description = "desc";
   std::string command_name = "foo";
 
   // Test that platform specific keys are honored on each platform, despite
   // fallback being given.
@@ skipping 67 matching lines @@
   key_dict->SetString("windows", "Ctrl+Shift+W");
 #endif
   EXPECT_FALSE(command.Parse(input.get(), command_name, 0, &error));
 
   // Make sure Mac specific keys are not processed on other platforms.
 #if !defined(OS_MACOSX)
   key_dict->SetString("windows", "Command+Shift+M");
   EXPECT_FALSE(command.Parse(input.get(), command_name, 0, &error));
 #endif
 }