Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2572573003:
Remove old diagnostic crash reports. (Closed)
|
Created:
4 years ago by Charlie Reis Modified:
3 years, 7 months ago CC:
chromium-reviews, sadrul, nasko+codewatch_chromium.org, lcwu+watch_chromium.org, jam, darin-cc_chromium.org, halliwell+watch_chromium.org, kalyank, mlamouri+watch-content_chromium.org, creis+watch_chromium.org, alokp+watch_chromium.org, site-isolation-reviews_chromium.org, Simeon Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionRemove old diagnostic crash reports.
We aren't actively investigating these, and they're generating
crash reports without telling us new information. Remove the old
diagnostics.
This basically reverts the following CLs from 575245:
https://codereview.chromium.org/1613453002/
https://codereview.chromium.org/1618063003/
https://codereview.chromium.org/1643673002/
https://codereview.chromium.org/1656603002/
(I left https://codereview.chromium.org/1581193002/ in place,
since it seems useful as a safeguard. I also left
https://codereview.chromium.org/1624413002/ because it seems
to be indicative of a problem we might not be catching.)
BUG=575245
TEST=Fewer crash reports
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
Review-Url: https://codereview.chromium.org/2572573003
Cr-Commit-Position: refs/heads/master@{#470669}
Committed: https://chromium.googlesource.com/chromium/src/+/825893d489d4d482b8ff1f346dc2a1195aa7fc7f
Patch Set 1 #Patch Set 2 : Remove more crash keys. #
Total comments: 2
Patch Set 3 : Preserve routing ID. #
Total comments: 8
Patch Set 4 : Rebase #Patch Set 5 : Fix suggestions #Patch Set 6 : Fix typo. #
Total comments: 3
Patch Set 7 : Restore RenderViewHostImpl dump #Messages
Total messages: 54 (36 generated)
Description was changed from ========== Remove old diagnostic crash reports now that bug seems fixed. We haven't seen any RenderFrameImpl::didCommitProvisionalLoad crashes since M53, so these extra dianostics shouldn't be needed anymore. BUG=575245 TEST=Fewer crash reports ========== to ========== Remove old diagnostic crash reports now that bug seems fixed. We haven't seen any RenderFrameImpl::didCommitProvisionalLoad crashes since M53, so these extra dianostics shouldn't be needed anymore. BUG=575245 TEST=Fewer crash reports CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ==========
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 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...
Description was changed from ========== Remove old diagnostic crash reports now that bug seems fixed. We haven't seen any RenderFrameImpl::didCommitProvisionalLoad crashes since M53, so these extra dianostics shouldn't be needed anymore. BUG=575245 TEST=Fewer crash reports CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ========== to ========== Remove old diagnostic crash reports now that bug seems fixed. We haven't seen any RenderFrameImpl::didCommitProvisionalLoad crashes since M53, so these extra dianostics shouldn't be needed anymore. This basically reverts the following CLs from 575245: https://codereview.chromium.org/1613453002/ https://codereview.chromium.org/1618063003/ https://codereview.chromium.org/1624413002/ https://codereview.chromium.org/1643673002/ https://codereview.chromium.org/1656603002/ (I left https://codereview.chromium.org/1581193002/ in place, since it seems useful as a safeguard.) BUG=575245 TEST=Fewer crash reports CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ==========
creis@chromium.org changed reviewers: + alexmos@chromium.org
Alex, can you take a look? It would help to get a sanity check that these can all be safely removed now, and that I did the reverts properly. From my quick queries on the crash server, it seems like RenderFrameHostManager::InitRenderFrame is the only one still getting hit. Maybe there's a legit case for that? Or does it seem like a problem? RenderFrameImpl::didCommitProvisionalLoad and RenderViewHostImpl::CreateRenderView haven't happened since M53. https://codereview.chromium.org/2572573003/diff/20001/content/renderer/render... File content/renderer/render_frame_impl.cc (right): https://codereview.chromium.org/2572573003/diff/20001/content/renderer/render... content/renderer/render_frame_impl.cc:1740: CHECK(!render_view->main_render_frame_); I left this in place, since it seems like a useful safeguard for the future (and I couldn't find cases of it getting hit).
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
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.
Thanks for the cleanup! Regarding whether we need this data anymore, I've double-checked the crash reports and found a few interesting cases: 1) 414a103900000000 and a11184db00000000 from 56.0.2890.0 seem to hit the original CHECK. There are some from M55 as well. 2) Looks like there are some other crashes in RFI::didCommitProvisionalLoad as well, e.g., 4182c82300000000 in 57.0.2948.0 and there are a couple in other places in M56 as well: https://crash.corp.google.com/browse?q=custom_data.ChromeCrashProto.magic_sig... Don't know if the crash keys would be helpful for 2), but given 1), do you think we should keep these for a bit longer, since the last crashes were in M56 and not M53? There's a chance Lucas's CL will fix some more of these problems (thanks to the is_active() fix in OnSwappedOut), so fingers crossed for that. https://codereview.chromium.org/2572573003/diff/20001/content/renderer/render... File content/renderer/render_frame_impl.cc (right): https://codereview.chromium.org/2572573003/diff/20001/content/renderer/render... content/renderer/render_frame_impl.cc:1740: CHECK(!render_view->main_render_frame_); On 2016/12/13 00:07:57, Charlie Reis wrote: > I left this in place, since it seems like a useful safeguard for the future (and > I couldn't find cases of it getting hit). Acknowledged. https://codereview.chromium.org/2572573003/diff/40001/content/browser/frame_h... File content/browser/frame_host/render_frame_host_manager.cc (left): https://codereview.chromium.org/2572573003/diff/40001/content/browser/frame_h... content/browser/frame_host/render_frame_host_manager.cc:1992: base::debug::DumpWithoutCrashing(); Looks like this one is still happening (e.g., fa1ecae300000000 in 57.0.2948.0), but I don't think getting here means that we've hit a bug. I think this should be safe to remove. (Most of the InitRenderFrame DWOC's are actually from below, for the CreateFrame bug which is still active, though there's a chance that Lucas's CL might fix it.) https://codereview.chromium.org/2572573003/diff/40001/content/browser/rendere... File content/browser/renderer_host/render_view_host_impl.cc (left): https://codereview.chromium.org/2572573003/diff/40001/content/browser/rendere... content/browser/renderer_host/render_view_host_impl.cc:284: proxy_route_id != MSG_ROUTING_NONE) Would there be value in keeping this as a DCHECK? And/or a CHECK, perhaps in RVI::Initialize? AFAICT, we happily create both a main RF and a proxy if this ever happens, and I don't think any good can come out of that. :) https://codereview.chromium.org/2572573003/diff/40001/content/browser/rendere... File content/browser/renderer_host/render_view_host_impl.cc (right): https://codereview.chromium.org/2572573003/diff/40001/content/browser/rendere... content/browser/renderer_host/render_view_host_impl.cc:14: #include "base/debug/dump_without_crashing.h" Can this be removed now? https://codereview.chromium.org/2572573003/diff/40001/content/renderer/render... File content/renderer/render_view_impl.cc (left): https://codereview.chromium.org/2572573003/diff/40001/content/renderer/render... content/renderer/render_view_impl.cc:601: base::debug::SetCrashKeyValue("rvinit_view_id", I'll mention that there was some value in looking at these ones in the crash dumps while debugging some of the other crashes, such as https://crbug.com/626802. It's probably ok to still remove them, and we can re-add them if we need them again under the right bug.
On 2016/12/13 20:28:55, alexmos wrote: > Thanks for the cleanup! Regarding whether we need this data anymore, I've > double-checked the crash reports and found a few interesting cases: > > 1) 414a103900000000 and a11184db00000000 from 56.0.2890.0 seem to hit the > original CHECK. There are some from M55 as well. > > 2) Looks like there are some other crashes in RFI::didCommitProvisionalLoad as > well, e.g., 4182c82300000000 in 57.0.2948.0 and there are a couple in other > places in M56 as well: > > https://crash.corp.google.com/browse?q=custom_data.ChromeCrashProto.magic_sig... > > Don't know if the crash keys would be helpful for 2), but given 1), do you think > we should keep these for a bit longer, since the last crashes were in M56 and > not M53? There's a chance Lucas's CL will fix some more of these problems > (thanks to the is_active() fix in OnSwappedOut), so fingers crossed for that. > Huh, I must have had something wrong in my crash query when I went to check! I do see the recent crashes using your query. Given that, let's leave the diagnostics in place for now and try to figure out why they're still happening. We can land this after resolving it.
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: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
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 ========== Remove old diagnostic crash reports now that bug seems fixed. We haven't seen any RenderFrameImpl::didCommitProvisionalLoad crashes since M53, so these extra dianostics shouldn't be needed anymore. This basically reverts the following CLs from 575245: https://codereview.chromium.org/1613453002/ https://codereview.chromium.org/1618063003/ https://codereview.chromium.org/1624413002/ https://codereview.chromium.org/1643673002/ https://codereview.chromium.org/1656603002/ (I left https://codereview.chromium.org/1581193002/ in place, since it seems useful as a safeguard.) BUG=575245 TEST=Fewer crash reports CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ========== to ========== Remove old diagnostic crash reports now that bug seems fixed. We haven't seen any RenderFrameImpl::didCommitProvisionalLoad crashes since M53, so these extra dianostics shouldn't be needed anymore. This basically reverts the following CLs from 575245: https://codereview.chromium.org/1613453002/ https://codereview.chromium.org/1618063003/ https://codereview.chromium.org/1643673002/ https://codereview.chromium.org/1656603002/ (I left https://codereview.chromium.org/1581193002/ in place, since it seems useful as a safeguard. I also left https://codereview.chromium.org/1624413002/ because it seems to be indicative of a problem we might not be catching.) BUG=575245 TEST=Fewer crash reports CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ==========
Description was changed from ========== Remove old diagnostic crash reports now that bug seems fixed. We haven't seen any RenderFrameImpl::didCommitProvisionalLoad crashes since M53, so these extra dianostics shouldn't be needed anymore. This basically reverts the following CLs from 575245: https://codereview.chromium.org/1613453002/ https://codereview.chromium.org/1618063003/ https://codereview.chromium.org/1643673002/ https://codereview.chromium.org/1656603002/ (I left https://codereview.chromium.org/1581193002/ in place, since it seems useful as a safeguard. I also left https://codereview.chromium.org/1624413002/ because it seems to be indicative of a problem we might not be catching.) BUG=575245 TEST=Fewer crash reports CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ========== to ========== Remove old diagnostic crash reports. We aren't actively investigating these, and they're generating crash reports without telling us new information. Remove the old diagnostics. This basically reverts the following CLs from 575245: https://codereview.chromium.org/1613453002/ https://codereview.chromium.org/1618063003/ https://codereview.chromium.org/1643673002/ https://codereview.chromium.org/1656603002/ (I left https://codereview.chromium.org/1581193002/ in place, since it seems useful as a safeguard. I also left https://codereview.chromium.org/1624413002/ because it seems to be indicative of a problem we might not be catching.) BUG=575245 TEST=Fewer crash reports CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ==========
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: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
Alex, can you give this another look? I updated it with what we discussed last week. Thanks! https://codereview.chromium.org/2572573003/diff/40001/content/browser/frame_h... File content/browser/frame_host/render_frame_host_manager.cc (left): https://codereview.chromium.org/2572573003/diff/40001/content/browser/frame_h... content/browser/frame_host/render_frame_host_manager.cc:1992: base::debug::DumpWithoutCrashing(); On 2016/12/13 20:28:55, alexmos (OOO until 5-2) wrote: > Looks like this one is still happening (e.g., fa1ecae300000000 in 57.0.2948.0), > but I don't think getting here means that we've hit a bug. I think this should > be safe to remove. (Most of the InitRenderFrame DWOC's are actually from below, > for the CreateFrame bug which is still active, though there's a chance that > Lucas's CL might fix it.) Yes, let's remove it-- I'm not convinced we're getting value from it. https://codereview.chromium.org/2572573003/diff/40001/content/browser/rendere... File content/browser/renderer_host/render_view_host_impl.cc (left): https://codereview.chromium.org/2572573003/diff/40001/content/browser/rendere... content/browser/renderer_host/render_view_host_impl.cc:284: proxy_route_id != MSG_ROUTING_NONE) On 2016/12/13 20:28:55, alexmos (OOO until 5-2) wrote: > Would there be value in keeping this as a DCHECK? And/or a CHECK, perhaps in > RVI::Initialize? AFAICT, we happily create both a main RF and a proxy if this > ever happens, and I don't think any good can come out of that. :) Sure, we can keep it as a DCHECK here. https://codereview.chromium.org/2572573003/diff/40001/content/browser/rendere... File content/browser/renderer_host/render_view_host_impl.cc (right): https://codereview.chromium.org/2572573003/diff/40001/content/browser/rendere... content/browser/renderer_host/render_view_host_impl.cc:14: #include "base/debug/dump_without_crashing.h" On 2016/12/13 20:28:55, alexmos (OOO until 5-2) wrote: > Can this be removed now? Done. https://codereview.chromium.org/2572573003/diff/40001/content/renderer/render... File content/renderer/render_view_impl.cc (left): https://codereview.chromium.org/2572573003/diff/40001/content/renderer/render... content/renderer/render_view_impl.cc:601: base::debug::SetCrashKeyValue("rvinit_view_id", On 2016/12/13 20:28:55, alexmos (OOO until 5-2) wrote: > I'll mention that there was some value in looking at these ones in the crash > dumps while debugging some of the other crashes, such as > https://crbug.com/626802. It's probably ok to still remove them, and we can > re-add them if we need them again under the right bug. Fair point, but I think it's worth trying to clean them up if we can. As you mention, we can add them back if needed for another investigation. https://codereview.chromium.org/2572573003/diff/100001/content/browser/frame_... File content/browser/frame_host/render_frame_host_manager.cc (right): https://codereview.chromium.org/2572573003/diff/100001/content/browser/frame_... content/browser/frame_host/render_frame_host_manager.cc:2083: base::debug::DumpWithoutCrashing(); Agreed that about 2/3 of the reports are from this one. We should probably remove these as well if we're not actively using them to diagnose https://crbug.com/591478. https://codereview.chromium.org/2572573003/diff/100001/content/renderer/rende... File content/renderer/render_frame_impl.cc (left): https://codereview.chromium.org/2572573003/diff/100001/content/renderer/rende... content/renderer/render_frame_impl.cc:5165: "commit_main_render_frame_id", I don't see any instances of this crash key value since M55. Interestingly, that's before we even moved this code from didCommitProvisionalLoad to SwapIn (in M57), so the fact that we're seeing crashes on the !in_frame_tree_ line above doesn't explain the difference. https://crash.corp.google.com/browse?q=OMIT%20RECORD%20IF%20SOME(ProductData.... If it does recur, we can always add diagnostics again.
Alex, can you give this another look? I updated it with what we discussed last week. Thanks! https://codereview.chromium.org/2572573003/diff/40001/content/browser/frame_h... File content/browser/frame_host/render_frame_host_manager.cc (left): https://codereview.chromium.org/2572573003/diff/40001/content/browser/frame_h... content/browser/frame_host/render_frame_host_manager.cc:1992: base::debug::DumpWithoutCrashing(); On 2016/12/13 20:28:55, alexmos (OOO until 5-2) wrote: > Looks like this one is still happening (e.g., fa1ecae300000000 in 57.0.2948.0), > but I don't think getting here means that we've hit a bug. I think this should > be safe to remove. (Most of the InitRenderFrame DWOC's are actually from below, > for the CreateFrame bug which is still active, though there's a chance that > Lucas's CL might fix it.) Yes, let's remove it-- I'm not convinced we're getting value from it. https://codereview.chromium.org/2572573003/diff/40001/content/browser/rendere... File content/browser/renderer_host/render_view_host_impl.cc (left): https://codereview.chromium.org/2572573003/diff/40001/content/browser/rendere... content/browser/renderer_host/render_view_host_impl.cc:284: proxy_route_id != MSG_ROUTING_NONE) On 2016/12/13 20:28:55, alexmos (OOO until 5-2) wrote: > Would there be value in keeping this as a DCHECK? And/or a CHECK, perhaps in > RVI::Initialize? AFAICT, we happily create both a main RF and a proxy if this > ever happens, and I don't think any good can come out of that. :) Sure, we can keep it as a DCHECK here. https://codereview.chromium.org/2572573003/diff/40001/content/browser/rendere... File content/browser/renderer_host/render_view_host_impl.cc (right): https://codereview.chromium.org/2572573003/diff/40001/content/browser/rendere... content/browser/renderer_host/render_view_host_impl.cc:14: #include "base/debug/dump_without_crashing.h" On 2016/12/13 20:28:55, alexmos (OOO until 5-2) wrote: > Can this be removed now? Done. https://codereview.chromium.org/2572573003/diff/40001/content/renderer/render... File content/renderer/render_view_impl.cc (left): https://codereview.chromium.org/2572573003/diff/40001/content/renderer/render... content/renderer/render_view_impl.cc:601: base::debug::SetCrashKeyValue("rvinit_view_id", On 2016/12/13 20:28:55, alexmos (OOO until 5-2) wrote: > I'll mention that there was some value in looking at these ones in the crash > dumps while debugging some of the other crashes, such as > https://crbug.com/626802. It's probably ok to still remove them, and we can > re-add them if we need them again under the right bug. Fair point, but I think it's worth trying to clean them up if we can. As you mention, we can add them back if needed for another investigation. https://codereview.chromium.org/2572573003/diff/100001/content/browser/frame_... File content/browser/frame_host/render_frame_host_manager.cc (right): https://codereview.chromium.org/2572573003/diff/100001/content/browser/frame_... content/browser/frame_host/render_frame_host_manager.cc:2083: base::debug::DumpWithoutCrashing(); Agreed that about 2/3 of the reports are from this one. We should probably remove these as well if we're not actively using them to diagnose https://crbug.com/591478. https://codereview.chromium.org/2572573003/diff/100001/content/renderer/rende... File content/renderer/render_frame_impl.cc (left): https://codereview.chromium.org/2572573003/diff/100001/content/renderer/rende... content/renderer/render_frame_impl.cc:5165: "commit_main_render_frame_id", I don't see any instances of this crash key value since M55. Interestingly, that's before we even moved this code from didCommitProvisionalLoad to SwapIn (in M57), so the fact that we're seeing crashes on the !in_frame_tree_ line above doesn't explain the difference. https://crash.corp.google.com/browse?q=OMIT%20RECORD%20IF%20SOME(ProductData.... If it does recur, we can always add diagnostics again.
LGTM, thanks for the cleanup! And I hope we can track down that DWOC in CreateRenderView - it's concerning that we're getting it again. It looks like we didn't get any of those reports in versions 57-58, but they started again with 59, with nine crashes so far in 59 and 60 [1]. I bet it has something to do with PlzNavigate, since all nine are in "BrowserSideNavigation: Enabled_50". Perhaps we should file a separate bug to investigate it. [1] https://crash.corp.google.com/browse?q=custom_data.ChromeCrashProto.magic_sig... https://codereview.chromium.org/2572573003/diff/100001/content/renderer/rende... File content/renderer/render_frame_impl.cc (left): https://codereview.chromium.org/2572573003/diff/100001/content/renderer/rende... content/renderer/render_frame_impl.cc:5165: "commit_main_render_frame_id", On 2017/05/09 16:42:26, Charlie Reis wrote: > I don't see any instances of this crash key value since M55. Interestingly, > that's before we even moved this code from didCommitProvisionalLoad to SwapIn > (in M57), so the fact that we're seeing crashes on the !in_frame_tree_ line > above doesn't explain the difference. > > https://crash.corp.google.com/browse?q=OMIT%20RECORD%20IF%20SOME(ProductData.... > > If it does recur, we can always add diagnostics again. Acknowledged.
On 2017/05/09 21:26:55, alexmos wrote: > LGTM, thanks for the cleanup! > > And I hope we can track down that DWOC in CreateRenderView - it's concerning > that we're getting it again. It looks like we didn't get any of those reports > in versions 57-58, but they started again with 59, with nine crashes so far in > 59 and 60 [1]. I bet it has something to do with PlzNavigate, since all nine > are in "BrowserSideNavigation: Enabled_50". Perhaps we should file a separate > bug to investigate it. > Good call-- I filed https://crbug.com/720116.
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...
creis@chromium.org changed reviewers: + grt@chromium.org, lcwu@chromium.org, rsesek@chromium.org
OWNERS reviews: rsesek: Can you review chrome/common/crash_keys.cc? grt: Can you review chrome/app/chrome_crash_reporter_client_win.cc? lcwu: Can you review chromecast/crash/cast_crash_keys.cc?
lcwu@google.com changed reviewers: + lcwu@google.com
chromecast/ lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?))
chrome/app/chrome_crash_reporter_client_win.cc lgtm
lgtm
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...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
chromecast/ lgtm
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...
CQ is committing da patch.
Bot data: {"patchset_id": 120001, "attempt_start_ts": 1494436175541840,
"parent_rev": "290f2bede4df8363122591f27baf118f227841a5", "commit_rev":
"825893d489d4d482b8ff1f346dc2a1195aa7fc7f"}
Message was sent while issue was closed.
Description was changed from ========== Remove old diagnostic crash reports. We aren't actively investigating these, and they're generating crash reports without telling us new information. Remove the old diagnostics. This basically reverts the following CLs from 575245: https://codereview.chromium.org/1613453002/ https://codereview.chromium.org/1618063003/ https://codereview.chromium.org/1643673002/ https://codereview.chromium.org/1656603002/ (I left https://codereview.chromium.org/1581193002/ in place, since it seems useful as a safeguard. I also left https://codereview.chromium.org/1624413002/ because it seems to be indicative of a problem we might not be catching.) BUG=575245 TEST=Fewer crash reports CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ========== to ========== Remove old diagnostic crash reports. We aren't actively investigating these, and they're generating crash reports without telling us new information. Remove the old diagnostics. This basically reverts the following CLs from 575245: https://codereview.chromium.org/1613453002/ https://codereview.chromium.org/1618063003/ https://codereview.chromium.org/1643673002/ https://codereview.chromium.org/1656603002/ (I left https://codereview.chromium.org/1581193002/ in place, since it seems useful as a safeguard. I also left https://codereview.chromium.org/1624413002/ because it seems to be indicative of a problem we might not be catching.) BUG=575245 TEST=Fewer crash reports CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation Review-Url: https://codereview.chromium.org/2572573003 Cr-Commit-Position: refs/heads/master@{#470669} Committed: https://chromium.googlesource.com/chromium/src/+/825893d489d4d482b8ff1f346dc2... ==========
Message was sent while issue was closed.
Committed patchset #7 (id:120001) as https://chromium.googlesource.com/chromium/src/+/825893d489d4d482b8ff1f346dc2... |
