Allow the automation provider to accept an ID for performing render-view

Allow AutomationProvider to work with other render view containers besides
tabs, particularly extension bg pages, infobars, and popups. This is
needed to enable WebDriver to work with these views.
BUG=93571
TEST=none


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

[dennis_jeffrey, 2011-12-03 00:19:37]

minor nits and a few questions

http://codereview.chromium.org/8790003/diff/1013/chrome/browser/automation/au...
File chrome/browser/automation/automation_provider_json.h (right):

http://codereview.chromium.org/8790003/diff/1013/chrome/browser/automation/au...
chrome/browser/automation/automation_provider_json.h:21: class
TabContentsWrapper;
Where is this needed?

http://codereview.chromium.org/8790003/diff/1013/chrome/browser/automation/au...
File chrome/browser/automation/automation_util.cc (right):

http://codereview.chromium.org/8790003/diff/1013/chrome/browser/automation/au...
chrome/browser/automation/automation_util.cc:399: bool GetTabForId(const
AutomationId& id, TabContents** tab) {
Would it make sense to move this function down to line 483 below?  It's not used
until after that point.

http://codereview.chromium.org/8790003/diff/1013/chrome/browser/automation/au...
File chrome/browser/automation/automation_util.h (right):

http://codereview.chromium.org/8790003/diff/1013/chrome/browser/automation/au...
chrome/browser/automation/automation_util.h:105: // Returns whether the given ID
refers to a actual automation entity.
nit: 'a actual' --> 'an actual'

http://codereview.chromium.org/8790003/diff/1013/chrome/browser/automation/te...
File chrome/browser/automation/testing_automation_provider.cc (right):

http://codereview.chromium.org/8790003/diff/1013/chrome/browser/automation/te...
chrome/browser/automation/testing_automation_provider.cc:4411:
extension_value->SetBoolean("is_user_installed",
there's a reference to "is_internal" in pyauto.py that needs to be changed due
to the change here.

http://codereview.chromium.org/8790003/diff/1013/chrome/browser/automation/te...
chrome/browser/automation/testing_automation_provider.cc:4412: location ==
Extension::INTERNAL || location == Extension::LOAD);
if the location is Extension::INTERNAL, does that also mean it's a
user-installed extension?

http://codereview.chromium.org/8790003/diff/1013/chrome/browser/automation/te...
chrome/browser/automation/testing_automation_provider.cc:6307: DictionaryValue
dict;
Some other functions use this mechanism for sending return values:

scoped_ptr<DictionaryValue> return_value(new DictionaryValue);
// fill up the dict
AutomationJSONReply(this, reply_message).SendSuccess(return_value.get());

Is there a particular reason it's done differently here?  Is one way better than
the other?  Does it matter?

[kkania, 2011-12-03 00:34:31]

http://codereview.chromium.org/8790003/diff/1013/chrome/browser/automation/au...
File chrome/browser/automation/automation_provider_json.h (right):

http://codereview.chromium.org/8790003/diff/1013/chrome/browser/automation/au...
chrome/browser/automation/automation_provider_json.h:21: class
TabContentsWrapper;
On 2011/12/03 00:19:37, dennis_jeffrey wrote:
> Where is this needed?

Removed.

http://codereview.chromium.org/8790003/diff/1013/chrome/browser/automation/au...
File chrome/browser/automation/automation_util.cc (right):

http://codereview.chromium.org/8790003/diff/1013/chrome/browser/automation/au...
chrome/browser/automation/automation_util.cc:399: bool GetTabForId(const
AutomationId& id, TabContents** tab) {
On 2011/12/03 00:19:37, dennis_jeffrey wrote:
> Would it make sense to move this function down to line 483 below?  It's not
used
> until after that point.

Done.

http://codereview.chromium.org/8790003/diff/1013/chrome/browser/automation/au...
File chrome/browser/automation/automation_util.h (right):

http://codereview.chromium.org/8790003/diff/1013/chrome/browser/automation/au...
chrome/browser/automation/automation_util.h:105: // Returns whether the given ID
refers to a actual automation entity.
On 2011/12/03 00:19:37, dennis_jeffrey wrote:
> nit: 'a actual' --> 'an actual'

Done.

http://codereview.chromium.org/8790003/diff/1013/chrome/browser/automation/te...
File chrome/browser/automation/testing_automation_provider.cc (right):

http://codereview.chromium.org/8790003/diff/1013/chrome/browser/automation/te...
chrome/browser/automation/testing_automation_provider.cc:4411:
extension_value->SetBoolean("is_user_installed",
On 2011/12/03 00:19:37, dennis_jeffrey wrote:
> there's a reference to "is_internal" in pyauto.py that needs to be changed due
> to the change here.

Whoops. I'm going to add both.

http://codereview.chromium.org/8790003/diff/1013/chrome/browser/automation/te...
chrome/browser/automation/testing_automation_provider.cc:4412: location ==
Extension::INTERNAL || location == Extension::LOAD);
On 2011/12/03 00:19:37, dennis_jeffrey wrote:
> if the location is Extension::INTERNAL, does that also mean it's a
> user-installed extension?

INTERNAL means its from the internal extensions directory,  and not external.
LOAD means it was loaded from an unpacked extension. Currently, these are the
only two location values that are user installed.

http://codereview.chromium.org/8790003/diff/1013/chrome/browser/automation/te...
chrome/browser/automation/testing_automation_provider.cc:6307: DictionaryValue
dict;
On 2011/12/03 00:19:37, dennis_jeffrey wrote:
> Some other functions use this mechanism for sending return values:
> 
> scoped_ptr<DictionaryValue> return_value(new DictionaryValue);
> // fill up the dict
> AutomationJSONReply(this, reply_message).SendSuccess(return_value.get());
> 
> Is there a particular reason it's done differently here?  Is one way better
than
> the other?  Does it matter?

This one is better because its one less heap allocation and less verbose, but it
doesn't really matter.

[dennis_jeffrey, 2011-12-03 00:58:59]

LGTM

a few nits

http://codereview.chromium.org/8790003/diff/4015/chrome/common/automation_id.h
File chrome/common/automation_id.h (right):

http://codereview.chromium.org/8790003/diff/4015/chrome/common/automation_id....
chrome/common/automation_id.h:18: // 1) supplying an ID of the wrong type can be
detected
add period at end of sentence

http://codereview.chromium.org/8790003/diff/4015/chrome/common/automation_id....
chrome/common/automation_id.h:40: // Constructs a invalid ID.
'a invalid' --> 'an invalid'

[commit-bot: I haz the power, 2011-12-04 00:54:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kkania@chromium.org/8790003/3023

[commit-bot: I haz the power, 2011-12-05 16:30:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kkania@chromium.org/8790003/9001

[commit-bot: I haz the power, 2011-12-05 21:05:49]

Change committed as 113023