use EXTENSION_FUNCTION_VALIDATE

use EXTENSION_FUNCTION_VALIDATE in extension_tabs_module.cc

BUG=11200
R=erikkay

Committed: http://src.chromium.org/viewvc/14976

[Erik does not do reviews, 2009-04-30 17:28:38]

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

http://codereview.chromium.org/100213/diff/1/3#newcode49
Line 49: EXTENSION_FUNCTION_VALIDATE(args->Get(L"ids", &ids_value) &&
use GetList instead

http://codereview.chromium.org/100213/diff/1/3#newcode148
Line 148: EXTENSION_FUNCTION_VALIDATE(args_->IsType(Value::TYPE_INTEGER) &&
just use GetAsInteger

http://codereview.chromium.org/100213/diff/1/3#newcode234
Line 234: EXTENSION_FUNCTION_VALIDATE(args_->IsType(Value::TYPE_INTEGER) &&
again, just GetAsInteger

http://codereview.chromium.org/100213/diff/1/3#newcode303
Line 303: args->GetInteger(L"index", &new_index) &&
For clarity's sake, I think these compound statements should all be split up. 
When we get a crash, it will be more helpful to know which one of these tests
failed (I didn't bother marking the other cases of these).

http://codereview.chromium.org/100213/diff/1/3#newcode337
Line 337: args_->GetAsInteger(&tab_id));
GetAsInteger is enough

[rafaelw, 2009-04-30 17:44:57]

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

http://codereview.chromium.org/100213/diff/1/3#newcode49
Line 49: EXTENSION_FUNCTION_VALIDATE(args->Get(L"ids", &ids_value) &&
On 2009/04/30 17:28:39, Erik Kay wrote:
> use GetList instead

Done.

http://codereview.chromium.org/100213/diff/1/3#newcode148
Line 148: EXTENSION_FUNCTION_VALIDATE(args_->IsType(Value::TYPE_INTEGER) &&
On 2009/04/30 17:28:39, Erik Kay wrote:
> just use GetAsInteger

Done.

http://codereview.chromium.org/100213/diff/1/3#newcode234
Line 234: EXTENSION_FUNCTION_VALIDATE(args_->IsType(Value::TYPE_INTEGER) &&
On 2009/04/30 17:28:39, Erik Kay wrote:
> again, just GetAsInteger

Done.

http://codereview.chromium.org/100213/diff/1/3#newcode303
Line 303: args->GetInteger(L"index", &new_index) &&
On 2009/04/30 17:28:39, Erik Kay wrote:
> For clarity's sake, I think these compound statements should all be split up. 
> When we get a crash, it will be more helpful to know which one of these tests
> failed (I didn't bother marking the other cases of these).

Done.

http://codereview.chromium.org/100213/diff/1/3#newcode337
Line 337: args_->GetAsInteger(&tab_id));
On 2009/04/30 17:28:39, Erik Kay wrote:
> GetAsInteger is enough

Done.

[Erik does not do reviews, 2009-04-30 17:52:08]

LGTM

http://codereview.chromium.org/100213/diff/1002/9
File chrome/browser/extensions/extension_tabs_module.cc (right):

http://codereview.chromium.org/100213/diff/1002/9#newcode185
Line 185: Browser* browser = BrowserList::GetLastActive();
Not related to this CL I know, but shouldn't all of these be changed to
GetLastActiveWithProfile?  There's now a profile() method on all of these
function objects.

http://codereview.chromium.org/100213/diff/1002/9#newcode190
Line 190: const DictionaryValue *args = static_cast<const
DictionaryValue*>(args_);
move this line up next to the validate for clarity

[rafaelw, 2009-04-30 18:35:37]

http://codereview.chromium.org/100213/diff/1002/9
File chrome/browser/extensions/extension_tabs_module.cc (right):

http://codereview.chromium.org/100213/diff/1002/9#newcode190
Line 190: const DictionaryValue *args = static_cast<const
DictionaryValue*>(args_);
On 2009/04/30 17:52:08, Erik Kay wrote:
> move this line up next to the validate for clarity

Done.