Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(32)

Issue 2636153002: Don't keep pointers to table sections when told to recalculate sections. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 months, 1 week ago by mstensho (USE GERRIT)
Modified:
7 months, 1 week ago
Reviewers:
dgrogan, eae
CC:
chromium-reviews, szager+layoutwatch_chromium.org, zoltan1, blink-reviews-layout_chromium.org, pdr+renderingwatchlist_chromium.org, eae+blinkwatch, leviw+renderwatch, jchaffraix+rendering, blink-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Don't keep pointers to table sections when told to recalculate sections. They may have been deleted. We should ideally assert in header(), footer() and firstBody() in LayoutTable that we're not waiting for a section recalc, but that's currently failing in one trybot; see crbug.com/693212 This would have fixed the original use-after-free issue with bug 680224, but before this CL landed, I found another fix that attacked the root cause instead. Still no point in keeping potentially dead pointers around, though. BUG=680224 Review-Url: https://codereview.chromium.org/2636153002 Cr-Commit-Position: refs/heads/master@{#451257} Committed: https://chromium.googlesource.com/chromium/src/+/25993aa7c80d68de1a53bff157d02c8bc8ec3458

Patch Set 1 #

Patch Set 2 : rebase master #

Patch Set 3 : rebase master #

Patch Set 4 : Disable one DCHECK that causes failures on the linux trybot. #

Messages

Total messages: 52 (23 generated)
mstensho (USE GERRIT)
8 months, 1 week ago (2017-01-17 15:24:52 UTC) #6
eae
This is great, thank you.
8 months, 1 week ago (2017-01-17 15:58:33 UTC) #7
eae
LGTM
8 months, 1 week ago (2017-01-17 15:58:39 UTC) #8
dgrogan
LGTM 2 We get an assertion failure in debug but a harmless (not use-afer-free) crash ...
8 months, 1 week ago (2017-01-17 16:00:22 UTC) #9
mstensho (USE GERRIT)
On 2017/01/17 16:00:22, dgrogan wrote: > LGTM 2 > > We get an assertion failure ...
8 months, 1 week ago (2017-01-17 16:31:16 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2636153002/1
8 months, 1 week ago (2017-01-17 17:14:07 UTC) #12
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/372212)
8 months, 1 week ago (2017-01-17 19:46:45 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2636153002/1
8 months, 1 week ago (2017-01-17 20:03:29 UTC) #16
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/372299)
8 months, 1 week ago (2017-01-17 22:31:28 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2636153002/1
8 months, 1 week ago (2017-01-18 08:54:32 UTC) #20
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/372845)
8 months, 1 week ago (2017-01-18 11:17:18 UTC) #22
mstensho (USE GERRIT)
On 2017/01/18 11:17:18, commit-bot: I haz the power wrote: > Try jobs failed on following ...
8 months, 1 week ago (2017-01-18 13:13:43 UTC) #23
dgrogan
I don't know how to reproduce locally either, but +wangxianzhu probably does :)
8 months, 1 week ago (2017-01-18 16:46:15 UTC) #24
dgrogan
I don't know how to reproduce locally either, but +wangxianzhu probably does :)
8 months, 1 week ago (2017-01-18 16:46:15 UTC) #25
Xianzhu
On 2017/01/18 16:46:15, dgrogan wrote: > I don't know how to reproduce locally either, but ...
8 months, 1 week ago (2017-01-18 17:05:37 UTC) #26
mstensho (USE GERRIT)
On 2017/01/18 17:05:37, Xianzhu wrote: > On 2017/01/18 16:46:15, dgrogan wrote: > > I don't ...
8 months, 1 week ago (2017-01-18 19:39:52 UTC) #27
Xianzhu
It seems that we need some time to fix the dirty layout issues. Should we ...
8 months, 1 week ago (2017-01-18 22:45:53 UTC) #28
mstensho (USE GERRIT)
On 2017/01/18 22:45:53, Xianzhu wrote: > It seems that we need some time to fix ...
8 months, 1 week ago (2017-01-19 10:00:16 UTC) #29
Xianzhu
Can anyone who can access the bug add me to its cc list?
8 months, 1 week ago (2017-01-19 17:09:21 UTC) #30
mstensho (USE GERRIT)
On 2017/01/19 17:09:21, Xianzhu wrote: > Can anyone who can access the bug add me ...
8 months, 1 week ago (2017-01-19 17:38:58 UTC) #32
eae
On 2017/01/19 17:38:58, mstensho wrote: > On 2017/01/19 17:09:21, Xianzhu wrote: > > Can anyone ...
8 months ago (2017-01-23 18:18:53 UTC) #33
mstensho (USE GERRIT)
On 2017/01/23 18:18:53, eae wrote: > On 2017/01/19 17:38:58, mstensho wrote: > > On 2017/01/19 ...
8 months ago (2017-01-23 18:22:52 UTC) #34
mstensho (USE GERRIT)
On 2017/01/18 22:45:53, Xianzhu wrote: > It seems that we need some time to fix ...
7 months, 1 week ago (2017-02-16 13:47:56 UTC) #39
Xianzhu
On 2017/02/16 13:47:56, mstensho wrote: > On 2017/01/18 22:45:53, Xianzhu wrote: > > It seems ...
7 months, 1 week ago (2017-02-16 16:45:35 UTC) #40
mstensho (USE GERRIT)
On 2017/02/16 16:45:35, Xianzhu wrote: > On 2017/02/16 13:47:56, mstensho wrote: > > On 2017/01/18 ...
7 months, 1 week ago (2017-02-16 21:37:57 UTC) #42
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2636153002/60001
7 months, 1 week ago (2017-02-16 21:40:50 UTC) #45
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/368319)
7 months, 1 week ago (2017-02-17 00:07:51 UTC) #47
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2636153002/60001
7 months, 1 week ago (2017-02-17 06:28:53 UTC) #49
commit-bot: I haz the power
7 months, 1 week ago (2017-02-17 07:11:12 UTC) #52
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/25993aa7c80d68de1a53bff157d0...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b