Fix assertion fail in RenderGeometryMap::absoluteRect when frame scale != 1.0

[Tien-Ren Chen]

Fix assertion fail in RenderGeometryMap::absoluteRect when frame scale != 1.0

[Tien-Ren Chen]

Created attachment 171758 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 171758 [details]
Patch

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

> Source/WebCore/rendering/RenderGeometryMap.cpp:125
> +        // If this box has a transform, it acts as a fixed position container
> +        // for fixed descendants, which prevents the propagation of 'fixed'
> +        // unless the layer itself is also fixed position.
> +        if (currentStep.m_hasTransform && !currentStep.m_isFixedPosition)
> +            inFixed = false;
> +        else if (currentStep.m_isFixedPosition)
> +            inFixed = true;

I'm not sure that moving this is correct. I think we may just want to special case the RenderView (step 0) to not disable the fixed handling if it has a transform.

Do we have test coverage for fixed inside transformed?

> LayoutTests/ChangeLog:4
> +        https://bugs.webkit.org/show_bug.cgi?id=100912

fail -> failure

> LayoutTests/ChangeLog:12
> +        tranformation case.

tranformation -> transformation

[Tien-Ren Chen]

Created attachment 171783 [details]
Patch

[Levi Weintraub]

Comment on attachment 171783 [details]
Patch

Great catch! I've hit that assert many times.

[WebKit Review Bot]

Comment on attachment 171783 [details]
Patch

Rejecting attachment 171783 [details] from commit-queue.

New failing tests:
compositing/geometry/fixed-position-composited-page-scale-scroll.html
platform/chromium/virtual/softwarecompositing/geometry/fixed-position-composited-page-scale-scroll.html
Full output: http://queues.webkit.org/results/14696313

[WebKit Review Bot]

Comment on attachment 171783 [details]
Patch

Attachment 171783 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14661592

New failing tests:
compositing/geometry/fixed-position-composited-page-scale-scroll.html
platform/chromium/virtual/softwarecompositing/geometry/fixed-position-composited-page-scale-scroll.html

[Tien-Ren Chen]

Created attachment 171918 [details]
Patch

[Tien-Ren Chen]

Change to ref-test

[Simon Fraser (smfr)]

Comment on attachment 171918 [details]
Patch

I don't see the point of having a ref test for an assertion failure. Ref tests are slower to run.

[Tien-Ren Chen]

Created attachment 171920 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 171920 [details]
Patch

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

> LayoutTests/compositing/geometry/fixed-position-composited-page-scale-scroll-expected.txt:2
> +TEST
> +TEST

It's useful for the test output to say what the test is testing. So something like "This test should not hit an assertion in RenderGeometryMap in debug builds".

[Tien-Ren Chen]

Created attachment 171929 [details]
Patch

[WebKit Review Bot]

Comment on attachment 171929 [details]
Patch

Clearing flags on attachment: 171929

Committed r133248: <http://trac.webkit.org/changeset/133248>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.