Issue 8804011: WebDriver extension support in TestingAutomationProvider.

Issue 8804011: WebDriver extension support in TestingAutomationProvider. (Closed)

Created:
9 years ago by kkania

Modified:
9 years ago

Reviewers:
dennis_jeffrey

CC:
chromium-reviews, Nirnimesh, John Grabowski, kkania, anantha, robertshield, dyu1, Paweł Hajdan Jr., dennis_jeffrey, frankf

Base URL:
http://git.chromium.org/chromium/src.git@master

Visibility:
Public.

More Reviews

Description

Few minor changes to TestingAutomationProvider extension automation: 1) Wait for extension popup to load 2) Wait for extensions hosts to load in WaitForAllViews 3) Allow more than one browser action to be visible in views. 4) Add explicit IsPageActionVisible method BUG=93571 TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=113088

Patch Set 1 : ... #

Patch Set 2 : ... #

Total comments: 12

Patch Set 3 : fix comments and minor issues #

Created: 9 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+361 lines, -142 lines)			Patch
M	chrome/browser/automation/automation_provider_json.h	View		3 chunks	+24 lines, -1 line	0 comments	Download
M	chrome/browser/automation/automation_provider_json.cc	View	1	3 chunks	+63 lines, -3 lines	0 comments	Download
M	chrome/browser/automation/automation_provider_observers.h	View	1 2	3 chunks	+38 lines, -13 lines	0 comments	Download
M	chrome/browser/automation/automation_provider_observers.cc	View	1	5 chunks	+54 lines, -9 lines	0 comments	Download
M	chrome/browser/automation/testing_automation_provider.h	View		2 chunks	+12 lines, -3 lines	0 comments	Download
M	chrome/browser/automation/testing_automation_provider.cc	View	1 2	12 chunks	+160 lines, -107 lines	0 comments	Download
M	chrome/test/pyautolib/pyauto.py	View	1 2	4 chunks	+10 lines, -6 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

dennis_jeffrey

LGTM Just a handful of minor comments. http://codereview.chromium.org/8804011/diff/1055/chrome/browser/automation/automation_provider_observers.h File chrome/browser/automation/automation_provider_observers.h (right): http://codereview.chromium.org/8804011/diff/1055/chrome/browser/automation/automation_provider_observers.h#newcode1650 chrome/browser/automation/automation_provider_observers.h:1650: // loading ...

9 years ago (2011-12-06 00:49:08 UTC) #2

kkania

9 years ago (2011-12-12 17:10:06 UTC) #3

http://codereview.chromium.org/8804011/diff/1055/chrome/browser/automation/au...
File chrome/browser/automation/automation_provider_observers.h (right):

http://codereview.chromium.org/8804011/diff/1055/chrome/browser/automation/au...
chrome/browser/automation/automation_provider_observers.h:1650: // loading
currently.
On 2011/12/06 00:49:08, dennis_jeffrey wrote:
> nit: 'loading currently' --> 'currently loading'

Done.

http://codereview.chromium.org/8804011/diff/1055/chrome/browser/automation/au...
chrome/browser/automation/automation_provider_observers.h:1652: public
content::NotificationObserver {
On 2011/12/06 00:49:08, dennis_jeffrey wrote:
> indent by 1 more space

Done.

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

http://codereview.chromium.org/8804011/diff/1055/chrome/browser/automation/te...
chrome/browser/automation/testing_automation_provider.cc:2244:
handler_map["WaitForAllTabsToStopLoading"] =
This isn't used on the python side yet, just C++.

On 2011/12/06 00:49:08, dennis_jeffrey wrote:
> Do you think it's worthwhile to change the name of this hook on the python
side
> of things too?

http://codereview.chromium.org/8804011/diff/1055/chrome/browser/automation/te...
chrome/browser/automation/testing_automation_provider.cc:4557: this,
reply_message, extension->id());
On 2011/12/06 00:49:08, dennis_jeffrey wrote:
> maybe add a comment to explicitly say that this observer will delete itself?

Done.

http://codereview.chromium.org/8804011/diff/1055/chrome/browser/automation/te...
chrome/browser/automation/testing_automation_provider.cc:4599: // TODO:
Implement the platform-specific GetExtensionId() in
Frank originally put this here, but I'm willing to do it too. I'll put my name

On 2011/12/06 00:49:08, dennis_jeffrey wrote:
> username for the TODO?

http://codereview.chromium.org/8804011/diff/1055/chrome/browser/automation/te...
chrome/browser/automation/testing_automation_provider.cc:4620: this,
reply_message, extension->id());
On 2011/12/06 00:49:08, dennis_jeffrey wrote:
> maybe add a comment to explicitly say that this observer will delete itself?

Done.

Expand Messages | Collapse Messages