Clear the groupedMapping parameter in layers when removed from the group

Clear the groupedMapping parameter in layers when removed from the group.

Also clean up the code somewhat: avoid magic numbers, return as soon as possible, clarify parameter meaning.

R=vollick
BUG=504688
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=198327

[Stephen Chennney, 2015-06-30 19:40:29]

The CQ bit was checked by schenney@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-06-30 19:41:01]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1209033009/1

[Stephen Chennney, 2015-06-30 19:43:48]

The CQ bit was unchecked by schenney@chromium.org

[Stephen Chennney, 2015-06-30 19:44:03]

Need a patch without errors.

https://codereview.chromium.org/1209033009/diff/1/Source/core/layout/composit...
File Source/core/layout/compositing/CompositedDeprecatedPaintLayerMapping.cpp
(right):

https://codereview.chromium.org/1209033009/diff/1/Source/core/layout/composit...
Source/core/layout/compositing/CompositedDeprecatedPaintLayerMapping.cpp:2288:
formerLayer->setGroupedMapping(nullptr);
This unfortunately has the side effect of doing some invalidation:

void
DeprecatedPaintLayer::setGroupedMapping(CompositedDeprecatedPaintLayerMapping*
groupedMapping, bool layerBeingDestroyed)
{
    if (groupedMapping == m_groupedMapping)
        return;

    if (!layerBeingDestroyed && m_groupedMapping) {
       
m_groupedMapping->setNeedsGraphicsLayerUpdate(GraphicsLayerUpdateSubtree);
        m_groupedMapping->removeLayerFromSquashingGraphicsLayer(this);
    }
    m_groupedMapping = groupedMapping;
    if (!layerBeingDestroyed && m_groupedMapping)
       
m_groupedMapping->setNeedsGraphicsLayerUpdate(GraphicsLayerUpdateSubtree);
}

I'm am not at all sure we should be doing it, rather than having a shortcut that
just sets the m_groupedMapping to null with no side effect.

https://codereview.chromium.org/1209033009/diff/1/Source/core/layout/composit...
Source/core/layout/compositing/CompositedDeprecatedPaintLayerMapping.cpp:2311:
// squashingLayer contents.
I'm having trouble interpreting this comment. Is it intending to say "Don't call
remove because ti invalidates the contents"? Or is it some cryptic comment about
the m_squashedLayers.remove call?

https://codereview.chromium.org/1209033009/diff/1/Source/core/layout/composit...
Source/core/layout/compositing/CompositedDeprecatedPaintLayerMapping.cpp:2315:
m_squashedLayers[i].paintLayer->setGroupedMapping(nullptr);
See comment about, might be a real bad idea.

[Stephen Chennney, 2015-06-30 19:46:05]

The CQ bit was checked by schenney@chromium.org to run a CQ dry run

[Stephen Chennney, 2015-06-30 19:46:28]

See comments on first patch set.

[commit-bot: I haz the power, 2015-06-30 19:46:56]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1209033009/20001

[Stephen Chennney, 2015-06-30 21:20:47]

On 2015/06/30 19:46:56, commit-bot: I haz the power wrote:
> Dry run: CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patch-status/1209033009/20001

Clearly broke something. Will look into why.

[commit-bot: I haz the power, 2015-06-30 21:20:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-30 21:20:59]

Dry run: Exceeded global retry quota

[Stephen Chennney, 2015-07-02 17:01:11]

The CQ bit was checked by schenney@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-07-02 17:01:19]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1209033009/40001

[Stephen Chennney, 2015-07-02 17:02:59]

I think this is ready to go. There's some code cleanup included and there is one
text repaint rect change due to, I think, something that previously was left in
a grouped mapping (a sign of the original bug) but is now not squashed.

The crash test cases are a mess. I can clean one up but first I'd like to get
the OK on the code changes.

[Stephen Chennney, 2015-07-02 17:03:23]

schenney@chromium.org changed reviewers:
+ ajuma@chromium.org

[Ian Vollick, 2015-07-02 17:29:00]

lgtm.

It would be nice to include the updated expectation for one platform in this CL
and NeedsRebaseline the others.

https://codereview.chromium.org/1209033009/diff/40001/Source/core/paint/Depre...
File Source/core/paint/DeprecatedPaintLayer.h (right):

https://codereview.chromium.org/1209033009/diff/40001/Source/core/paint/Depre...
Source/core/paint/DeprecatedPaintLayer.h:319: void
setGroupedMapping(CompositedDeprecatedPaintLayerMapping* groupedMapping, bool
issueInvalidations);
Could you please use an enum rather than a boolean? I was confused at the
callsites and meaningful 2nd parameter would have really helped.

[commit-bot: I haz the power, 2015-07-02 18:11:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-07-02 18:11:04]

Dry run: This issue passed the CQ dry run.

[Stephen Chennney, 2015-07-06 17:36:38]

The CQ bit was checked by schenney@chromium.org

[Stephen Chennney, 2015-07-06 17:36:39]

The patchset sent to the CQ was uploaded after l-g-t-m from vollick@chromium.org
Link to the patchset: https://codereview.chromium.org/1209033009/#ps60001
(title: "Switch to enum")

[commit-bot: I haz the power, 2015-07-06 17:37:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1209033009/60001

[commit-bot: I haz the power, 2015-07-06 18:57:05]

Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=198327