When an iframe gains composited content, we should toggle the enclosing frame into compositing mode, both to preserve layering (since main frame content can overlay iframes), and so that 3d-transformed iframes work.
Created attachment 53867 [details] Patch
Comment on attachment 53867 [details] Patch > diff --git a/WebCore/rendering/RenderIFrame.h b/WebCore/rendering/RenderIFrame.h > index 60e6fdc..0fe22f5 100644 > --- a/WebCore/rendering/RenderIFrame.h > +++ b/WebCore/rendering/RenderIFrame.h > @@ -34,18 +34,43 @@ class RenderIFrame : public RenderFrameBase { > public: > RenderIFrame(Element*); > > +#if USE(ACCELERATED_COMPOSITING) > + bool requiresAcceleratedCompositing() const; > +#endif > + > private: > virtual void calcHeight(); > virtual void calcWidth(); > > virtual void layout(); > > +#if USE(ACCELERATED_COMPOSITING) > + virtual bool requiresLayer() const; > +#endif > + virtual bool isRenderIFrame() const { return true; } > + > virtual const char* renderName() const { return "RenderPartObject"; } // Lying for now to avoid breaking tests > > bool flattenFrame(); > > }; > > +inline RenderIFrame* toRenderIFrame(RenderObject* object) > +{ > + ASSERT(!object || !strcmp(object->renderName(), "RenderPartObject")); // FIXME: fix the string when we fix renderName(). > + return static_cast<RenderIFrame*>(object); > +} I don't get why you have to do a string compare here. Why can't you use isRenderIFrame()? I understand that's just in the ASSERT, but why isn't the isRenderIFrame() test sufficient?
Comment on attachment 53867 [details] Patch > + (WebCore::RenderLayerCompositor::enableCompositingMode): Call out to the RenderView when > + the compositing mode changes, so that the parent document can updates its compositing status. Typo: “updates” Like Chris said, the assertions in the to* methods should just use is*, not awkward string compare.
Comment on attachment 53867 [details] Patch http://trac.webkit.org/changeset/57919 Keeping bug open for more work.
This has been failing on SL since it landed: http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r57919%20(8634)/results.html Looks like tabs vs. spaces, possibly?
I'm not sure why SheriffBot never complained here.
Ah, I should have fixed this result via bug 37885.
I think http://trac.webkit.org/changeset/57913 is actually causing the failure, but the question is why did it not show up until 57919?
Fixed that test result in http://trac.webkit.org/changeset/57953 .
I think this may also have caused a test to start failing in debug: http://build.webkit.org/results/SnowLeopard%20Intel%20Leaks/r57921%20(6175)/transitions/transition-end-event-destroy-iframe-stderr.txt ASSERTION FAILED: !needsLayout() (/Volumes/Data/WebKit-BuildSlave/snowleopard-intel-leaks/build/WebCore/page/FrameView.cpp:1801 virtual void WebCore::FrameView::paintContents(WebCore::GraphicsContext*, const WebCore::IntRect&)) At least it's the only painting related change in the range (r57916-r57921)
The !needsLayout() assertion should have been fixed by this commit (rolling out an earlier change of mine): http://trac.webkit.org/changeset/57895 Maybe the bots are just very behind?
http://build.webkit.org/results/SnowLeopard%20Intel%20Leaks/r57952%20(6190)/results.html Shows the crash and that's a build of r57952.
Thank you for fixing the test results, btw.
Please file a new bug on the transitions/transition-end-event-destroy-iframe.html assertion.
(In reply to comment #14) > Please file a new bug on the > transitions/transition-end-event-destroy-iframe.html assertion. I'm confused. Do we believe this change is the cause? If so, why would we leave the bot failing the ASSERT?
Comment on attachment 53867 [details] Patch Cleared Dan Bernstein's review+ from obsolete attachment 53867 [details] so that this bug does not appear in http://webkit.org/pending-commit.
This caused regressions. See bug 38072.
This is done now.