Bug 52655 - Hidden composited iframes cause infinite loop
Summary: Hidden composited iframes cause infinite loop
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All OS X 10.5
: P2 Normal
Assignee: Simon Fraser (smfr)
URL: http://www.eppendorfna.com/int/index....
Keywords: InRadar
: 68605 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-01-18 12:57 PST by Patrick
Modified: 2011-10-21 11:06 PDT (History)
12 users (show)

See Also:


Attachments
Test case (10.00 KB, application/x-tar)
2011-01-18 12:59 PST, Patrick
no flags Details
Patch (4.36 KB, patch)
2011-02-25 19:28 PST, Yael
no flags Details | Formatted Diff | Diff
Patch (9.26 KB, patch)
2011-10-20 17:21 PDT, Simon Fraser (smfr)
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick 2011-01-18 12:57:15 PST
This happens on platforms that do not support independently composited iframes.

What I think is happening is that a style change causes a layer in the hidden iframe to composite which bubbles up to the owner element.  On the Mac platform pre-WebKit2, allowsIndependentlyCompositedIFrames returns true which stops propagation to the parent's compsitor.

On a platform like Android (or if you return false from allowsIndependentlyCompositedIFrames), the parent compositor is turned on which schedules a style change.  The parent compositor traverses all the layers and turns off compositing because the iframe layer is not part of any layer list.

I have attached a basic test that I used to reproduce the infinite loop on Safari.  I had to edit RenderLayerCompositor.cpp to not allow independently composited iframes.
Comment 1 Patrick 2011-01-18 12:59:25 PST
Created attachment 79314 [details]
Test case
Comment 2 Yael 2011-02-25 19:28:35 PST
Created attachment 83918 [details]
Patch

Add a check if an iframe is visible when deciding if it requires a compositing layer.
Without the check we would create a compositing layer, but no backing and that caused the looping.
Comment 3 Simon Fraser (smfr) 2011-02-25 19:48:44 PST
Comment on attachment 83918 [details]
Patch

This seems like a specific case of the more general issue that visibility:hidden and compositing don't play nicely together. Bug 29984 covers that.

This patch is not correct because it needs to consider ancestor elements of the iframe that are composited and have visibility:hidden.
Comment 4 Yael 2011-02-26 07:47:08 PST
(In reply to comment #3)
> (From update of attachment 83918 [details])
> This seems like a specific case of the more general issue that visibility:hidden and compositing don't play nicely together. Bug 29984 covers that.
> 
> This patch is not correct because it needs to consider ancestor elements of the iframe that are composited and have visibility:hidden.

Thank you for your review.
I should work on adding a check for the iframe's ancestors' visibility.

I tested bugs 29984 and 26770, and they seem to affect only the mac port for Safari. The Chrome port on Linux and on mac handles the test cases properly. Do you have an idea why is that?
thanks,
Comment 5 Yael 2011-03-09 14:59:47 PST
(In reply to comment #4)
> I tested bugs 29984 and 26770, and they seem to affect only the mac port for Safari. The Chrome port on Linux and on mac handles the test cases properly. Do you have an idea why is that?
I was actually wrong. If I leave only one of the divs in the test from https://bug-29984-attachments.webkit.org/attachment.cgi?id=40479, I see the same infinite loop in QtTestBrower. And there are no iframes in that test.
Comment 6 Antonio Gomes 2011-09-22 07:06:32 PDT
*** Bug 68605 has been marked as a duplicate of this bug. ***
Comment 7 Simon Fraser (smfr) 2011-09-22 11:27:59 PDT
<rdar://problem/9485360>
Comment 8 Simon Fraser (smfr) 2011-09-22 11:31:48 PDT
There's a general issue with visibility and compositing that I hope to work on soon. iframe-specific patches are only bandaids at this point.
Comment 9 Fady Samuel 2011-10-13 11:48:03 PDT
(In reply to comment #8)
> There's a general issue with visibility and compositing that I hope to work on soon. iframe-specific patches are only bandaids at this point.

I ran into this problem locally. I was wondering if you had filed a more general bug report against this issue? Have you begun work on this? Is this something I can help looking into? Thanks.
Comment 10 Simon Fraser (smfr) 2011-10-14 15:23:18 PDT
Looking at this now.
Comment 11 Simon Fraser (smfr) 2011-10-14 17:08:44 PDT
Here's the smoking gun. Inside of recalcStyle(), notifyIFramesOfCompositingChange() is calling scheduleSetNeedsStyleRecalc(), so we never get out of updateStyleForAllDocuments.


#0  WebCore::ContainerNode::scheduleSetNeedsStyleRecalc (this=0x1001d4f00, changeType=WebCore::SyntheticStyleChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/ContainerNode.cpp:759
#1  0x00000001038ceeb0 in WebCore::RenderLayerCompositor::notifyIFramesOfCompositingChange (this=0x10017f850) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderLayerCompositor.cpp:1900
#2  0x00000001038ce58c in WebCore::RenderLayerCompositor::enableCompositingMode (this=0x10017f850, enable=true) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderLayerCompositor.cpp:125
#3  0x00000001038d14a4 in WebCore::RenderLayerCompositor::updateBacking (this=0x10017f850, layer=0x1001dcb28, shouldRepaint=WebCore::RenderLayerCompositor::CompositingChangeRepaintNow) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderLayerCompositor.cpp:340
#4  0x00000001038d1b93 in WebCore::RenderLayerCompositor::updateLayerCompositingState (this=0x10017f850, layer=0x1001dcb28, shouldRepaint=WebCore::RenderLayerCompositor::CompositingChangeRepaintNow) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderLayerCompositor.cpp:410
#5  0x00000001038bc222 in WebCore::RenderLayer::styleChanged (this=0x1001dcb28, diff=WebCore::StyleDifferenceEqual, oldStyle=0x123481310) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderLayer.cpp:4204
#6  0x000000010384c48b in WebCore::RenderBoxModelObject::styleDidChange (this=0x1001dc818, diff=WebCore::StyleDifferenceEqual, oldStyle=0x123481310) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderBoxModelObject.cpp:373
#7  0x0000000103834bc0 in WebCore::RenderBox::styleDidChange (this=0x1001dc818, diff=WebCore::StyleDifferenceEqual, oldStyle=0x123481310) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderBox.cpp:345
#8  0x00000001039121fd in WebCore::RenderReplaced::styleDidChange (this=0x1001dc818, diff=WebCore::StyleDifferenceEqual, oldStyle=0x123481310) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderReplaced.cpp:71
#9  0x00000001039f269d in WebCore::RenderWidget::styleDidChange (this=0x1001dc818, diff=WebCore::StyleDifferenceEqual, oldStyle=0x123481310) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderWidget.cpp:215
#10 0x0000000103902023 in WebCore::RenderObject::setStyle (this=0x1001dc818, style=@0x7fff5fbfd160) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderObject.cpp:1727
#11 0x000000010390177b in WebCore::RenderObject::setAnimatableStyle (this=0x1001dc818, style=@0x7fff5fbfd1a0) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderObject.cpp:1640
#12 0x000000010370a9e3 in WebCore::Node::setRenderStyle (this=0x1001d4f00, s=@0x7fff5fbfd2d0) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Node.cpp:1493
#13 0x0000000102d77b49 in WebCore::Element::recalcStyle (this=0x1001d4f00, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Element.cpp:1124
#14 0x0000000102d77e3f in WebCore::Element::recalcStyle (this=0x1001d46f0, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Element.cpp:1158
#15 0x0000000102d77e3f in WebCore::Element::recalcStyle (this=0x1001d2f90, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Element.cpp:1158
#16 0x0000000102d77e3f in WebCore::Element::recalcStyle (this=0x1001d27b0, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Element.cpp:1158
#17 0x0000000102d77e3f in WebCore::Element::recalcStyle (this=0x110e4aa40, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Element.cpp:1158
#18 0x0000000102d77e3f in WebCore::Element::recalcStyle (this=0x100182c10, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Element.cpp:1158
#19 0x0000000102ba7161 in WebCore::Document::recalcStyle (this=0x10082ce00, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Document.cpp:1562
#20 0x0000000102ba81d9 in WebCore::Document::updateStyleIfNeeded (this=0x10082ce00) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Document.cpp:1616
#21 0x0000000102ba82e2 in WebCore::Document::updateStyleForAllDocuments () at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Document.cpp:1633
#22 0x0000000103a40a6e in WebCore::ScheduledAction::execute (this=0x1006b3ec0, document=0x10082ce00) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/bindings/js/ScheduledAction.cpp:131
#23 0x0000000103a40894 in WebCore::ScheduledAction::execute (this=0x1006b3ec0, context=0x10082cff8) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/bindings/js/ScheduledAction.cpp:80
#24 0x0000000102d10636 in WebCore::DOMTimer::fired (this=0x1006b3d80) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/page/DOMTimer.cpp:148
#25 0x0000000103c95bd7 in WebCore::ThreadTimers::sharedTimerFiredInternal (this=0x107a17910) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/platform/ThreadTimers.cpp:115
#26 0x0000000103c959a9 in WebCore::ThreadTimers::sharedTimerFired () at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/platform/ThreadTimers.cpp:93
Comment 12 Fady Samuel 2011-10-17 08:00:02 PDT
What's the right solution for this? A flag that indicates we're in the process of recalculating style so that scheduleSetNeedsStyleRecalc() is not called?

(In reply to comment #11)
> Here's the smoking gun. Inside of recalcStyle(), notifyIFramesOfCompositingChange() is calling scheduleSetNeedsStyleRecalc(), so we never get out of updateStyleForAllDocuments.
> 
> 
> #0  WebCore::ContainerNode::scheduleSetNeedsStyleRecalc (this=0x1001d4f00, changeType=WebCore::SyntheticStyleChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/ContainerNode.cpp:759
> #1  0x00000001038ceeb0 in WebCore::RenderLayerCompositor::notifyIFramesOfCompositingChange (this=0x10017f850) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderLayerCompositor.cpp:1900
> #2  0x00000001038ce58c in WebCore::RenderLayerCompositor::enableCompositingMode (this=0x10017f850, enable=true) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderLayerCompositor.cpp:125
> #3  0x00000001038d14a4 in WebCore::RenderLayerCompositor::updateBacking (this=0x10017f850, layer=0x1001dcb28, shouldRepaint=WebCore::RenderLayerCompositor::CompositingChangeRepaintNow) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderLayerCompositor.cpp:340
> #4  0x00000001038d1b93 in WebCore::RenderLayerCompositor::updateLayerCompositingState (this=0x10017f850, layer=0x1001dcb28, shouldRepaint=WebCore::RenderLayerCompositor::CompositingChangeRepaintNow) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderLayerCompositor.cpp:410
> #5  0x00000001038bc222 in WebCore::RenderLayer::styleChanged (this=0x1001dcb28, diff=WebCore::StyleDifferenceEqual, oldStyle=0x123481310) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderLayer.cpp:4204
> #6  0x000000010384c48b in WebCore::RenderBoxModelObject::styleDidChange (this=0x1001dc818, diff=WebCore::StyleDifferenceEqual, oldStyle=0x123481310) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderBoxModelObject.cpp:373
> #7  0x0000000103834bc0 in WebCore::RenderBox::styleDidChange (this=0x1001dc818, diff=WebCore::StyleDifferenceEqual, oldStyle=0x123481310) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderBox.cpp:345
> #8  0x00000001039121fd in WebCore::RenderReplaced::styleDidChange (this=0x1001dc818, diff=WebCore::StyleDifferenceEqual, oldStyle=0x123481310) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderReplaced.cpp:71
> #9  0x00000001039f269d in WebCore::RenderWidget::styleDidChange (this=0x1001dc818, diff=WebCore::StyleDifferenceEqual, oldStyle=0x123481310) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderWidget.cpp:215
> #10 0x0000000103902023 in WebCore::RenderObject::setStyle (this=0x1001dc818, style=@0x7fff5fbfd160) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderObject.cpp:1727
> #11 0x000000010390177b in WebCore::RenderObject::setAnimatableStyle (this=0x1001dc818, style=@0x7fff5fbfd1a0) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/rendering/RenderObject.cpp:1640
> #12 0x000000010370a9e3 in WebCore::Node::setRenderStyle (this=0x1001d4f00, s=@0x7fff5fbfd2d0) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Node.cpp:1493
> #13 0x0000000102d77b49 in WebCore::Element::recalcStyle (this=0x1001d4f00, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Element.cpp:1124
> #14 0x0000000102d77e3f in WebCore::Element::recalcStyle (this=0x1001d46f0, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Element.cpp:1158
> #15 0x0000000102d77e3f in WebCore::Element::recalcStyle (this=0x1001d2f90, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Element.cpp:1158
> #16 0x0000000102d77e3f in WebCore::Element::recalcStyle (this=0x1001d27b0, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Element.cpp:1158
> #17 0x0000000102d77e3f in WebCore::Element::recalcStyle (this=0x110e4aa40, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Element.cpp:1158
> #18 0x0000000102d77e3f in WebCore::Element::recalcStyle (this=0x100182c10, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Element.cpp:1158
> #19 0x0000000102ba7161 in WebCore::Document::recalcStyle (this=0x10082ce00, change=WebCore::Node::NoChange) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Document.cpp:1562
> #20 0x0000000102ba81d9 in WebCore::Document::updateStyleIfNeeded (this=0x10082ce00) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Document.cpp:1616
> #21 0x0000000102ba82e2 in WebCore::Document::updateStyleForAllDocuments () at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/dom/Document.cpp:1633
> #22 0x0000000103a40a6e in WebCore::ScheduledAction::execute (this=0x1006b3ec0, document=0x10082ce00) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/bindings/js/ScheduledAction.cpp:131
> #23 0x0000000103a40894 in WebCore::ScheduledAction::execute (this=0x1006b3ec0, context=0x10082cff8) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/bindings/js/ScheduledAction.cpp:80
> #24 0x0000000102d10636 in WebCore::DOMTimer::fired (this=0x1006b3d80) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/page/DOMTimer.cpp:148
> #25 0x0000000103c95bd7 in WebCore::ThreadTimers::sharedTimerFiredInternal (this=0x107a17910) at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/platform/ThreadTimers.cpp:115
> #26 0x0000000103c959a9 in WebCore::ThreadTimers::sharedTimerFired () at /Volumes/SSData/Development/OSX/webkit/OpenSource/Source/WebCore/platform/ThreadTimers.cpp:93
Comment 13 Simon Fraser (smfr) 2011-10-17 08:47:27 PDT
I don't know yet. Still working on it.
Comment 14 Simon Fraser (smfr) 2011-10-18 12:12:19 PDT
The real issue here is that we enter compositing via RenderLayerCompositor::updateLayerCompositingState(), but then drop out of compositing again at the end of RenderLayerCompositor::computeCompositingRequirements(), because we didn't hit the composited layer because of visibility:hidden.

The larger question is whether we should be in compositing mode if some hidden layer wants to be composited.
Comment 15 Darin Adler 2011-10-18 13:36:21 PDT
(In reply to comment #14)
> The larger question is whether we should be in compositing mode if some hidden layer wants to be composited.

Seems like a simple tradeoff:

1) Making the layer visible will be smoother if we keep the page in compositing mode.

2) If the layer stays hidden, then performance and memory use will be better if we keep the page out of compositing mode.

Is there anything else I’m missing?
Comment 16 Simon Fraser (smfr) 2011-10-20 17:21:09 PDT
Created attachment 111878 [details]
Patch
Comment 17 James Robinson 2011-10-20 17:28:18 PDT
Comment on attachment 111878 [details]
Patch

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

> Source/WebCore/rendering/RenderLayerCompositor.h:155
> +    void layerBecameComposited(const RenderLayer*) { ++m_compositedLayerCount; }
> +    void layerBecameNonComposited(const RenderLayer*)

why are you passing a layer here if you just keep a count? Is the idea to expand this to keeping a Set<> sometimes?
Comment 18 Simon Fraser (smfr) 2011-10-20 17:31:48 PDT
(In reply to comment #17)
> (From update of attachment 111878 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=111878&action=review
> 
> > Source/WebCore/rendering/RenderLayerCompositor.h:155
> > +    void layerBecameComposited(const RenderLayer*) { ++m_compositedLayerCount; }
> > +    void layerBecameNonComposited(const RenderLayer*)
> 
> why are you passing a layer here if you just keep a count? Is the idea to expand this to keeping a Set<> sometimes?

No, I just thought it made sense for a delegate-type method. It might be useful in future, but I had no specific plans for it.
Comment 19 mitz 2011-10-20 18:10:12 PDT
The function name should start with "has" rather than "have" for consistency with the previously-declared function and with the subject of the phrase (a RenderLayerCompositor).
Comment 20 Darin Adler 2011-10-20 18:14:14 PDT
Comment on attachment 111878 [details]
Patch

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

> Source/WebCore/rendering/RenderLayerCompositor.cpp:251
> +    int nonRootCompositedLayerCount = m_compositedLayerCount;
> +    if (rootLayer->isComposited())
> +        --nonRootCompositedLayerCount;
> +
> +    return nonRootCompositedLayerCount;

I would write this as:

    // We want to return true if only the root layer is composited.
    return m_compositedLayerCount == rootLayer->isComposited();

Maybe that’s not as clear to you, but it seems pretty clear to me.

>>> Source/WebCore/rendering/RenderLayerCompositor.h:155
>>> +    void layerBecameNonComposited(const RenderLayer*)
>> 
>> why are you passing a layer here if you just keep a count? Is the idea to expand this to keeping a Set<> sometimes?
> 
> No, I just thought it made sense for a delegate-type method. It might be useful in future, but I had no specific plans for it.

In debug builds you should keep a HashSet.
Comment 21 Darin Adler 2011-10-20 18:15:35 PDT
Comment on attachment 111878 [details]
Patch

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

> Source/WebCore/rendering/RenderLayerCompositor.cpp:322
> +            if (childList.isEmpty() && !haveAnyCompositedLayers(updateRoot))

I think a comment should say something about why haveAnyCompositedLayers needs to be called.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:788
>      // If we're back at the root, and no other layers need to be composited, and the root layer itself doesn't need
>      // to be composited, then we can drop out of compositing mode altogether.
> -    if (layer->isRootLayer() && !childState.m_subtreeIsCompositing && !requiresCompositingLayer(layer) && !m_forceCompositingMode) {
> +    if (layer->isRootLayer() && !childState.m_subtreeIsCompositing && !requiresCompositingLayer(layer) && !m_forceCompositingMode && !haveAnyCompositedLayers(layer)) {

I think a comment should say something about why haveAnyCompositedLayers needs to be called.

> Source/WebCore/rendering/RenderLayerCompositor.h:259
> +    bool haveAnyCompositedLayers(const RenderLayer* rootLayer) const;

It’s not clear to me at least that the root layer is not considered when asking if there are any composited layers.
Comment 22 Simon Fraser (smfr) 2011-10-20 20:01:32 PDT
http://trac.webkit.org/changeset/98060
Comment 23 Jeff Miller 2011-10-21 10:21:51 PDT
This broke the Windows build, Visual Studio doesn't like this:


bool RenderLayerCompositor::hasAnyAdditionalCompositedLayers(const RenderLayer* rootLayer) const
{
    return m_compositedLayerCount > rootLayer->isComposited();
}


5>c:\cygwin\home\jeffm\opensource\source\webcore\rendering\RenderLayerCompositor.cpp(247) : error C2220: warning treated as error - no 'object' file generated
5>c:\cygwin\home\jeffm\opensource\source\webcore\rendering\RenderLayerCompositor.cpp(247) : warning C4804: '>' : unsafe use of type 'bool' in operation
Comment 24 Jeff Miller 2011-10-21 10:28:51 PDT
(In reply to comment #23)
> This broke the Windows build, Visual Studio doesn't like this:
> 
> 
> bool RenderLayerCompositor::hasAnyAdditionalCompositedLayers(const RenderLayer* rootLayer) const
> {
>     return m_compositedLayerCount > rootLayer->isComposited();
> }
> 
> 
> 5>c:\cygwin\home\jeffm\opensource\source\webcore\rendering\RenderLayerCompositor.cpp(247) : error C2220: warning treated as error - no 'object' file generated
> 5>c:\cygwin\home\jeffm\opensource\source\webcore\rendering\RenderLayerCompositor.cpp(247) : warning C4804: '>' : unsafe use of type 'bool' in operation

VS seems OK with this construct:

    return m_compositedLayerCount > (rootLayer->isComposited() ? 1 : 0);
Comment 25 Jeff Miller 2011-10-21 11:06:42 PDT
(In reply to comment #24)
> (In reply to comment #23)
> > This broke the Windows build, Visual Studio doesn't like this:
> > 
> > 
> > bool RenderLayerCompositor::hasAnyAdditionalCompositedLayers(const RenderLayer* rootLayer) const
> > {
> >     return m_compositedLayerCount > rootLayer->isComposited();
> > }
> > 
> > 
> > 5>c:\cygwin\home\jeffm\opensource\source\webcore\rendering\RenderLayerCompositor.cpp(247) : error C2220: warning treated as error - no 'object' file generated
> > 5>c:\cygwin\home\jeffm\opensource\source\webcore\rendering\RenderLayerCompositor.cpp(247) : warning C4804: '>' : unsafe use of type 'bool' in operation
> 
> VS seems OK with this construct:
> 
>     return m_compositedLayerCount > (rootLayer->isComposited() ? 1 : 0);

Simon fixed this in <http://trac.webkit.org/changeset/98112>