[chromium] Enable more of webkit_unit_tests in component builds
Created attachment 187460 [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.
This adds a number of APIs to Platform that don't make sense for normal operation, but I think the benefits of running more webkit_unit_tests in component builds is worth it. This will compile right now, but without https://codereview.chromium.org/12220091/ the tests won't work.
Comment on attachment 187460 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=187460&action=review > Source/Platform/chromium/public/Platform.h:366 > + virtual void registerMockedURL(const WebURL&, const WebURLResponse&, const WebString& filePath) { } Would it make sense to lump all these methods together behind a tear-off interface? That way, in Chrome we could just make the method to get the tear-off return NULL. > Source/Platform/chromium/public/Platform.h:382 > // Tracing ------------------------------------------------------------- nit: two new lines above section comment
LevelDBTest.cpp is the one holdout. It uses webkit_support::ScopedTempDirectory / webkit_support::CreateScopedTempDirectory which are thin wrappers around base::ScopedTempDir. This would be possible to plumb this through as well and remove the component/non-component distinction for webkit_unit_tests completely, but that's probably for another patch.
(In reply to comment #4) > (From update of attachment 187460 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=187460&action=review > > > Source/Platform/chromium/public/Platform.h:366 > > + virtual void registerMockedURL(const WebURL&, const WebURLResponse&, const WebString& filePath) { } > > Would it make sense to lump all these methods together behind a tear-off interface? That way, in Chrome we could just make the method to get the tear-off return NULL. Sure. WebTesting? WebTestingSupport?
(In reply to comment #6) > (In reply to comment #4) > > (From update of attachment 187460 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=187460&action=review > > > > > Source/Platform/chromium/public/Platform.h:366 > > > + virtual void registerMockedURL(const WebURL&, const WebURLResponse&, const WebString& filePath) { } > > > > Would it make sense to lump all these methods together behind a tear-off interface? That way, in Chrome we could just make the method to get the tear-off return NULL. > > Sure. WebTesting? WebTestingSupport? I'd probably go with WebTestingSupport. We have other Web*Support tear-offs.
Created attachment 187462 [details] with tearoff WebUnitTestSupport interface
To keep things running, I'll have to: 1.) Check in the Platform/...h changes without the unit test changes so the header is available in chromium 2.) Roll (1) into chromium 3.) Check in chromium implementation of test functions 4.) Roll (3) into WebKit 5.) Check in WebKit/chromium/tests/... changes
(In reply to comment #9) > To keep things running, I'll have to: > Slight refinement: 1.) Check in the Platform/...h changes without the unit test changes so the header is available in chromium with #define HAVE_WEBUNITTESTSUPPORT 2.) Check in chromium implementation with #include/etc guarded by #if HAVE_WEBUNITTESTSUPPORT 3.) Wait for (1) to roll into (2), roll (2) into WebKit 4.) Check in WebKit/chromium/tests/... changes 5.) Remove #define and #ifs at leisure
Committed r142388: <http://trac.webkit.org/changeset/142388>
(In reply to comment #11) > Committed r142388: <http://trac.webkit.org/changeset/142388> This is the Platform API, still need to land the chromium side + roll it in + land the actual unit test changes.
The chromium side change has rolled in, but in local testing I'm finding many of the tests that will now start running in debug+component builds are flaky or buggy. It seems that pretty much none of the tests in WebFrameTest.cpp clean up properly. I guess this is somewhat expected given that they haven't run on any bots in debug mode in forever :/
Created attachment 187500 [details] Patch for landing
(In reply to comment #14) > Created an attachment (id=187500) [details] > Patch for landing http://www.youtube.com/watch?v=FONN-0uoTHI
Comment on attachment 187500 [details] Patch for landing Clearing flags on attachment: 187500 Committed r142422: <http://trac.webkit.org/changeset/142422>
All reviewed patches have been landed. Closing bug.
This is great. Thanks for doing this.