Make chrome.tabs.update's tabId parameter optional.

Make chrome.tabs.update's tabId parameter optional.

BUG=97330
TEST=added appropriate browser test


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=102235

[miket_OOO, 2011-09-21 22:49:41]

YOUR INPUT REQUESTED THANKS

[Mihai Parparita -not on Chrome, 2011-09-21 23:24:05]

http://codereview.chromium.org/7981040/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_tabs_module.cc (right):

http://codereview.chromium.org/7981040/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_tabs_module.cc:898: Value* tab_value = NULL;
ExecuteCodeInTabFunction::RunImpl, ReloadTabFunction::RunImpl, and
DetectTabLanguageFunction::RunImpl all have very similar code. It seems like
it's time to move it to a helper function (perhaps by expanding what
ExtensionTabUtil::GetDefaultTab does).

http://codereview.chromium.org/7981040/diff/1/chrome/common/extensions/api/ex...
File chrome/common/extensions/api/extension_api.json (right):

http://codereview.chromium.org/7981040/diff/1/chrome/common/extensions/api/ex...
chrome/common/extensions/api/extension_api.json:1538: "type": "integer",
You seem to have ended up with some tabs on these lines.

http://codereview.chromium.org/7981040/diff/1/chrome/test/data/extensions/api...
File chrome/test/data/extensions/api_test/tabs/basics/update.js (right):

http://codereview.chromium.org/7981040/diff/1/chrome/test/data/extensions/api...
chrome/test/data/extensions/api_test/tabs/basics/update.js:1: var tabIds = new
Array();
The presubmit check will nag you about a missing license block in the .js file.

Also, var tabIds = [] is preferred to using new Array.

http://codereview.chromium.org/7981040/diff/1/chrome/test/data/extensions/api...
chrome/test/data/extensions/api_test/tabs/basics/update.js:33: function
testUpdatingBySpecificTabIds() {
This is already tested by update in crud2.html.

http://codereview.chromium.org/7981040/diff/1/chrome/test/data/extensions/api...
chrome/test/data/extensions/api_test/tabs/basics/update.js:58: }));}));}));
This looks hard to read or edit, perhaps keeping the closing braces on their own
lines at the indentation levels that they were opened at would help (see nested
callbacks in crud2.html and other test).

[miket_OOO, 2011-09-22 00:06:44]

Please see comments.

http://codereview.chromium.org/7981040/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_tabs_module.cc (right):

http://codereview.chromium.org/7981040/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_tabs_module.cc:898: Value* tab_value = NULL;
On 2011/09/21 23:24:05, Mihai Parparita wrote:
> ExecuteCodeInTabFunction::RunImpl, ReloadTabFunction::RunImpl, and
> DetectTabLanguageFunction::RunImpl all have very similar code. It seems like
> it's time to move it to a helper function (perhaps by expanding what
> ExtensionTabUtil::GetDefaultTab does).

Yes, I agree. If I promise not to avoid doing it, would you be OK with my doing
it in a separate CL? I would indeed enjoy refactoring this code.

http://codereview.chromium.org/7981040/diff/1/chrome/common/extensions/api/ex...
File chrome/common/extensions/api/extension_api.json (right):

http://codereview.chromium.org/7981040/diff/1/chrome/common/extensions/api/ex...
chrome/common/extensions/api/extension_api.json:1538: "type": "integer",
On 2011/09/21 23:24:05, Mihai Parparita wrote:
> You seem to have ended up with some tabs on these lines.

Fixed. I saw this but misread the filename and thought it was generated code.

http://codereview.chromium.org/7981040/diff/1/chrome/test/data/extensions/api...
File chrome/test/data/extensions/api_test/tabs/basics/update.js (right):

http://codereview.chromium.org/7981040/diff/1/chrome/test/data/extensions/api...
chrome/test/data/extensions/api_test/tabs/basics/update.js:1: var tabIds = new
Array();
On 2011/09/21 23:24:05, Mihai Parparita wrote:
> The presubmit check will nag you about a missing license block in the .js
file.
> 
> Also, var tabIds = [] is preferred to using new Array.

Everything in this file is fixed.

[Mihai Parparita -not on Chrome, 2011-09-22 00:22:19]

lgtm

[commit-bot: I haz the power, 2011-09-22 03:59:03]

Change committed as 102235