RESOLVED FIXED 106434
[chromium] move dumpcreateview methods to testrunner library 
https://bugs.webkit.org/show_bug.cgi?id=106434
Summary [chromium] move dumpcreateview methods to testrunner library
Dan Carney
Reported 2013-01-09 03:01:53 PST
[chromium] move dumpcreateview methods to testrunner library
Attachments
Patch (8.79 KB, patch)
2013-01-09 03:02 PST, Dan Carney 		no flags
DetailsFormatted DiffDiff
Patch (15.97 KB, patch)
2013-01-09 07:45 PST, Dan Carney 		no flags
DetailsFormatted DiffDiff
Patch (16.12 KB, patch)
2013-01-09 08:08 PST, Dan Carney 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Dan Carney
Comment 1 2013-01-09 03:02:27 PST
Created attachment 181886 [details] Patch
WebKit Review Bot
Comment 2 2013-01-09 03:04:19 PST
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
jochen
Comment 3 2013-01-09 04:06:44 PST
Comment on attachment 181886 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=181886&action=review I would recommend to move this method together with canOpenWindows, so you can move the first two if statements of WebViewHost::createView to WebTestProxyBase::createView (and add WebTestProxy::createView to invoke it) > Tools/DumpRenderTree/chromium/TestRunner/src/TestRunner.h:169 > + void dumpCreateView(const CppArgumentList&, CppVariant*); this should go in the section of methods that modify TestRunner's state (after dumpTitleChanges iirc)
Dan Carney
Comment 4 2013-01-09 07:45:27 PST
Created attachment 181925 [details] Patch
jochen
Comment 5 2013-01-09 07:57:35 PST
Comment on attachment 181925 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=181925&action=review > Tools/DumpRenderTree/chromium/TestRunner/public/WebTestProxy.h:58 > +class WebURLRequest; these should be alphabetically sorted. First all classes, then all structs > Tools/DumpRenderTree/chromium/TestRunner/public/WebTestRunner.h:60 > + virtual bool canOpenWindows() { return false; } const missing > Tools/DumpRenderTree/chromium/TestRunner/src/WebTestProxy.cpp:146 > +static string URLDescription(const GURL& url) you can remove the static and move it in the anonymous namespace above
Dan Carney
Comment 6 2013-01-09 08:08:08 PST
Created attachment 181929 [details] Patch
jochen
Comment 7 2013-01-09 08:09:24 PST
Comment on attachment 181929 [details] Patch LGTM
WebKit Review Bot
Comment 8 2013-01-09 13:08:37 PST
Comment on attachment 181929 [details] Patch Clearing flags on attachment: 181929 Committed r139231: <http://trac.webkit.org/changeset/139231>
WebKit Review Bot
Comment 9 2013-01-09 13:08:41 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.