Bug 46907 – [Chromium] Enable webkit unit tests in chromium multi-dll build

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Victor Wang]

In chromium Windows multi-dll build, webkit is built as a shared library (dll). To run the webkit unit tests in this mode, we need to compile the unit tests code in the webkit.dll and export an webkit api that launches all the tests.

[Victor Wang]

Created attachment 69341 [details]
Proposed Patch

[Darin Fisher (:fishd, Google)]

Comment on attachment 69341 [details]
Proposed Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=69341&action=review

> WebKit/chromium/tests/RunAllTests.cpp:44
> +#if defined(WIN32) && defined(WEBKIT_DLL_UNITTEST)

is it possible to unify these two branches so that we can avoid the
#ifdef here?

[Victor Wang]

(In reply to comment #2)
> (From update of attachment 69341 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=69341&action=review
> 
> > WebKit/chromium/tests/RunAllTests.cpp:44
> > +#if defined(WIN32) && defined(WEBKIT_DLL_UNITTEST)
> 
> is it possible to unify these two branches so that we can avoid the
> #ifdef here?

Not sure a good way to do this. The atExitManager (TestSuite) must to be created before WebKit support SetUpTestEnvironment, but it can not be created more than once in the binary.

[Darin Fisher (:fishd, Google)]

Also, it would be nice to avoid directly using base::AtExitManager from the WebKit code base.  We need to hide such dependencies.  Otherwise, it will be very difficult to ever rename AtExitManager or change the way it works (if we wanted to do so).  Circular dependencies between Chromium and WebKit repositories add pain.

I have some ideas about how to resolve this.  Perhaps we could introduce a method to webkit_support that makes a callback to run the tests.  Then, the details of how the AtExitManager gets initialized could live in the Chromium repository.

[Victor Wang]

Created attachment 69972 [details]
Proposed Patch

Do not see a simple way to avoid #ifdef. New patch uploaded per our discussion, it hides the AtExitManager by creating a TestSuite instance.

[Darin Fisher (:fishd, Google)]

Comment on attachment 69972 [details]
Proposed Patch

R=me

[Victor Wang]

Committed r69243: <http://trac.webkit.org/changeset/69243>

[WebKit Review Bot]

http://trac.webkit.org/changeset/69243 might have broken Chromium Win Release

[Victor Wang]

(In reply to comment #8)
> http://trac.webkit.org/changeset/69243 might have broken Chromium Win Release

Think this is false alarm. The build was broken by another patch (http://trac.webkit.org/changeset/69247) and was fixed later (http://trac.webkit.org/changeset/69251).