Don't check for layout in a canvas if it it's already needed

Don't check for layout in a canvas if it it's already needed

In this clusterfuzz test case a float is deleted but its entry in the floating
objects list of a sibling renderer is accessed before layout has had time to
remove reference to it. The read attempt pre-empts layout because the change in
zoom factor prompts the canvas renderer to recompute its width/height to check
if layout is required. If layout is already required this isn't necessary and,
what's more, if layout is already required it may be because renderer(s) in its
floating object list have been deleted and aren't safe to access while computing
offset as part of the width calculations.

So return early when the check for layout is unnecessary and may even crash.

BUG=445285
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=187935

[rhogan, 2015-01-01 14:04:23]

robhogan@gmail.com changed reviewers:
+ inferno@chromium.org, pdr@chromium.org

[inferno, 2015-01-05 06:34:47]

inferno@chromium.org changed reviewers:
+ dsinclair@chromium.org, jshin@chromium.org

[inferno, 2015-01-05 06:34:47]

I don't know this code well enough to review. added more reviewers.

[dsinclair, 2015-01-05 14:58:18]

dsinclair@chromium.org changed reviewers:
+ jchaffraix@chromium.org

[dsinclair, 2015-01-05 14:58:18]

https://codereview.chromium.org/828163002/diff/20001/LayoutTests/fast/block/c...
File
LayoutTests/fast/block/crash-when-element-becomes-positioned-and-doesnt-clear-floating-objects.html
(right):

https://codereview.chromium.org/828163002/diff/20001/LayoutTests/fast/block/c...
LayoutTests/fast/block/crash-when-element-becomes-positioned-and-doesnt-clear-floating-objects.html:14:
styleSheet0.insertRule('.container,.container,.container,.container{background-size:50;
float:right; }',styleSheet0.cssRules.length);
Do these need to be inserted, or can they be added to a <style> tag?

[Julien - ping for review, 2015-01-06 11:36:13]

lgtm.

I would advise to shorten the description's width (it fits the 80 characters
limit but is close so a revert would trim the lines, which is why 72 is a better
limit). That said, there is no consensus on the line size so the description is
fine as-is.

https://codereview.chromium.org/828163002/diff/20001/LayoutTests/fast/block/c...
File
LayoutTests/fast/block/crash-when-element-becomes-positioned-and-doesnt-clear-floating-objects.html
(right):

https://codereview.chromium.org/828163002/diff/20001/LayoutTests/fast/block/c...
LayoutTests/fast/block/crash-when-element-becomes-positioned-and-doesnt-clear-floating-objects.html:14:
styleSheet0.insertRule('.container,.container,.container,.container{background-size:50;
float:right; }',styleSheet0.cssRules.length);
On 2015/01/05 at 14:58:18, dsinclair wrote:
> Do these need to be inserted, or can they be added to a <style> tag?

Also do we need the same selector repeated 4 times in the rule? (and resp. 3 and
2 times below too).

https://codereview.chromium.org/828163002/diff/20001/LayoutTests/fast/block/c...
LayoutTests/fast/block/crash-when-element-becomes-positioned-and-doesnt-clear-floating-objects.html:21:
document.body.style.zoom=0.5145357251167297
Let's add a comment about this line changing the size of the canvas, thus
causing the crash.

Maybe we could measure the size of the canvas to be sure it's correct using
check-layout.js? I usually prefer check-layout.js tests to vanilla crash test as
it tests correctness along with not-crashing.

[rhogan, 2015-01-06 20:46:40]

The CQ bit was checked by robhogan@gmail.com

[commit-bot: I haz the power, 2015-01-06 20:47:33]

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

[commit-bot: I haz the power, 2015-01-06 21:33:34]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=187935

[Justin Novosad, 2015-01-07 17:08:44]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/810943003/ by junov@chromium.org.

The reason for reverting is: Speculative revert for crashes on WinXP bots. See
crbug.com/446834
I will re-land if this does not fix the crashes..