[chromium] Clean up dependencies for Canvas2DLayerBridgeTest and GraphicsLayerChromiumTest unit tests
Created attachment 157842 [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 157842 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=157842&action=review > Source/WebKit/chromium/tests/GraphicsLayerChromiumTest.cpp:-122 > - DebugScopedSetMainThread m_main; Ok, but please run these unit tests in a debug build before committing this. > Source/WebKit/chromium/tests/GraphicsLayerChromiumTest.cpp:274 > + EXPECT_TRUE(m_platformLayer->hasActiveAnimation()); I'm not sure that these GraphicsLayerChromiumTests are really doing anything anymore? I agree with your ChangeLog entry that this used to just test conversion of WebCore types into CC types, but now all they're testing is that different types of animations get added and now exist. Does it make sense to even have more than one of these?
Comment on attachment 157842 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=157842&action=review >> Source/WebKit/chromium/tests/GraphicsLayerChromiumTest.cpp:-122 >> - DebugScopedSetMainThread m_main; > > Ok, but please run these unit tests in a debug build before committing this. Now that these tests don't use CC things they can't really get to the compositor thread (and I've verified that the dchecks are happy). >> Source/WebKit/chromium/tests/GraphicsLayerChromiumTest.cpp:274 >> + EXPECT_TRUE(m_platformLayer->hasActiveAnimation()); > > I'm not sure that these GraphicsLayerChromiumTests are really doing anything anymore? I agree with your ChangeLog entry that this used to just test conversion of WebCore types into CC types, but now all they're testing is that different types of animations get added and now exist. Does it make sense to even have more than one of these? That's a good point. I think that the Preserve3DWith.. one is still useful since we're switching an animation between different WebLayers. The rest of these mostly test that GraphicsLayerChromium doesn't bail completely on these animation types. Maybe I should just be checking the return value of addAnimation() ? Note that createTransform...BigRotation() is testing that GraphicsLayerChromium _does_ barf on a particular animation....but that would be better as an AnimationTranslationUtil test. I'll take a pass at cleaning these up.
Created attachment 158105 [details] Patch
Take two - I've moved most of the animation-related tests to AnimationTranslationUtilTest.cpp. I also had to clean up a few unit tests that were leaking their "fake" thread identity.
Comment on attachment 158105 [details] Patch R=me. Thanks for moving those tests around.
Committed r125466: <http://trac.webkit.org/changeset/125466>