Bug 91451 – REGRESSION: RenderInline::absoluteQuads produces incorrect results for fixed position.

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Kiran Muppala]

RenderInline::absoluteQuads is producing incorrect results if having a fixed position.  This leads to assertion failures in RenderGeometryMap::absoluteRect when opening webpages with such content.

On investigation, found that the RenderGeometryStep copy constructor was not coping the offset and hence the result was incorrect.  However, while looking at RenderGeometryStep's copy constructor I also observed that in general it is not safe to copy a RenderGeometryMap, since it's individual step's have a owned pointer to their respective transforms.  So, the broader issue here is that RenderInline::absoluteQuads creates copies of RenderGeometryMap indirectly by passing copies of AbsoluteQuadsGeneratorContext.  To avoid issues with copying a GeometryMap it is better to modify RenderInline::absoluteQuads to pass AbsoluteQuadsGeneratorContext by reference instead.

[Kiran Muppala]

<rdar://problem/11839315>

[Kiran Muppala]

Created attachment 152672 [details]
Patch

[WebKit Review Bot]

Comment on attachment 152672 [details]
Patch

Clearing flags on attachment: 152672

Committed r122802: <http://trac.webkit.org/changeset/122802>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Daniel Bates]

(In reply to comment #3)
> (From update of attachment 152672 [details])
> Clearing flags on attachment: 152672
> 
> Committed r122802: <http://trac.webkit.org/changeset/122802>

For some reason this commit caused the Chromium Mac build to fail because the compiler detected that the private instance variable AbsoluteQuadsGeneratorContext::m_wasFixed was unused (see remark (*)):

/Volumes/data/b/WebKit-BuildSlave/chromium-mac-release/build/Source/WebCore/WebCore.gyp/../rendering/RenderInline.cpp:665:11: error: private field 'm_wasFixed' is not used [-Werror,-Wunused-private-field]
    bool* m_wasFixed;

<http://build.webkit.org/builders/Chromium%20Mac%20Release/builds/40008/steps/compile-webkit/logs/stdio>

Removed unused private instance variable AbsoluteQuadsGeneratorContext::m_wasFixed and committed this in <http://trac.webkit.org/changeset/122812>.

(*) AbsoluteQuadsGeneratorContext::m_wasFixed has remained unused since it was added in <http://trac.webkit.org/changeset/116718> (bug #85725).

[Kiran Muppala]

(In reply to comment #5)
> (In reply to comment #3)
> > (From update of attachment 152672 [details] [details])
> > Clearing flags on attachment: 152672
> > 
> > Committed r122802: <http://trac.webkit.org/changeset/122802>
> 
> For some reason this commit caused the Chromium Mac build to fail because the compiler detected that the private instance variable AbsoluteQuadsGeneratorContext::m_wasFixed was unused (see remark (*)):
> 
> /Volumes/data/b/WebKit-BuildSlave/chromium-mac-release/build/Source/WebCore/WebCore.gyp/../rendering/RenderInline.cpp:665:11: error: private field 'm_wasFixed' is not used [-Werror,-Wunused-private-field]
>     bool* m_wasFixed;
> 
> <http://build.webkit.org/builders/Chromium%20Mac%20Release/builds/40008/steps/compile-webkit/logs/stdio>
> 
> Removed unused private instance variable AbsoluteQuadsGeneratorContext::m_wasFixed and committed this in <http://trac.webkit.org/changeset/122812>.
> 
> (*) AbsoluteQuadsGeneratorContext::m_wasFixed has remained unused since it was added in <http://trac.webkit.org/changeset/116718> (bug #85725).

I am not sure why commit r122802, started to trigger the failure, may be because prior to it the sole instantiation of AbsoluteQuadsGeneratorContext was a temporary and that possibly allowed the compiler to be lax with the checks.    Thanks for the fix.  The code is cleaner without the unused variable for sure.