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

Remove the limit of number of commands per extension, but keep the limit of 4 commands *with shortcuts* per extension.

To facilitate this, commands without shortcuts are now allowed.

BUG=167419
TEST=Automated test covers this, but you can manually test by adding an extension with 5 commands, 4 of which specify shortcuts in the manifest and see that it works. Once you add the manifest shortcut to the fifth extension, it should fail to load. The user should not be bound by this limit when adding shortcuts through chrome://extensions.
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=190921

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

LGTM with nits

https://codereview.chromium.org/12965009/diff/11011/chrome/common/extensions/...
File chrome/common/extensions/command.cc (right):

https://codereview.chromium.org/12965009/diff/11011/chrome/common/extensions/...
chrome/common/extensions/command.cc:213:
extension_manifest_values::kPageActionCommandEvent &&
If you use values instead of extension_manifest_values this would look less
ugly.

https://codereview.chromium.org/12965009/diff/11011/chrome/common/extensions/...
File chrome/common/extensions/command_unittest.cc (right):

https://codereview.chromium.org/12965009/diff/11011/chrome/common/extensions/...
chrome/common/extensions/command_unittest.cc:44: // so it can't be blank (and we
CHECK() when it is). A blank shortcut is
You should mention this in the CL description too.

https://codereview.chromium.org/12965009/diff/11011/chrome/common/extensions/...
chrome/common/extensions/command_unittest.cc:77: { true, ctrl_f,
"_execute_page_action",    "Ctrl+ F", "" },
Is this intentional?

https://codereview.chromium.org/12965009/diff/11011/chrome/common/extensions/...
chrome/common/extensions/command_unittest.cc:105: if (kTests[i].key[0] != 0) {
'\0' would make it a little clearer here that it's a char comparison.

[Finnur, 2013-03-27 13:00:38]

https://codereview.chromium.org/12965009/diff/11011/chrome/common/extensions/...
File chrome/common/extensions/command.cc (right):

https://codereview.chromium.org/12965009/diff/11011/chrome/common/extensions/...
chrome/common/extensions/command.cc:213:
extension_manifest_values::kPageActionCommandEvent &&
Good point.

https://codereview.chromium.org/12965009/diff/11011/chrome/common/extensions/...
File chrome/common/extensions/command_unittest.cc (right):

https://codereview.chromium.org/12965009/diff/11011/chrome/common/extensions/...
chrome/common/extensions/command_unittest.cc:44: // so it can't be blank (and we
CHECK() when it is). A blank shortcut is
On 2013/03/27 00:44:46, Yoyo Zhou wrote:
> You should mention this in the CL description too.

Done.

https://codereview.chromium.org/12965009/diff/11011/chrome/common/extensions/...
chrome/common/extensions/command_unittest.cc:77: { true, ctrl_f,
"_execute_page_action",    "Ctrl+ F", "" },
Intentional? Nope. Removed.

[Finnur, 2013-03-27 13:04:48]

Committed patchset #5 manually as r190921 (presubmit successful).