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

Issue 1133533003: Omnibox - Log Addition Crash Data for Rare HQP Crash (Closed)

Created:
5 years, 7 months ago by Mark P
Modified:
5 years, 7 months ago
CC:
chromium-reviews, James Su
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Omnibox - Log Addition Crash Data for Rare HQP Crash In the days of yore, there was an HQP crash: https://code.google.com/p/chromium/issues/detail?id=359270 I put a band-aid on the crash: https://codereview.chromium.org/222783006 Later, I fixed the root cause: https://codereview.chromium.org/255423002 Or at least I thought I did and close that days-of-yore bug. Apparently this didn't entirely fix the problem. (See the linked bug below.) This change adds some information to the minidumps to allow me to investigate what kinds of inputs are causing trouble. BUG=464926 Committed: https://crrev.com/55b415e1407aa2fd0b7f095c9972c28fb053cb96 Cr-Commit-Position: refs/heads/master@{#329703}

Patch Set 1 #

Patch Set 2 : better key name #

Total comments: 4

Patch Set 3 : dcheck -> check #

Patch Set 4 : more descriptive CHECK message - DO NOT SUBMIT #

Patch Set 5 : register crash key #

Patch Set 6 : reuse constant #

Total comments: 2

Patch Set 7 : use fixed_keys array #

Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -1 line) Patch
M chrome/browser/autocomplete/history_quick_provider.cc View 1 2 3 4 5 3 chunks +9 lines, -1 line 0 comments Download
M chrome/common/crash_keys.h View 1 2 3 4 5 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/common/crash_keys.cc View 1 2 3 4 5 6 2 chunks +3 lines, -0 lines 0 comments Download

Messages

Total messages: 25 (9 generated)
Mark P
This like replaces https://codereview.chromium.org/1124033003/ I like this idea of yours. Please take a look. thanks, ...
5 years, 7 months ago (2015-05-11 23:31:35 UTC) #2
Peter Kasting
LGTM https://codereview.chromium.org/1133533003/diff/20001/chrome/browser/autocomplete/history_quick_provider.cc File chrome/browser/autocomplete/history_quick_provider.cc (right): https://codereview.chromium.org/1133533003/diff/20001/chrome/browser/autocomplete/history_quick_provider.cc#newcode268 chrome/browser/autocomplete/history_quick_provider.cc:268: DCHECK(!new_matches.empty()); Can you upgrade this DCHECK to CHECK ...
5 years, 7 months ago (2015-05-11 23:34:42 UTC) #3
Mark P
https://codereview.chromium.org/1133533003/diff/20001/chrome/browser/autocomplete/history_quick_provider.cc File chrome/browser/autocomplete/history_quick_provider.cc (right): https://codereview.chromium.org/1133533003/diff/20001/chrome/browser/autocomplete/history_quick_provider.cc#newcode268 chrome/browser/autocomplete/history_quick_provider.cc:268: DCHECK(!new_matches.empty()); On 2015/05/11 23:34:42, Peter Kasting wrote: > Can ...
5 years, 7 months ago (2015-05-11 23:45:15 UTC) #4
Peter Kasting
https://codereview.chromium.org/1133533003/diff/20001/chrome/browser/autocomplete/history_quick_provider.cc File chrome/browser/autocomplete/history_quick_provider.cc (right): https://codereview.chromium.org/1133533003/diff/20001/chrome/browser/autocomplete/history_quick_provider.cc#newcode268 chrome/browser/autocomplete/history_quick_provider.cc:268: DCHECK(!new_matches.empty()); On 2015/05/11 23:45:15, Mark P wrote: > On ...
5 years, 7 months ago (2015-05-11 23:47:13 UTC) #5
Mark P
https://codereview.chromium.org/1133533003/diff/20001/chrome/browser/autocomplete/history_quick_provider.cc File chrome/browser/autocomplete/history_quick_provider.cc (right): https://codereview.chromium.org/1133533003/diff/20001/chrome/browser/autocomplete/history_quick_provider.cc#newcode268 chrome/browser/autocomplete/history_quick_provider.cc:268: DCHECK(!new_matches.empty()); On 2015/05/11 23:47:13, Peter Kasting wrote: > On ...
5 years, 7 months ago (2015-05-11 23:49:59 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1133533003/40001
5 years, 7 months ago (2015-05-11 23:51:47 UTC) #9
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_gn_rel on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_gn_rel/builds/87687)
5 years, 7 months ago (2015-05-12 00:49:32 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1133533003/40001
5 years, 7 months ago (2015-05-12 04:11:42 UTC) #13
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_gn_rel on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_gn_rel/builds/87786)
5 years, 7 months ago (2015-05-12 05:00:46 UTC) #15
Mark P
rsesek, Can you take a look at this change from the crash keys perspective? I ...
5 years, 7 months ago (2015-05-12 23:47:28 UTC) #17
Robert Sesek
Looks mostly right. FYI documentation is here: http://dev.chromium.org/developers/debugging-with-crash-keys https://codereview.chromium.org/1133533003/diff/100001/chrome/common/crash_keys.cc File chrome/common/crash_keys.cc (right): https://codereview.chromium.org/1133533003/diff/100001/chrome/common/crash_keys.cc#newcode248 chrome/common/crash_keys.cc:248: // ...
5 years, 7 months ago (2015-05-13 15:22:34 UTC) #18
Mark P
https://codereview.chromium.org/1133533003/diff/100001/chrome/common/crash_keys.cc File chrome/common/crash_keys.cc (right): https://codereview.chromium.org/1133533003/diff/100001/chrome/common/crash_keys.cc#newcode248 chrome/common/crash_keys.cc:248: // Register a key to help investigate bug 464926. ...
5 years, 7 months ago (2015-05-13 17:50:14 UTC) #19
Robert Sesek
lgtm
5 years, 7 months ago (2015-05-13 18:33:46 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1133533003/120001
5 years, 7 months ago (2015-05-13 19:13:32 UTC) #23
commit-bot: I haz the power
Committed patchset #7 (id:120001)
5 years, 7 months ago (2015-05-13 20:05:40 UTC) #24
commit-bot: I haz the power
5 years, 7 months ago (2015-05-13 20:06:44 UTC) #25
Message was sent while issue was closed.
Patchset 7 (id:??) landed as
https://crrev.com/55b415e1407aa2fd0b7f095c9972c28fb053cb96
Cr-Commit-Position: refs/heads/master@{#329703}

Powered by Google App Engine
This is Rietveld 408576698