Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(303)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2133603002: Fix crash when removing contents from a document having multiple bodies (Closed)

Created:
4 years, 5 months ago by Xianzhu
Modified:
4 years, 5 months ago
Reviewers:
chrishtr
CC:
ajuma+watch_chromium.org, blink-reviews, blink-reviews-layout_chromium.org, blink-reviews-platform-graphics_chromium.org, Rik, chromium-reviews, danakj+watch_chromium.org, dshwang, drott+blinkwatch_chromium.org, krit, eae+blinkwatch, f(malita), jbroman, jchaffraix+rendering, Justin Novosad, leviw+renderwatch, pdr+graphicswatchlist_chromium.org, pdr+renderingwatchlist_chromium.org, rwlbuis, Stephen Chennney, szager+layoutwatch_chromium.org, zoltan1
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix crash when removing contents from a document having multiple bodies Removed the early return under condition 'isBody()' from LayoutObjectChildList::invalidatePaintOnRemoval() to ensure the painting layer and body object are invalidated. BTW fixed issues of DisplayItemClient aliveness checking which made it not actually work for subsequences. (If we didn't have the issues, we should have caught this bug through aliveness-checking.) Still not sure if we could have more reduced test. For the test, removing anything from the test would make the test not reproducing the bug. A normal removal of <body> couldn't reproduce the bug because we will invalidate the painting layer in other paths (e.g. layout triggered invalidation, etc.). BUG=626182 Committed: https://crrev.com/a0ccb0df60af245b02a405fef95822b658f10381 Cr-Commit-Position: refs/heads/master@{#404552}

Patch Set 1 #

Patch Set 2 : - #

Total comments: 2

Patch Set 3 : Handle LayoutScrollbarPart aliveness #

Messages

Total messages: 15 (8 generated)
Xianzhu
https://codereview.chromium.org/2133603002/diff/20001/third_party/WebKit/Source/core/frame/FrameView.cpp File third_party/WebKit/Source/core/frame/FrameView.cpp (left): https://codereview.chromium.org/2133603002/diff/20001/third_party/WebKit/Source/core/frame/FrameView.cpp#oldcode2627 third_party/WebKit/Source/core/frame/FrameView.cpp:2627: #endif This cleared the aliveness information after each painting, ...
4 years, 5 months ago (2016-07-08 16:56:40 UTC) #3
chrishtr
lgtm
4 years, 5 months ago (2016-07-08 17:43:22 UTC) #5
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/2133603002/20001
4 years, 5 months ago (2016-07-08 17:43:49 UTC) #6
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/259343)
4 years, 5 months ago (2016-07-08 19:38:08 UTC) #8
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/2133603002/40001
4 years, 5 months ago (2016-07-08 20:25:18 UTC) #11
commit-bot: I haz the power
Committed patchset #3 (id:40001)
4 years, 5 months ago (2016-07-09 00:47:56 UTC) #13
commit-bot: I haz the power
4 years, 5 months ago (2016-07-09 00:49:12 UTC) #15
Message was sent while issue was closed. 
Patchset 3 (id:??) landed as
https://crrev.com/a0ccb0df60af245b02a405fef95822b658f10381
Cr-Commit-Position: refs/heads/master@{#404552}

Powered by Google App Engine
This is Rietveld 408576698