Issue 2861019: Enable "get html from page" functionality for PyAuto....

Issue 2861019: Enable "get html from page" functionality for PyAuto.... (Closed)

Created:
10 years, 6 months ago by John Grabowski

Modified:
9 years, 6 months ago

Reviewers:
Paweł Hajdan Jr., Nirnimesh

CC:
chromium-reviews, ben+cc_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src/

Visibility:
Public.

Description

Enable "get html from page" functionality for PyAuto. BUG=36184 TEST=Run PyAuto's content.py and make sure it passes. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=50678

Patch Set 3 : '' #

Total comments: 3

Created: 10 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+259 lines, -10 lines)			Patch
M	chrome/browser/automation/automation_provider.h	View	1 2	2 chunks	+10 lines, -0 lines	0 comments	Download
M	chrome/browser/automation/automation_provider.cc	View	1 2	12 chunks	+72 lines, -10 lines	3 comments	Download
M	chrome/browser/automation/automation_provider_observers.h	View	1 2	2 chunks	+21 lines, -0 lines	0 comments	Download
M	chrome/browser/automation/automation_provider_observers.cc	View	1 2	2 chunks	+24 lines, -0 lines	0 comments	Download
M	chrome/test/functional/PYAUTO_TESTS	View	1 2	1 chunk	+1 line, -0 lines	0 comments	Download
A	chrome/test/functional/content.py	View		1 chunk	+92 lines, -0 lines	0 comments	Download
M	chrome/test/pyautolib/pyauto.py	View	1 2	2 chunks	+35 lines, -0 lines	0 comments	Download
M	chrome/test/pyautolib/pyautolib.i	View	1 2	1 chunk	+4 lines, -0 lines	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

Nirnimesh

LGTM, after green trybot. http://codereview.chromium.org/2861019/diff/1/3 File chrome/browser/automation/automation_provider.h (right): http://codereview.chromium.org/2861019/diff/1/3#newcode329 chrome/browser/automation/automation_provider.h:329: Ah, I was doing this ...

10 years, 6 months ago (2010-06-23 21:48:11 UTC) #2

John Grabowski

Feedback applied (incl FilePath::StringType fix); thx. Will wait for green win try server before landing.

10 years, 6 months ago (2010-06-23 22:34:02 UTC) #3

Paweł Hajdan Jr.

Drive-by. Could you do a follow-up CL? http://codereview.chromium.org/2861019/diff/15001/16001 File chrome/browser/automation/automation_provider.cc (right): http://codereview.chromium.org/2861019/diff/15001/16001#newcode1612 chrome/browser/automation/automation_provider.cc:1612: std::string AutomationProvider::JSONErrorString(std::string ...

10 years, 6 months ago (2010-06-24 06:21:48 UTC) #4

John Grabowski

10 years, 6 months ago (2010-06-25 00:14:28 UTC) #5

Yes; http://codereview.chromium.org/2821018

jrg


On Wed, Jun 23, 2010 at 11:21 PM, <phajdan.jr@chromium.org> wrote:

> Drive-by. Could you do a follow-up CL?
>
>
> http://codereview.chromium.org/2861019/diff/15001/16001
> File chrome/browser/automation/automation_provider.cc (right):
>
> http://codereview.chromium.org/2861019/diff/15001/16001#newcode1612
> chrome/browser/automation/automation_provider.cc:1612: std::string
> AutomationProvider::JSONErrorString(std::string err) {
> Could you move this from being AutomationProvider's member to an
> anonymous namespace in this file? This would help to make the header
> file less cluttered.
>
> http://codereview.chromium.org/2861019/diff/15001/16001#newcode1617
> chrome/browser/automation/automation_provider.cc:1617: // Don't allow
> input string to break JSON by embedding quotes.
> How about using functions from base/json/string_escape.h instead of this
> hack?
>
> http://codereview.chromium.org/2861019/diff/15001/16001#newcode2245
> chrome/browser/automation/automation_provider.cc:2245: // if we get
> here, error.
> nit: "if" -> "If".
>
>
> http://codereview.chromium.org/2861019/show
>

Expand Messages | Collapse Messages