| Summary: | [chromium] move dumpcreateview methods to testrunner library | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Dan Carney <dcarney> | ||||||||
| Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | abarth, dglazkov, fishd, jamesr, jochen, tkent+wkapi, webkit.review.bot | ||||||||
| Priority: | P2 | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Dan Carney
2013-01-09 03:01:53 PST
Created attachment 181886 [details]
Patch
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. 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) Created attachment 181925 [details]
Patch
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 Created attachment 181929 [details]
Patch
Comment on attachment 181929 [details]
Patch
LGTM
Comment on attachment 181929 [details] Patch Clearing flags on attachment: 181929 Committed r139231: <http://trac.webkit.org/changeset/139231> All reviewed patches have been landed. Closing bug. |