New issue
Advanced search Search tips

Issue 898860 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Feature



Sign in to add a comment

Crashes should be reported via Reporting API

Project Member Reported by paulmeyer@chromium.org, Oct 25

Issue description

Crash reports are a new type of report to be routed into the Reporting API backend. These reports will indicate to developers that a crash has occurred on their page (and possibly, for what reason, e.g. OOM).

Working spec:
https://w3c.github.io/reporting/#crash-report 
 

Comment 1 Deleted

We had an informal discussion about this feature amongst some members of the security team. Are we going to limit sending reports that are known renderer OOMs to the frame that crashed? Or will we also send information about renderer non-OOM crashes and potentially even browser process crashes?
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8846a41dca530bc32d3d22a0a0d8e83d466357b6

commit 8846a41dca530bc32d3d22a0a0d8e83d466357b6
Author: Paul Meyer <paulmeyer@chromium.org>
Date: Wed Oct 31 21:59:40 2018

Implement crash report prototype (behind flag)

This patch implements crash reporting, as detailed in the
reporting spec: https://w3c.github.io/reporting/#crash-report

This is a fixed version of the previously reverted CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1174965

Bug:  898860 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I95c3995bac79801e7321f8d0909563bf7bf89db8
Reviewed-on: https://chromium-review.googlesource.com/c/1220031
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Douglas Creager <dcreager@chromium.org>
Commit-Queue: Paul Meyer <paulmeyer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604388}
[modify] https://crrev.com/8846a41dca530bc32d3d22a0a0d8e83d466357b6/chrome/browser/net/reporting_browsertest.cc
[modify] https://crrev.com/8846a41dca530bc32d3d22a0a0d8e83d466357b6/content/browser/bad_message.h
[modify] https://crrev.com/8846a41dca530bc32d3d22a0a0d8e83d466357b6/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/8846a41dca530bc32d3d22a0a0d8e83d466357b6/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/8846a41dca530bc32d3d22a0a0d8e83d466357b6/content/common/frame.mojom
[modify] https://crrev.com/8846a41dca530bc32d3d22a0a0d8e83d466357b6/content/public/browser/render_frame_host.h
[modify] https://crrev.com/8846a41dca530bc32d3d22a0a0d8e83d466357b6/content/public/common/content_features.cc
[modify] https://crrev.com/8846a41dca530bc32d3d22a0a0d8e83d466357b6/content/public/common/content_features.h
[modify] https://crrev.com/8846a41dca530bc32d3d22a0a0d8e83d466357b6/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/8846a41dca530bc32d3d22a0a0d8e83d466357b6/content/test/test_render_frame.cc
[modify] https://crrev.com/8846a41dca530bc32d3d22a0a0d8e83d466357b6/services/network/network_context.cc
[modify] https://crrev.com/8846a41dca530bc32d3d22a0a0d8e83d466357b6/services/network/network_context.h
[modify] https://crrev.com/8846a41dca530bc32d3d22a0a0d8e83d466357b6/services/network/public/mojom/network_context.mojom
[modify] https://crrev.com/8846a41dca530bc32d3d22a0a0d8e83d466357b6/services/network/test/test_network_context.h
[modify] https://crrev.com/8846a41dca530bc32d3d22a0a0d8e83d466357b6/tools/metrics/histograms/enums.xml

#2:

No browser crashes will be reported; only renderer crashes. They are not currently limited to OOM crashes, but reports are marked regarding whether OOM was the known cause.
OK. When we discussed this, we had a slight preference for only sending info about known OOMs. Do website operators benefit from getting information about renderer crashes? Those seem like they'd just be Chrome bugs, and the operator wouldn't have the ability to look up the crash information anyways.
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 13

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/255b079424cd6bbb15e26bb80585d0e384fa65c4

commit 255b079424cd6bbb15e26bb80585d0e384fa65c4
Author: Paul Meyer <paulmeyer@chromium.org>
Date: Tue Nov 13 01:16:49 2018

Enable crash reports (via Reporting API) by default.

Intent to ship:
https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/ryK8SHcBzZw/2cjTF_OCBwAJ

Bug:  898860 
Change-Id: I25325827f7432dc141ae8cfe0d50e587a8f32664
Reviewed-on: https://chromium-review.googlesource.com/c/1320205
Commit-Queue: Avi Drissman <avi@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Rick Byers <rbyers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607401}
[modify] https://crrev.com/255b079424cd6bbb15e26bb80585d0e384fa65c4/content/public/common/content_features.cc

Cc: msramek@chromium.org vogelheim@chromium.org
This feature needs to go through Privacy Review before launch.

Please file a launch + privacy bug, and un-launch this before going to Beta if we cannot resolve the issue in time.


Privacy Working Group came across the intent-to-ship as part of our 'intent thread' triage. We would have preferred to hear about it sooner. We are happy with the general idea & design of giving strongly redacted crash info to the application, but have some specific concerns. We consider at least the first concern to be serious:


Concern 1:

The Chrome crash reports are considered PII (personal data), and are covered by our Privacy Policy and Privacy Whitepaper. Per policy, crash reports are not tied to any stable user ids.

However, with this feature, the report ID is available to everyone, including any Google properties. If now any Google properties use the reporting API and record the crash IDs, we have (unwittingly, and thus without oversight) created a mapping of crash report data to user accounts, which would likely be in violation of our own policy.

(Example: Say, GMail wishes to track crashes and stores the reports from the Reporting API along with their user sessions. In such a hypothetical scenario the application team would likely not consider their data PII; but they + us are the same legal entity, and that legal entity would now have the crash data PII + crash ID <-> user id mapping.)

While your feature doesn't cause this violation by itself, it does create the means for unchecked policy violations by other teams.


Concern 2:

You argue that the Crash ID is (generally) not interpretable by (outside) developers. We agree. But then, why give it to them?

It seems like there is an intended use case that would (somehow) close the loop from Crash ID to crash report, but it is left entirely unspecified how that might look like. This feels almost like a "backdoor", in that we hand out GUIDs whose sole purpose is to identify a certain piece of data; but don't say anything about how that would be used. But quite apparently the intent is that it will get used, somehow, under some circumstances.



Loosely related questions:

- I found one piece of code that randomly generates the crash ID (good!), but that seems to assume that it can also be generated elsewhere. Where can I find that other place that might generate it?

https://cs.chromium.org/chromium/src/content/browser/frame_host/render_frame_host_impl.cc?l=6108-6109


- In RenderFrameHostImpl:::NotifyWebReportingCrashID, the code overwrites the pre-existing GUID iff it is valid. Is this intentional?

https://cs.chromium.org/chromium/src/content/browser/frame_host/render_frame_host_impl.cc?l=2946-2948

Project Member

Comment 9 by bugdroid1@chromium.org, Dec 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9887251b9a16ab4651666766fb5717fc9ba8497e

commit 9887251b9a16ab4651666766fb5717fc9ba8497e
Author: Paul Meyer <paulmeyer@chromium.org>
Date: Mon Dec 03 16:41:29 2018

Remove crash IDs from Reporting API crash reports.

Due to some privacy concerns that were raised (see bug comment #8), this
patch removes crash IDs from Reporting API crash reports. The changes to
spec are here: https://github.com/w3c/reporting/pull/136

Bug:  898860 
Change-Id: I4085cb837c6b904d4ea1b32a6dd0930d507bb760
Reviewed-on: https://chromium-review.googlesource.com/c/1354229
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Paul Meyer <paulmeyer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613120}
[modify] https://crrev.com/9887251b9a16ab4651666766fb5717fc9ba8497e/chrome/browser/net/reporting_browsertest.cc
[modify] https://crrev.com/9887251b9a16ab4651666766fb5717fc9ba8497e/content/browser/bad_message.h
[modify] https://crrev.com/9887251b9a16ab4651666766fb5717fc9ba8497e/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/9887251b9a16ab4651666766fb5717fc9ba8497e/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/9887251b9a16ab4651666766fb5717fc9ba8497e/content/common/frame.mojom
[modify] https://crrev.com/9887251b9a16ab4651666766fb5717fc9ba8497e/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/9887251b9a16ab4651666766fb5717fc9ba8497e/content/test/test_render_frame.cc
[modify] https://crrev.com/9887251b9a16ab4651666766fb5717fc9ba8497e/tools/metrics/histograms/enums.xml

Project Member

Comment 10 by bugdroid1@chromium.org, Dec 28

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f9914a73ac7a8181a75f12e32da20943df1058be

commit f9914a73ac7a8181a75f12e32da20943df1058be
Author: Paul Meyer <paulmeyer@chromium.org>
Date: Fri Dec 28 21:20:30 2018

Add "unresponsive" reason for crash reports.

This patch adds "unresponsive" as a possible value for the |reason|
field in crash reports (Reporting API), as well as logic to detect when
the page has been killed due to being unresponsive.

This is in line with this section of the Reporting API spec:
https://w3c.github.io/reporting/#crashreportbody-reason

Bug:  898860 
Change-Id: I755e97a63d9b67f6def0b65c0847f8a7065c6d7f
Reviewed-on: https://chromium-review.googlesource.com/c/1382753
Commit-Queue: Paul Meyer <paulmeyer@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619195}
[modify] https://crrev.com/f9914a73ac7a8181a75f12e32da20943df1058be/chrome/browser/net/reporting_browsertest.cc
[modify] https://crrev.com/f9914a73ac7a8181a75f12e32da20943df1058be/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/f9914a73ac7a8181a75f12e32da20943df1058be/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/f9914a73ac7a8181a75f12e32da20943df1058be/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/f9914a73ac7a8181a75f12e32da20943df1058be/content/browser/renderer_host/render_process_host_impl.h

Status: Fixed (was: Started)

Sign in to add a comment