Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2260383002:
Don't crash after calling frame_->stopLoading(). (Closed)
|
Created:
4 years, 4 months ago by Charlie Reis Modified:
4 years, 4 months ago Reviewers:
dcheng CC:
chromium-reviews, nasko+codewatch_chromium.org, mlamouri+watch-content_chromium.org, jam, darin-cc_chromium.org, creis+watch_chromium.org, nasko, site-isolation-reviews_chromium.org, Nate Chapin, lfg Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionDon't crash after calling frame_->stopLoading().
It's possible for frame_ to be deleted after this call, so add null
checks after call sites.
BUG=639688, 639689
TEST=No more OnStop or NavigateInternal crashes in RenderFrameImpl.
Committed: https://crrev.com/594e9909a655dd435a9a7b0ccc049423f71b6dc7
Cr-Commit-Position: refs/heads/master@{#413438}
Patch Set 1 #
Messages
Total messages: 18 (13 generated)
The CQ bit was checked by creis@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: Exceeded global retry quota
The CQ bit was checked by creis@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.
Description was changed from ========== Don't crash after calling frame_->stopLoading(). It's possible for frame_ to be deleted after this call, so add null checks after call sites. BUG= TEST=No more OnStop or NavigateInternal crashes in RenderFrameImpl. ========== to ========== Don't crash after calling frame_->stopLoading(). It's possible for frame_ to be deleted after this call, so add null checks after call sites. BUG=639688, 639689 TEST=No more OnStop or NavigateInternal crashes in RenderFrameImpl. ==========
creis@chromium.org changed reviewers: + dcheng@chromium.org
Daniel, do you know how calling frame_->stopLoading() might cause frame_ to be null? I'm curious if this is just a case we need to handle, or if it's indicative of a problem. (It's also unclear to me if the RenderFrame itself may be deleted after the stopLoading call, or if a null check on frame_ is sufficient.) Thoughts? I'd like to land something to reduce the crashes here, since Fracas labeled it as ReleaseBlock-Dev.
LGTM. stopLoading() can dispatch events, so I guess this is expected.
The CQ bit was checked by creis@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Don't crash after calling frame_->stopLoading(). It's possible for frame_ to be deleted after this call, so add null checks after call sites. BUG=639688, 639689 TEST=No more OnStop or NavigateInternal crashes in RenderFrameImpl. ========== to ========== Don't crash after calling frame_->stopLoading(). It's possible for frame_ to be deleted after this call, so add null checks after call sites. BUG=639688, 639689 TEST=No more OnStop or NavigateInternal crashes in RenderFrameImpl. ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Don't crash after calling frame_->stopLoading(). It's possible for frame_ to be deleted after this call, so add null checks after call sites. BUG=639688, 639689 TEST=No more OnStop or NavigateInternal crashes in RenderFrameImpl. ========== to ========== Don't crash after calling frame_->stopLoading(). It's possible for frame_ to be deleted after this call, so add null checks after call sites. BUG=639688, 639689 TEST=No more OnStop or NavigateInternal crashes in RenderFrameImpl. Committed: https://crrev.com/594e9909a655dd435a9a7b0ccc049423f71b6dc7 Cr-Commit-Position: refs/heads/master@{#413438} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/594e9909a655dd435a9a7b0ccc049423f71b6dc7 Cr-Commit-Position: refs/heads/master@{#413438} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
