Add option --dump-render-tree to content_shell to dump the render tree as text.

Add option --dump-render-tree to content_shell to dump the render tree as text.

BUG=111316
TEST=run out/Debug/content_shell --dump-render-tree file://$(pwd)/third_party/WebKit/LayoutTests/fast/html/section-element.html > section-element-actual.txt - should be the same as section-element-expected.txt


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

[jochen (gone - plz use gerrit), 2012-01-26 13:09:32]

John, Dirk, plz review

I picked --tree as DumpRenderTree has --notree to disable text output

[jam, 2012-01-26 18:47:34]

sweet, some nits

http://codereview.chromium.org/9289045/diff/1/content/public/browser/web_cont...
File content/public/browser/web_contents_delegate.h (right):

http://codereview.chromium.org/9289045/diff/1/content/public/browser/web_cont...
content/public/browser/web_contents_delegate.h:119: virtual void
DidFinishLoad(WebContents* contents) {}
we should avoid adding methods to this interface since it's already big,
especially if that callback can be gathered from another place, i.e.
WebContentsObserver

http://codereview.chromium.org/9289045/diff/1/content/shell/shell_browser_main.h
File content/shell/shell_browser_main.h (right):

http://codereview.chromium.org/9289045/diff/1/content/shell/shell_browser_mai...
content/shell/shell_browser_main.h:49: bool dump_as_text_;
do we really need a member variable for this, instead of having whoever cares
check the command line?

http://codereview.chromium.org/9289045/diff/1/content/shell/shell_render_view...
File content/shell/shell_render_view_observer.cc (right):

http://codereview.chromium.org/9289045/diff/1/content/shell/shell_render_view...
content/shell/shell_render_view_observer.cc:23: std::string
dumpDocumentText(WebFrame* frame) {
nit: here and below, CamelCase

http://codereview.chromium.org/9289045/diff/1/content/shell/shell_switches.cc
File content/shell/shell_switches.cc (right):

http://codereview.chromium.org/9289045/diff/1/content/shell/shell_switches.cc...
content/shell/shell_switches.cc:10: const char kTree[] = "tree";
nit: i think when looking at DRT, it's very clear in that context what --notree
means. here because content_shell does much more than just dump render tree, can
we make this --dump-render-tree? that would make it obvious to everyone

[jochen (gone - plz use gerrit), 2012-01-27 08:49:55]

http://codereview.chromium.org/9289045/diff/1/content/public/browser/web_cont...
File content/public/browser/web_contents_delegate.h (right):

http://codereview.chromium.org/9289045/diff/1/content/public/browser/web_cont...
content/public/browser/web_contents_delegate.h:119: virtual void
DidFinishLoad(WebContents* contents) {}
On 2012/01/26 18:47:34, John Abd-El-Malek wrote:
> we should avoid adding methods to this interface since it's already big,
> especially if that callback can be gathered from another place, i.e.
> WebContentsObserver

Done.

http://codereview.chromium.org/9289045/diff/1/content/shell/shell_browser_main.h
File content/shell/shell_browser_main.h (right):

http://codereview.chromium.org/9289045/diff/1/content/shell/shell_browser_mai...
content/shell/shell_browser_main.h:49: bool dump_as_text_;
On 2012/01/26 18:47:34, John Abd-El-Malek wrote:
> do we really need a member variable for this, instead of having whoever cares
> check the command line?

Done.

http://codereview.chromium.org/9289045/diff/1/content/shell/shell_render_view...
File content/shell/shell_render_view_observer.cc (right):

http://codereview.chromium.org/9289045/diff/1/content/shell/shell_render_view...
content/shell/shell_render_view_observer.cc:23: std::string
dumpDocumentText(WebFrame* frame) {
On 2012/01/26 18:47:34, John Abd-El-Malek wrote:
> nit: here and below, CamelCase

Done.

http://codereview.chromium.org/9289045/diff/1/content/shell/shell_switches.cc
File content/shell/shell_switches.cc (right):

http://codereview.chromium.org/9289045/diff/1/content/shell/shell_switches.cc...
content/shell/shell_switches.cc:10: const char kTree[] = "tree";
On 2012/01/26 18:47:34, John Abd-El-Malek wrote:
> nit: i think when looking at DRT, it's very clear in that context what
--notree
> means. here because content_shell does much more than just dump render tree,
can
> we make this --dump-render-tree? that would make it obvious to everyone

Done.

[Dirk Pranke, 2012-01-27 17:43:11]

looks more or less fine to me. I defer to John for the approval.

[jam, 2012-01-27 18:22:30]

http://codereview.chromium.org/9289045/diff/6001/content/shell/shell.cc
File content/shell/shell.cc (right):

http://codereview.chromium.org/9289045/diff/6001/content/shell/shell.cc#newco...
content/shell/shell.cc:49: void ShellObserver::DidFinishLoad(int64 frame_id,
nit: up to you, but if a class is declared and defined in a cc file, it's always
shorter (and IMO more readable) to inline all the methods).

http://codereview.chromium.org/9289045/diff/6001/content/shell/shell.cc#newco...
content/shell/shell.cc:114: new ShellObserver(tab_contents);
actually, do you need a new WebContentsObserver or can you just make Shell be a
WCO?

[jochen (gone - plz use gerrit), 2012-01-28 19:10:27]

PTAL

I merged the WebContentsObserver into the Shell class

[jam, 2012-01-28 21:13:52]

lgtm

[commit-bot: I haz the power, 2012-01-28 21:22:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jochen@chromium.org/9289045/13001

[commit-bot: I haz the power, 2012-01-29 02:44:22]

Try job failure for 9289045-13001 (retry) on win_rel for step "browser_tests".
It's a second try, previously, steps "browser_tests, ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-01-29 08:16:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jochen@chromium.org/9289045/13001

[commit-bot: I haz the power, 2012-01-29 10:57:36]

Change committed as 119642