New issue
Advanced search Search tips

Issue 641982 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: ----

Blocking:
issue 79180



Sign in to add a comment

Dangling Reference bug in LayoutGrid

Project Member Reported by pilki@google.com, Aug 29 2016

Issue description

In https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/LayoutGrid.cpp , there is a dangling reference bug:
maxTrackSize is a reference to the m_maxTrackBreadth local variable of the temporary gridTrackSize
 
Components: Blink
Labels: -OS-Android
Components: -Blink Blink>Layout
Labels: TE-NeedsTriageHelp
Added TE-NeedsTriageHelp as it can't be triaged from TE end.
Project Member

Comment 5 by sheriffbot@chromium.org, Aug 30 2016

Labels: Hotlist-Google

Comment 6 by e...@chromium.org, Sep 8 2016

Components: -Blink>Layout Blink>Layout>Grid
Owner: svil...@igalia.com
Status: Assigned (was: Unconfirmed)

Comment 7 by svil...@igalia.com, Sep 8 2016

pilki which line are you talking about? Line 2029 does not have any reference to those variables

Comment 8 by pilki@google.com, Sep 8 2016

const GridLength& maxTrackSize = gridTrackSize(ForRows, trackPosition, sizingOperation).maxTrackBreadth();

line 2083 now.

the fix is just to remove the & to copy the GridLength.

Comment 9 by svil...@igalia.com, Sep 8 2016

Yeah I've already noticed that. There are 3 more that I'm removing too.

Comment 10 by r...@igalia.com, Sep 8 2016

Blocking: 79180
Cc: jfernan...@igalia.com r...@igalia.com
Labels: -TE-NeedsTriageHelp
Project Member

Comment 11 by bugdroid1@chromium.org, Sep 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eac7349745d4d88c888718f54f40c13aa3132085

commit eac7349745d4d88c888718f54f40c13aa3132085
Author: svillar <svillar@igalia.com>
Date: Thu Sep 08 16:37:47 2016

[css-grid] Fix a dangling reference

The code was making a reference to an attribute of a temporary object created by gridTrackSize().

BUG= 641982 

Review-Url: https://codereview.chromium.org/2325603003
Cr-Commit-Position: refs/heads/master@{#417309}

[modify] https://crrev.com/eac7349745d4d88c888718f54f40c13aa3132085/third_party/WebKit/Source/core/layout/LayoutGrid.cpp

Comment 12 by pilki@google.com, Sep 8 2016

I don't seem to be able to comment on the diff. But you should leave the 'const'.
Good to have, but not really needed right?

Comment 14 by pilki@google.com, Sep 9 2016

Exactly.
pilki I'll let you verify and close the bug.

BTW do you have a test case which triggered the issue?

Comment 16 by pilki@google.com, Sep 9 2016

The fix looks good to me.

I don't have a test case, this was found by static analysis.

I don't seem to have the power to close the bug though. Can you do it?
Status: Fixed (was: Assigned)

Sign in to add a comment