Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2697533003:
Fix crash in widget destruction (Closed)
|
Created:
3 years, 10 months ago by yiyix Modified:
3 years, 10 months ago CC:
chromium-reviews, oshima+watch_chromium.org, aboxhall+watch_chromium.org, tfarina, nektar+watch_chromium.org, yuzo+watch_chromium.org, je_julie, dmazzoni+watch_chromium.org, dtseng+watch_chromium.org, davemoore+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFix crash in widget destruction
During the widget destruction, chrome crashes if this widget's |view|
visibility changes during |view|'s destruction.
During the destruction of the widget, the root view is reset to nullptr
first, then the destruction of its children views start. If |view|
updates its visibility, it needs to access the root view which is set
to nullptr earlier. This is where the crash happens.
TEST=AXViewTest.LayoutCalledInvalidateRootView
BUG=680536
Review-Url: https://codereview.chromium.org/2697533003
Cr-Commit-Position: refs/heads/master@{#450198}
Committed: https://chromium.googlesource.com/chromium/src/+/437537edc6fe22fd892be8fc83a5b303378f19b7
Patch Set 1 #Patch Set 2 : fix mac #
Messages
Total messages: 31 (25 generated)
Description was changed from ========== work version BUG=680536 ========== to ========== Fix crash in widget destruction During the widget destruction, chrome crashes if this widget's |view| visibility changes during |view|'s destruction. During the destruction of the widget, the root view is reset to nullptr first, then the destruction of its children views start. If |view| updates its visibility, it needs to access the root view which is set to nullptr earlier. This is where the crash happens. TEST=AXViewTest.LayoutCalledInvalidateRootView BUG=680536 ==========
yiyix@chromium.org changed reviewers: + sadrul@chromium.org
yiyix@chromium.org changed reviewers: - sadrul@chromium.org
Patchset #2 (id:20001) has been deleted
Patchset #1 (id:1) has been deleted
Patchset #1 (id:40001) has been deleted
yiyix@chromium.org changed reviewers: + sadrul@chromium.org
@sadrul, could you please review this change? Thank you very much
The CQ bit was checked by yiyix@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
dmazzoni@chromium.org changed reviewers: + dmazzoni@chromium.org
lgtm, thanks for the good test
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
lgtm thanks! (and as discussed offline, making the test aura-only should fix the failure on mac bots, since mac does not use aura or AxAuraObjCache)
Patchset #2 (id:80001) has been deleted
The CQ bit was checked by yiyix@chromium.org to run a CQ dry run
On 2017/02/13 20:21:36, sadrul wrote: > lgtm thanks! (and as discussed offline, making the test aura-only should fix the > failure on mac bots, since mac does not use aura or AxAuraObjCache) @sadrul and dmazzonl, thank you for the review.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by yiyix@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by yiyix@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dmazzoni@chromium.org, sadrul@chromium.org Link to the patchset: https://codereview.chromium.org/2697533003/#ps100001 (title: "fix mac")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 100001, "attempt_start_ts": 1487037640246000,
"parent_rev": "879aaac59dca262da89e57b8048722dd69050ff9", "commit_rev":
"437537edc6fe22fd892be8fc83a5b303378f19b7"}
Message was sent while issue was closed.
Description was changed from ========== Fix crash in widget destruction During the widget destruction, chrome crashes if this widget's |view| visibility changes during |view|'s destruction. During the destruction of the widget, the root view is reset to nullptr first, then the destruction of its children views start. If |view| updates its visibility, it needs to access the root view which is set to nullptr earlier. This is where the crash happens. TEST=AXViewTest.LayoutCalledInvalidateRootView BUG=680536 ========== to ========== Fix crash in widget destruction During the widget destruction, chrome crashes if this widget's |view| visibility changes during |view|'s destruction. During the destruction of the widget, the root view is reset to nullptr first, then the destruction of its children views start. If |view| updates its visibility, it needs to access the root view which is set to nullptr earlier. This is where the crash happens. TEST=AXViewTest.LayoutCalledInvalidateRootView BUG=680536 Review-Url: https://codereview.chromium.org/2697533003 Cr-Commit-Position: refs/heads/master@{#450198} Committed: https://chromium.googlesource.com/chromium/src/+/437537edc6fe22fd892be8fc83a5... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:100001) as https://chromium.googlesource.com/chromium/src/+/437537edc6fe22fd892be8fc83a5... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
