|
|
Chromium Code Reviews
DescriptionUse the correct crash report time
We should use the upload_time rather than the capture_time.
BUG=671897
TEST=Build for desktop linux, run, go to chrome://crash, then go to chrome://crashes,
make sure there are reports there within the last hour, then file a feedback report,
crash ids should show in the system information under "crash_report_ids".
Committed: https://crrev.com/188de26d9ef72e7c6b1a691614378842789e1a93
Cr-Commit-Position: refs/heads/master@{#437320}
Patch Set 1 #
Messages
Total messages: 15 (7 generated)
afakhry@chromium.org changed reviewers: + steel@chromium.org
Rahul, could you please take a look?
Shouldn't we fix breakpad/crashpad to correctly save the capture time? Using upload time doesn't actually get us crashes that happened in the last hour - just those uploaded in the last hour.
Description was changed from ========== Use the correct crash report time We should use the upload_time rather than the capture_time. BUG=671897 TEST=Build for desktop linux, run, go to chrome://crash, then go to chrome://crashes, make sure there are reports there within the last hour, then file a feedback report, crash ids should show in the system information under "crash_report_ids". ========== to ========== Use the correct crash report time We should use the upload_time rather than the capture_time. BUG=671897 TEST=Build for desktop linux, run, go to chrome://crash, then go to chrome://crashes, make sure there are reports there within the last hour, then file a feedback report, crash ids should show in the system information under "crash_report_ids". ==========
afakhry@chromium.org changed reviewers: + mark@chromium.org
On 2016/12/08 01:43:15, Rahul Chaturvedi wrote: > Shouldn't we fix breakpad/crashpad to correctly save the capture time? > Using upload time doesn't actually get us crashes that happened in the last hour > - just those uploaded in the last hour. I did some debugging and found out that the upload_time is the value always used in chrome://crashes, even if the crash happened a couple of seconds ago. See: https://cs.chromium.org/chromium/src/components/crash/core/browser/crashes_ui... +mark@chromium.org for comment about the use of upload_time vs capture_time.
On 2016/12/08 01:52:31, afakhry wrote: > On 2016/12/08 01:43:15, Rahul Chaturvedi wrote: > > Shouldn't we fix breakpad/crashpad to correctly save the capture time? > > Using upload time doesn't actually get us crashes that happened in the last > hour > > - just those uploaded in the last hour. > > I did some debugging and found out that the upload_time is the value always used > in chrome://crashes, even if the crash happened a couple of seconds ago. See: > https://cs.chromium.org/chromium/src/components/crash/core/browser/crashes_ui... > > mailto:+mark@chromium.org for comment about the use of upload_time vs capture_time. I think that the reason for this is simple: Breakpad doesn’t record capture time, but Crashpad does. When we started with this thing, everyone was on Breakpad, and it was an “upload list,” not even a “crash list.” All we had were uploaded crashes and their upload times. Crashpad fixes that, recording all crashes (even when upload is turned off) and tracking capture time independently from upload time. But we don’t have Crashpad everywhere yet, we’ve still got Breakpad-using platforms, which now each have different capabilities in what they can do in this department.
Given Mark's explaination, lgtm
The CQ bit was checked by afakhry@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": 1, "attempt_start_ts": 1481225068810820, "parent_rev":
"cd6132ec609f03397db2400647eec79a6a66a724", "commit_rev":
"6cfb6ec0ea6c9e29bb4ef990185be05508743b71"}
Message was sent while issue was closed.
Description was changed from ========== Use the correct crash report time We should use the upload_time rather than the capture_time. BUG=671897 TEST=Build for desktop linux, run, go to chrome://crash, then go to chrome://crashes, make sure there are reports there within the last hour, then file a feedback report, crash ids should show in the system information under "crash_report_ids". ========== to ========== Use the correct crash report time We should use the upload_time rather than the capture_time. BUG=671897 TEST=Build for desktop linux, run, go to chrome://crash, then go to chrome://crashes, make sure there are reports there within the last hour, then file a feedback report, crash ids should show in the system information under "crash_report_ids". ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Use the correct crash report time We should use the upload_time rather than the capture_time. BUG=671897 TEST=Build for desktop linux, run, go to chrome://crash, then go to chrome://crashes, make sure there are reports there within the last hour, then file a feedback report, crash ids should show in the system information under "crash_report_ids". ========== to ========== Use the correct crash report time We should use the upload_time rather than the capture_time. BUG=671897 TEST=Build for desktop linux, run, go to chrome://crash, then go to chrome://crashes, make sure there are reports there within the last hour, then file a feedback report, crash ids should show in the system information under "crash_report_ids". Committed: https://crrev.com/188de26d9ef72e7c6b1a691614378842789e1a93 Cr-Commit-Position: refs/heads/master@{#437320} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/188de26d9ef72e7c6b1a691614378842789e1a93 Cr-Commit-Position: refs/heads/master@{#437320} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
