Dangling Reference bug in LayoutGrid |
|||||||
Issue descriptionIn 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
,
Aug 29 2016
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/LayoutGrid.cpp?q=LayoutGrid::assumedRowsSizeForOrthogonalChild&sq=package:chromium&l=2029 It's on line 2029 (sorry I forgot it in the initial report)
,
Aug 29 2016
,
Aug 30 2016
Added TE-NeedsTriageHelp as it can't be triaged from TE end.
,
Aug 30 2016
,
Sep 8 2016
,
Sep 8 2016
pilki which line are you talking about? Line 2029 does not have any reference to those variables
,
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.
,
Sep 8 2016
Yeah I've already noticed that. There are 3 more that I'm removing too.
,
Sep 8 2016
,
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
,
Sep 8 2016
I don't seem to be able to comment on the diff. But you should leave the 'const'.
,
Sep 9 2016
Good to have, but not really needed right?
,
Sep 9 2016
Exactly.
,
Sep 9 2016
pilki I'll let you verify and close the bug. BTW do you have a test case which triggered the issue?
,
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?
,
Sep 9 2016
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by bzanotti@chromium.org
, Aug 29 2016Labels: -OS-Android