[chromium] move WebThemeEngine implementations to TestRunner library
Created attachment 193940 [details] Patch
For some reason this doesn't compile on Windows: 57>E:\b\build\slave\win_layout\build\src\third_party\WebKit\Tools\DumpRenderTree\chromium\TestRunner\src\WebThemeControlDRTWin.cpp(280): error C3861: 'failedAssertion': identifier not found 57>E:\b\build\slave\win_layout\build\src\third_party\WebKit\Tools\DumpRenderTree\chromium\TestRunner\src\WebThemeControlDRTWin.cpp(313): error C3861: 'failedAssertion': identifier not found 57>E:\b\build\slave\win_layout\build\src\third_party\WebKit\Tools\DumpRenderTree\chromium\TestRunner\src\WebThemeControlDRTWin.cpp(318): error C3861: 'failedAssertion': identifier not found 57>E:\b\build\slave\win_layout\build\src\third_party\WebKit\Tools\DumpRenderTree\chromium\TestRunner\src\WebThemeControlDRTWin.cpp(484): error C3861: 'failedAssertion': identifier not found These are WEBKIT_ASSERT_NOT_REACHED() calls which is defined in WebCommon.h included by TestCommon.h included by WebThemeControlDRTWin.cpp So given that the macro expands, and it's also working in the other files that use it, I have no idea what's going on :-/
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 193940 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193940&action=review > Tools/DumpRenderTree/DumpRenderTree.gypi:93 > + 'chromium/TestRunner/src/WebThemeControlDRTWin.cpp', > + 'chromium/TestRunner/src/WebThemeControlDRTWin.h', The suffix DRT doesn't really make sense inside TestRunner....
Comment on attachment 193940 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193940&action=review > Tools/DumpRenderTree/chromium/TestRunner/src/WebThemeControlDRTWin.cpp:41 > +#include "skia/ext/skia_utils_win.h" Do we need to add a dependency on skia to the TestRunner GYP declarations?
(In reply to comment #5) > (From update of attachment 193940 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=193940&action=review > > > Tools/DumpRenderTree/chromium/TestRunner/src/WebThemeControlDRTWin.cpp:41 > > +#include "skia/ext/skia_utils_win.h" > > Do we need to add a dependency on skia to the TestRunner GYP declarations? It's already there for the platform canvas in WebTestProxy
Created attachment 193946 [details] Patch
(In reply to comment #4) > (From update of attachment 193940 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=193940&action=review > > > Tools/DumpRenderTree/DumpRenderTree.gypi:93 > > + 'chromium/TestRunner/src/WebThemeControlDRTWin.cpp', > > + 'chromium/TestRunner/src/WebThemeControlDRTWin.h', > > The suffix DRT doesn't really make sense inside TestRunner.... Renamed WebTheme(.*)DRT(.*) to WebTestTheme$1$2
Comment on attachment 193946 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193946&action=review rs=me > Tools/DumpRenderTree/chromium/TestRunner/src/TestInterfaces.h:90 > + std::auto_ptr<WebKit::WebThemeEngine> m_themeEngine; Isn't this the bad case for auto_ptr? Rather than forward declaring WebThemeEngine, we should probably #include it.
Created attachment 193955 [details] Patch for landing
(In reply to comment #9) > (From update of attachment 193946 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=193946&action=review > > rs=me > > > Tools/DumpRenderTree/chromium/TestRunner/src/TestInterfaces.h:90 > > + std::auto_ptr<WebKit::WebThemeEngine> m_themeEngine; > > Isn't this the bad case for auto_ptr? Rather than forward declaring WebThemeEngine, we should probably #include it. done I also figured out the reason for the compile failure, it was missing a using namespace WebKit...
Comment on attachment 193955 [details] Patch for landing Clearing flags on attachment: 193955 Committed r146290: <http://trac.webkit.org/changeset/146290>
All reviewed patches have been landed. Closing bug.
This broke the Chromium Mac build: http://build.webkit.org/builders/Chromium%20Mac%20Release/builds/58149 Rolling it out.
Reverted r146290 for reason: Broke Chromium Mac build. Committed r146293: <http://trac.webkit.org/changeset/146293>
FYI, this broke the Windows build too: http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Win%20Builder/builds/37214
Created attachment 193987 [details] Patch for landing
Comment on attachment 193987 [details] Patch for landing Clearing flags on attachment: 193987 Committed r146304: <http://trac.webkit.org/changeset/146304>
Re-opened since this is blocked by bug 112788
Created attachment 194183 [details] Patch for landing
Comment on attachment 194183 [details] Patch for landing Clearing flags on attachment: 194183 Committed r146439: <http://trac.webkit.org/changeset/146439>