WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
37878
Hook compositing layers together across iframes
https://bugs.webkit.org/show_bug.cgi?id=37878
Summary
Hook compositing layers together across iframes
Simon Fraser (smfr)
Reported
2010-04-20 11:50:32 PDT
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.
Attachments
Patch
(12.86 KB, patch)
2010-04-20 12:43 PDT
,
Simon Fraser (smfr)
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1
2010-04-20 12:43:23 PDT
Created
attachment 53867
[details]
Patch
Chris Marrin
Comment 2
2010-04-20 12:49:35 PDT
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?
mitz
Comment 3
2010-04-20 14:19:16 PDT
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.
Simon Fraser (smfr)
Comment 4
2010-04-20 14:24:46 PDT
Comment on
attachment 53867
[details]
Patch
http://trac.webkit.org/changeset/57919
Keeping bug open for more work.
Eric Seidel (no email)
Comment 5
2010-04-20 22:38:05 PDT
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?
Eric Seidel (no email)
Comment 6
2010-04-20 22:38:28 PDT
I'm not sure why SheriffBot never complained here.
Simon Fraser (smfr)
Comment 7
2010-04-20 22:42:31 PDT
Ah, I should have fixed this result via
bug 37885
.
Eric Seidel (no email)
Comment 8
2010-04-20 22:44:52 PDT
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?
Simon Fraser (smfr)
Comment 9
2010-04-20 22:49:14 PDT
Fixed that test result in
http://trac.webkit.org/changeset/57953
.
Eric Seidel (no email)
Comment 10
2010-04-20 22:55:02 PDT
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
)
Simon Fraser (smfr)
Comment 11
2010-04-20 23:05:24 PDT
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?
Eric Seidel (no email)
Comment 12
2010-04-20 23:06:33 PDT
http://build.webkit.org/results/SnowLeopard%20Intel%20Leaks/r57952%20(6190)/results.html
Shows the crash and that's a build of
r57952
.
Eric Seidel (no email)
Comment 13
2010-04-20 23:06:56 PDT
Thank you for fixing the test results, btw.
Simon Fraser (smfr)
Comment 14
2010-04-20 23:13:29 PDT
Please file a new bug on the transitions/transition-end-event-destroy-iframe.html assertion.
Eric Seidel (no email)
Comment 15
2010-04-20 23:17:55 PDT
(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?
Eric Seidel (no email)
Comment 16
2010-04-21 18:13:16 PDT
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
.
Simon Fraser (smfr)
Comment 17
2010-04-23 18:01:41 PDT
This caused regressions. See
bug 38072
.
Simon Fraser (smfr)
Comment 18
2010-07-08 22:39:09 PDT
This is done now.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug