Bug 224410 – Simplify WebKitTestRunner preference reset to be more like DRT

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Sam Weinig]

Simplify WebKitTestRunner preference reset to be more like DRT

[Sam Weinig]

Created attachment 425695 [details]
Patch

[Sam Weinig]

Created attachment 425711 [details]
Patch

[Darin Adler]

Comment on attachment 425711 [details]
Patch

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

> Source/WebKit/UIProcess/API/C/WKPreferencesRefPrivate.h:54
> +WK_EXPORT void WKPreferencesStartBatchingUpdates(WKPreferencesRef);
> +WK_EXPORT void WKPreferencesEndBatchingUpdates(WKPreferencesRef);

What’s the story on adding new C API (including private/SPI) vs. Objective-C/Swift API? Are there cross-platform reason to keep adding to the C API? I know that on Apple platforms we want the Objective-C/Swift to have parity or be a superset.

> Tools/WebKitTestRunner/TestOptions.cpp:60
>              { "AcceleratedDrawingEnabled", false },

Is there some way to share this list with DumpRenderTree?

[Sam Weinig]

(In reply to Darin Adler from comment #3)
> Comment on attachment 425711 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=425711&action=review
> 
> > Source/WebKit/UIProcess/API/C/WKPreferencesRefPrivate.h:54
> > +WK_EXPORT void WKPreferencesStartBatchingUpdates(WKPreferencesRef);
> > +WK_EXPORT void WKPreferencesEndBatchingUpdates(WKPreferencesRef);
> 
> What’s the story on adding new C API (including private/SPI) vs.
> Objective-C/Swift API? Are there cross-platform reason to keep adding to the
> C API? I know that on Apple platforms we want the Objective-C/Swift to have
> parity or be a superset.

Anything that WebKitTestRunner uses needs either needs to be in the C-SPI or duplicated in the port specific APIs. So, for things like this, that at the moment are mostly about the test runner, I am keeping them in C.

> 
> > Tools/WebKitTestRunner/TestOptions.cpp:60
> >              { "AcceleratedDrawingEnabled", false },
> 
> Is there some way to share this list with DumpRenderTree?

I hope so. My goal is to converge DumpRenderTree and WebKitTestRunner TestOptions.h/cpp files (they are very similar now), and then share them via the TestRunnerShared directory. That allows us to maintain some differences via #ifdefs if we need it (since each project will still compile their own copy), and we will need it, as there are some preferences WebKitLegacy does not want enabled, as they don't work there.

[EWS]

Committed r275810 (236381@main): <https://commits.webkit.org/236381@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 425711 [details].

[Radar WebKit Bug Importer]

<rdar://problem/76514638>

[Fujii Hironori]

Filed: Bug 225087 – REGRESSION(r275810): [WebKitTestRunner] fast/text/basic/004.html fails after running fast/layoutformattingcontext tests

[Fujii Hironori]

(In reply to Sam Weinig from comment #4)
> That allows us to maintain some differences
> via #ifdefs if we need it (since each project will still compile their own copy),

WinCairo is still supporting both WK1 and WK2. But, it doesn't compile TestRunnerShared twice. (Bug 216465)

[Sam Weinig]

(In reply to Fujii Hironori from comment #8)
> (In reply to Sam Weinig from comment #4)
> > That allows us to maintain some differences
> > via #ifdefs if we need it (since each project will still compile their own copy),
> 
> WinCairo is still supporting both WK1 and WK2. But, it doesn't compile
> TestRunnerShared twice. (Bug 216465)

It unfortunately will need to. While TestRunnerShared is for shared code between DRT and WKTR, it needs to be shared only at the source level to allow preferences to differ between the WK1 and WK2.

[Fujii Hironori]

(In reply to Sam Weinig from comment #9)
> (In reply to Fujii Hironori from comment #8)
> > (In reply to Sam Weinig from comment #4)
> > > That allows us to maintain some differences
> > > via #ifdefs if we need it (since each project will still compile their own copy),
> > 
> > WinCairo is still supporting both WK1 and WK2. But, it doesn't compile
> > TestRunnerShared twice. (Bug 216465)
> 
> It unfortunately will need to. While TestRunnerShared is for shared code
> between DRT and WKTR, it needs to be shared only at the source level to
> allow preferences to differ between the WK1 and WK2.

TestRunnerShared is a library shared between DRT and WKTR. It should be possible to change the behavior by using a runtime flag.

[Darin Adler]

(In reply to Fujii Hironori from comment #10)
> TestRunnerShared is a library shared between DRT and WKTR. It should be
> possible to change the behavior by using a runtime flag.

It is shared code. It doesn’t have to be a shared library.

Let us build it separately for the two uses. It is our library, we get to decide how it's built.

I don’t agree that we need to change it so it can be a single shared library with a runtime flag. We *could* do that, or we could compile it twice on platforms that support both legacy WebKit and modern.

I prefer the "compile it twice" approach.