Considering implementing the Reporting API

RESOLVED FIXED in Firefox 65

Status

()

enhancement
P3
normal
RESOLVED FIXED
7 months ago
3 months ago

People

(Reporter: baku, Assigned: baku)

Tracking

(Blocks 1 bug, {dev-doc-needed})

unspecified
mozilla65
Points:
---

Firefox Tracking Flags

(firefox65 fixed)

Details

(Whiteboard: [domsecurity-backlog1][wptsync upstream], URL)

Attachments

(8 attachments, 21 obsolete attachments)

25.17 KB, patch
smaug
: review+
Details | Diff | Splinter Review
20.95 KB, patch
smaug
: review+
Details | Diff | Splinter Review
9.60 KB, patch
smaug
: review+
Details | Diff | Splinter Review
15.18 KB, patch
smaug
: review+
Details | Diff | Splinter Review
21.78 KB, patch
smaug
: review+
Details | Diff | Splinter Review
12.11 KB, patch
smaug
: review+
Details | Diff | Splinter Review
7.77 KB, patch
smaug
: review+
Details | Diff | Splinter Review
11.78 KB, patch
smaug
: review+
Details | Diff | Splinter Review
(Assignee)

Description

7 months ago
Reporting API is needed for FeaturePolicy and other new APIs.
(Assignee)

Updated

7 months ago
Whiteboard: [domsecurity-backlog1]
(Assignee)

Comment 1

7 months ago
Posted patch part 1 - Header Parsing (obsolete) — Splinter Review
This first patch is about parsing the Report-To header and create a data struct is memory.
Attachment #9010206 - Flags: feedback?(bugs)
(Assignee)

Comment 2

7 months ago
Posted patch part 2 - WebIDL interfaces (obsolete) — Splinter Review
Main WebIDL interfaces
Assignee: nobody → amarchesini
Attachment #9010207 - Flags: feedback?(bugs)
(Assignee)

Comment 3

7 months ago
Attachment #9010208 - Flags: feedback?(bugs)
(Assignee)

Comment 4

7 months ago
Attachment #9010210 - Flags: feedback?(bugs)
(Assignee)

Comment 5

7 months ago
Posted patch part 5 - IPC (obsolete) — Splinter Review
Attachment #9010211 - Flags: feedback?(bugs)
(Assignee)

Comment 6

7 months ago
WIP, patch unfinished.
Duplicate of this bug: 1489431
(Assignee)

Comment 8

7 months ago
I still don't know if I have time to work on this. If somebody wants to continue these patches, I'm happy to help.
Assignee: amarchesini → nobody
Has our legal team / privacy team reviewed this feature?
In particular, its compliance with the GDPR laws in Europe.

Personally, I find the discussion of Privacy in the spec under
https://w3c.github.io/reporting/#privacy
rather naive.  It fails to recognize that some of those practices
could very well be illegal under the GDPR.  Implementing these
features in the UA instead might make the UA vendor liable,
which it would not be if the web page itself did the tracking.

Also, https://w3c.github.io/reporting/#disable
says "User agents MUST allow users to disable reporting".
The GDPR requires that the user has given explicit consent using
a "positive opt-in" mechanism before collecting any personal data.
An opt-out mechanism is not good enough.
https://ico.org.uk/for-organisations/guide-to-the-general-data-protection-regulation-gdpr/lawful-basis-for-processing/consent/
(Assignee)

Comment 10

7 months ago
Thanks for pointing out this issue. Privacy is one of the reason why I stopped the implementation before implementing the sending of the reports.

Just yesterday I was discussing with ckerschb the idea of landing just part of this API.
Reporting API is composed, basically, by 3 independent blocks:
 - the report-to header and the delivering of report messages to a server.
 - ReportingObserver webIDL component which allow a context to receive reports for its own origin (similar to BroadcastChannel).
 - A set of ReportBody: deprecated APIs, feature policy violations, and so on.

What I propose is to implement just the last 2 parts, without sending reports to servers.
But is that compatible with other browsers? Though, perhaps we could pretend that as if user had always opted-out. What do other browsers do (is that Chrome in practice)?

If the spec doesn't comply with GDPR, it needs to be changed. Has anyone filed a spec issue?
Or contacted legal to ask feedback to this issue?
Flags: needinfo?(amarchesini)
(Assignee)

Comment 12

7 months ago
Marshall, can you please help us with this API? We would like to know if this API is OK to be implemented from a GDPR point of view. The spec doesn't say anything in particular. Note that we have a similar implementation for CSP3 violation reporting.
Flags: needinfo?(amarchesini) → needinfo?(merwin)
(Assignee)

Comment 13

6 months ago
Posted patch part 1 - WebIDL (obsolete) — Splinter Review
Assignee: nobody → amarchesini
Attachment #9010206 - Attachment is obsolete: true
Attachment #9010207 - Attachment is obsolete: true
Attachment #9010208 - Attachment is obsolete: true
Attachment #9010210 - Attachment is obsolete: true
Attachment #9010211 - Attachment is obsolete: true
Attachment #9010212 - Attachment is obsolete: true
Attachment #9010206 - Flags: feedback?(bugs)
Attachment #9010207 - Flags: feedback?(bugs)
Attachment #9010208 - Flags: feedback?(bugs)
Attachment #9010210 - Flags: feedback?(bugs)
Attachment #9010211 - Flags: feedback?(bugs)
Attachment #9015793 - Flags: review?(bugs)
(Assignee)

Comment 14

6 months ago
Attachment #9015794 - Flags: review?(bugs)
(Assignee)

Comment 15

6 months ago
Attachment #9015795 - Flags: review?(bugs)
(Assignee)

Comment 16

6 months ago
Posted patch part 4 - IPC (obsolete) — Splinter Review
Attachment #9015796 - Flags: review?(bugs)
(Assignee)

Comment 17

6 months ago
Posted patch part 5 - tests (obsolete) — Splinter Review
Attachment #9015844 - Flags: review?(bugs)
(Assignee)

Comment 18

6 months ago
Posted patch part 5 - tests (obsolete) — Splinter Review
Attachment #9015844 - Attachment is obsolete: true
Attachment #9015844 - Flags: review?(bugs)
Attachment #9015862 - Flags: review?(bugs)
(Assignee)

Comment 19

6 months ago
Attachment #9015886 - Flags: review?(bugs)
(Assignee)

Comment 20

6 months ago
Attachment #9015889 - Flags: review?(bugs)
(Assignee)

Comment 21

6 months ago
Posted patch part 1 - WebIDL (obsolete) — Splinter Review
Attachment #9015793 - Attachment is obsolete: true
Attachment #9015793 - Flags: review?(bugs)
Attachment #9015899 - Flags: review?(bugs)
Comment on attachment 9015899 [details] [diff] [review]
part 1 - WebIDL

>+ReportingObserver::Constructor(const GlobalObject& aGlobal,
>+                               ReportingObserverCallback& aCallback,
>+                               const ReportingObserverOptions& aOptions,
>+                               ErrorResult& aRv)
>+{
>+  nsCOMPtr<nsIGlobalObject> global = do_QueryInterface(aGlobal.GetAsSupports());
>+  MOZ_ASSERT(global);
>+
>+  nsCOMPtr<nsPIDOMWindowInner> window = do_QueryInterface(global);
>+  if (NS_WARN_IF(!window)) {
>+    aRv.Throw(NS_ERROR_UNEXPECTED);
>+    return nullptr;
>+  }
>+
>+  // ReportingObserver behaves as BroadcastChannel and we don't want to allow
>+  // the communication between contexts blocked by anti-tracking policies.
>+  if (nsContentUtils::IsThirdPartyWindowOrChannel(window, nullptr,
>+                                                  nullptr) &&
>+      nsContentUtils::StorageAllowedForWindow(window) !=
>+        nsContentUtils::StorageAccess::eAllow) {
>+    aRv.Throw(NS_ERROR_DOM_SECURITY_ERR);
>+    return nullptr;
I don't like throwing from a constructor which per spec can't throw. This could break pages (sure, those would be 3rd party but still).
Could we rather make the ReportingObserver somehow dummy or so?
(looks like BroadcastChannel is similarly broken)
Attachment #9015899 - Flags: review?(bugs) → review-

Updated

6 months ago
Attachment #9015794 - Flags: review?(bugs) → review+
Comment on attachment 9015795 [details] [diff] [review]
part 3 - DOM Bindings + Deprecation reports


>+++ b/dom/reporting/ReportingUtils.cpp
>@@ -0,0 +1,19 @@
>+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
>+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
>+/* This Source Code Form is subject to the terms of the Mozilla Public
>+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
>+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
>+
>+#include "mozilla/dom/ReportingUtils.h"
>+#include "mozilla/dom/ReportBody.h"
>+
>+using namespace mozilla::dom;
>+
>+/* static */ void
>+ReportingUtils::Report(const nsAString& aType,
Shouldn't this take nsAtom*. Or, perhaps enum value. That way we'd guarantee no weird things get passed here.
... maybe enum is too restricting and converting to string is harder. So, use atom here?

You'd need to add deprecation to xpcom/ds/StaticAtoms.py
Attachment #9015795 - Flags: review?(bugs) → review+
Comment on attachment 9015889 [details] [diff] [review]
part 7 - FeaturePolicy WPTs


>-  assert_equals(report.body.feature, "camera");
>+  assert_equals(report.body.featureId, "camera");
there is no featureId in any ReportBodies

check also other properties.
Attachment #9015889 - Flags: review?(bugs) → review-

Updated

6 months ago
Attachment #9015862 - Flags: review?(bugs) → review+
(Assignee)

Comment 25

6 months ago
Posted patch part 1 - WebIDL (obsolete) — Splinter Review
Attachment #9015899 - Attachment is obsolete: true
Attachment #9019043 - Flags: review?(bugs)
(Assignee)

Comment 26

6 months ago
Comment on attachment 9015889 [details] [diff] [review]
part 7 - FeaturePolicy WPTs

I and smaug have discussed the comments on IRC.
Attachment #9015889 - Flags: review- → review?(bugs)
(Assignee)

Comment 27

6 months ago
Note that FeaturePolicyViolationReportBody is part of https://wicg.github.io/feature-policy
Comment on attachment 9015889 [details] [diff] [review]
part 7 - FeaturePolicy WPTs

>-  assert_equals(typeof report.body.message, "string");
>-  assert_equals(typeof report.body.lineNumber, "number");
>-  assert_equals(typeof report.body.columnNumber, "number");
>+  assert_equals(report.body.lineNumber, 25);
>+  assert_equals(report.body.columnNumber, 24);
>+  assert_equals(report.body.disposition, "enforce");
Do all the browsers report exactly the same lines/columns? If not, could you not do this change.
(I guess .message check should be dropped, and disposition check is fine)

> var check_report_format = (reports, observer) => {
>   let report = reports[0];
>-  assert_equals(report.type, "feature-policy");
>+  assert_equals(report.type, "feature-policy-violation");
>   assert_equals(report.url, document.location.href);
>-  assert_equals(report.body.feature, "document-write");
>+  assert_equals(report.body.featureId, "document-write");
>   assert_equals(report.body.sourceFile, document.location.href);
>-  assert_equals(typeof report.body.message, "string");
>-  assert_equals(typeof report.body.lineNumber, "number");
>-  assert_equals(typeof report.body.columnNumber, "number");
>+  assert_equals(report.body.lineNumber, 27);
>+  assert_equals(report.body.columnNumber, 18);
>+  assert_equals(report.body.disposition, "enforce");
similar here


> var check_report_format = (reports, observer) => {
>   let report = reports[0];
>-  assert_equals(report.type, "feature-policy");
>+  assert_equals(report.type, "feature-policy-violation");
>   assert_equals(report.url, document.location.href);
>-  assert_equals(report.body.feature, "fullscreen");
>+  assert_equals(report.body.featureId, "fullscreen");
>   assert_equals(report.body.sourceFile, document.location.href);
>-  assert_equals(typeof report.body.message, "string");
>-  assert_equals(typeof report.body.lineNumber, "number");
>-  assert_equals(typeof report.body.columnNumber, "number");
>+  assert_equals(report.body.lineNumber, 24);
>+  assert_equals(report.body.columnNumber, 32);
>+  assert_equals(report.body.disposition, "enforce");
and here


> var check_report_format = (reports, observer) => {
>   let report = reports[0];
>-  assert_equals(report.type, "feature-policy");
>+  assert_equals(report.type, "feature-policy-violation");
>   assert_equals(report.url, document.location.href);
>-  assert_equals(report.body.feature, "geolocation");
>-  assert_equals(report.body.sourceFile, document.location.href);
>-  assert_equals(typeof report.body.message, "string");
>+  assert_equals(report.body.featureId, "geolocation");
>   assert_equals(typeof report.body.lineNumber, "number");
>   assert_equals(typeof report.body.columnNumber, "number");
>+  assert_equals(report.body.disposition, "enforce");
(and here you do keep typeof checks, which is fine)


> 
> var check_report_format = (reports, observer) => {
>   let report = reports[0];
>-  assert_equals(report.type, "feature-policy");
>+  assert_equals(report.type, "feature-policy-violation");
>   assert_equals(report.url, document.location.href);
>-  assert_equals(report.body.feature, "microphone");
>+  assert_equals(report.body.featureId, "microphone");
>   assert_equals(report.body.sourceFile, document.location.href);
>-  assert_equals(typeof report.body.message, "string");
>-  assert_equals(typeof report.body.lineNumber, "number");
>-  assert_equals(typeof report.body.columnNumber, "number");
>+  assert_equals(report.body.lineNumber, 25);
>+  assert_equals(report.body.columnNumber, 24);
>+  assert_equals(report.body.disposition, "enforce");
but here not.
> var check_report_format = (reports, observer) => {
>   let report = reports[0];
>-  assert_equals(report.type, "feature-policy");
>+  assert_equals(report.type, "feature-policy-violation");
>   assert_equals(report.url, document.location.href);
>-  assert_equals(report.body.feature, "midi");
>+  assert_equals(report.body.featureId, "midi");
>   assert_equals(report.body.sourceFile, document.location.href);
>-  assert_equals(typeof report.body.message, "string");
>-  assert_equals(typeof report.body.lineNumber, "number");
>-  assert_equals(typeof report.body.columnNumber, "number");
>+  assert_equals(report.body.lineNumber, 25);
>+  assert_equals(report.body.columnNumber, 44);
>+  assert_equals(report.body.disposition, "enforce");
ditto

>     <script>
> var t = async_test("PaymentRequest Report Format");
> 
> var check_report_format = (reports, observer) => {
>   let report = reports[0];
>-  assert_equals(report.type, "feature-policy");
>+  assert_equals(report.type, "feature-policy-violation");
>   assert_equals(report.url, document.location.href);
>-  assert_equals(report.body.feature, "payment");
>+  assert_equals(report.body.featureId, "payment");
>   assert_equals(report.body.sourceFile, document.location.href);
>-  assert_equals(typeof report.body.message, "string");
>-  assert_equals(typeof report.body.lineNumber, "number");
>-  assert_equals(typeof report.body.columnNumber, "number");
>+  assert_equals(report.body.lineNumber, 27);
>+  assert_equals(report.body.columnNumber, 8);
>+  assert_equals(report.body.disposition, "enforce");
ditto

> var t = async_test("Sync-xhr Report Format");
> 
> var check_report_format = (reports, observer) => {
>   let report = reports[0];
>-  assert_equals(report.type, "feature-policy");
>+  assert_equals(report.type, "feature-policy-violation");
>   assert_equals(report.url, document.location.href);
>-  assert_equals(report.body.feature, "sync-xhr");
>+  assert_equals(report.body.featureId, "sync-xhr");
>   assert_equals(report.body.sourceFile, document.location.href);
>-  assert_equals(typeof report.body.message, "string");
>-  assert_equals(typeof report.body.lineNumber, "number");
>-  assert_equals(typeof report.body.columnNumber, "number");
>+  assert_equals(report.body.lineNumber, 29);
>+  assert_equals(report.body.columnNumber, 15);
>+  assert_equals(report.body.disposition, "enforce");
ditto

> var t = async_test("USB Report Format");
> 
> var check_report_format = (reports, observer) => {
>   let report = reports[0];
>-  assert_equals(report.type, "feature-policy");
>+  assert_equals(report.type, "feature-policy-violation");
>   assert_equals(report.url, document.location.href);
>-  assert_equals(report.body.feature, "usb");
>+  assert_equals(report.body.featureId, "usb");
>   assert_equals(report.body.sourceFile, document.location.href);
>-  assert_equals(typeof report.body.message, "string");
>-  assert_equals(typeof report.body.lineNumber, "number");
>-  assert_equals(typeof report.body.columnNumber, "number");
>+  assert_equals(report.body.lineNumber, 25);
>+  assert_equals(report.body.columnNumber, 15);
ditto


>     <script>
> var check_report_format = (reports, observer) => {
>   let report = reports[0];
>-  assert_equals(report.type, "feature-policy");
>+  assert_equals(report.type, "feature-policy-violation");
>   assert_equals(report.url, document.location.href);
>-  assert_equals(report.body.feature, "vr");
>+  assert_equals(report.body.featureId, "vr");
>   assert_equals(report.body.sourceFile, document.location.href);
>-  assert_equals(typeof report.body.message, "string");
>-  assert_equals(typeof report.body.lineNumber, "number");
>-  assert_equals(typeof report.body.columnNumber, "number");
>+  assert_equals(report.body.lineNumber, 25);
>+  assert_equals(report.body.columnNumber, 44);
>+  assert_equals(report.body.disposition, "enforce");
ditto

>   <body>
>     <script>
> var check_report_format = (reports, observer) => {
>   let report = reports[0];
>-  assert_equals(report.type, "feature-policy");
>+  assert_equals(report.type, "feature-policy-violation");
>   assert_equals(report.url, document.location.href);
>-  assert_equals(report.body.feature, "vr");
>+  assert_equals(report.body.featureId, "vr");
>   assert_equals(report.body.sourceFile, document.location.href);
>-  assert_equals(typeof report.body.message, "string");
>-  assert_equals(typeof report.body.lineNumber, "number");
>-  assert_equals(typeof report.body.columnNumber, "number");
>+  assert_equals(report.body.lineNumber, 25);
>+  assert_equals(report.body.columnNumber, 55);
>+  assert_equals(report.body.disposition, "enforce");
and here


So, don't make the lineNumber and columnNumber changes unless browsers report those values the same way.
Attachment #9015889 - Flags: review?(bugs) → review+

Updated

6 months ago
Attachment #9019043 - Flags: review?(bugs) → review+
Comment on attachment 9015796 [details] [diff] [review]
part 4 - IPC

(needs changes per IRC)
Attachment #9015796 - Flags: review?(bugs)
Comment on attachment 9015886 [details] [diff] [review]
part 6 - FeaturePolicy violation

(needs changes per IRC)
Attachment #9015886 - Flags: review?(bugs)
(Assignee)

Comment 31

6 months ago
Posted patch part 4 - reporting (obsolete) — Splinter Review
Attachment #9015796 - Attachment is obsolete: true
Attachment #9021765 - Flags: review?(bugs)
(Assignee)

Comment 32

6 months ago
Attachment #9015886 - Attachment is obsolete: true
Attachment #9021766 - Flags: review?(bugs)
(Assignee)

Comment 33

6 months ago
Posted patch part 8 - memory-pressure (obsolete) — Splinter Review
Attachment #9021767 - Flags: review?(bugs)
(Assignee)

Comment 34

6 months ago
Attachment #9019043 - Attachment is obsolete: true
Attachment #9021790 - Flags: review?(bugs)
(Assignee)

Comment 35

6 months ago
Attachment #9015794 - Attachment is obsolete: true
Attachment #9021791 - Flags: review?(bugs)
(Assignee)

Comment 36

6 months ago
Attachment #9015795 - Attachment is obsolete: true
Attachment #9021792 - Flags: review?(bugs)
(Assignee)

Comment 37

6 months ago
Posted patch part 4 - reporting (obsolete) — Splinter Review
Attachment #9021765 - Attachment is obsolete: true
Attachment #9021765 - Flags: review?(bugs)
Attachment #9021793 - Flags: review?(bugs)
(Assignee)

Comment 38

6 months ago
Attachment #9021794 - Flags: review?(bugs)
(Assignee)

Updated

6 months ago
Attachment #9021766 - Attachment is obsolete: true
Attachment #9021766 - Flags: review?(bugs)
(Assignee)

Comment 39

6 months ago
Attachment #9021793 - Attachment is obsolete: true
Attachment #9021793 - Flags: review?(bugs)
Attachment #9021879 - Flags: review?(bugs)
(Assignee)

Comment 40

6 months ago
Attachment #9015862 - Attachment is obsolete: true
Attachment #9021882 - Flags: review?(bugs)
(Assignee)

Comment 41

6 months ago
Attachment #9021794 - Attachment is obsolete: true
Attachment #9021794 - Flags: review?(bugs)
Attachment #9021884 - Flags: review?(bugs)

Updated

6 months ago
Flags: needinfo?(merwin)
Comment on attachment 9021767 [details] [diff] [review]
part 8 - memory-pressure

So ReportingObserver is kept alive as long as the window, even if disconnect() is called, since observer service has a strong reference to the object. window's unlink must get called to ensure no runtime leaks. But, in general unlink may not be called ever.
And calling Shutdown() in dtor is rather useless, since sure the object isn't registered to observer service at that point anymore.
Attachment #9021767 - Flags: review?(bugs) → review-
Comment on attachment 9021790 [details] [diff] [review]
part 1 - WebIDL

Need to then in other patches ensure that ReportingObserver which is observing changes is kept alive even if GC runs.
Attachment #9021790 - Flags: review?(bugs) → review+

Updated

6 months ago
Attachment #9021791 - Flags: review?(bugs) → review+
Comment on attachment 9021884 [details] [diff] [review]
part 6 - FeaturePolicy violation


>+  nsAutoCString fileName;
>+  Nullable<int32_t> lineNumber;
>+  Nullable<int32_t> columnNumber;
>+  uint32_t line = 0;
>+  uint32_t column = 0;
>+  if (nsJSUtils::GetCallingLocation(cx, fileName, &line, &column)) {
>+    lineNumber.SetValue(static_cast<int32_t>(line));
>+    columnNumber.SetValue(static_cast<int32_t>(column));
>+  }
lease test how this all works when JS code in window A calls some API in window B and that operation isn't
allowed because of feature policy.
Attachment #9021884 - Flags: review?(bugs) → review+

Updated

6 months ago
Attachment #9021882 - Flags: review?(bugs) → review+
Comment on attachment 9021792 [details] [diff] [review]
part 3 - DOM Bindings + Deprecation reports


>@@ -4065,16 +4180,18 @@ private:
>     WorkerPrivate* wp = aWorkerPrivate;
>     while (wp->GetParent()) {
>       wp = wp->GetParent();
>     }
> 
>     nsPIDOMWindowInner* window = wp->GetWindow();
>     if (window && window->GetExtantDoc()) {
>       window->GetExtantDoc()->WarnOnceAbout(mOperation);
>+      MaybeReportDeprecation(aWorkerPrivate, mOperation, mFileName,
>+                             mLineNumber, mColumnNumber);
>     }
Why do we post this to the main thread?
The spec says
"Note: reporting observers can only observe reports from the same environment settings object"

>@@ -4096,28 +4213,55 @@ DeprecationWarning(JSContext* aCx, JSObj
> void
> DeprecationWarning(const GlobalObject& aGlobal,
>                    nsIDocument::DeprecatedOperations aOperation)
> {
>   if (NS_IsMainThread()) {
>     nsCOMPtr<nsPIDOMWindowInner> window = do_QueryInterface(aGlobal.GetAsSupports());
>     if (window && window->GetExtantDoc()) {
>       window->GetExtantDoc()->WarnOnceAbout(aOperation);
>+
>+      nsAutoCString fileName;
>+      Nullable<uint32_t> lineNumber;
>+      Nullable<uint32_t> columnNumber;
>+      uint32_t line = 0;
>+      uint32_t column = 0;
>+      if (nsJSUtils::GetCallingLocation(aGlobal.Context(), fileName,
>+                                        &line, &column)) {
>+        lineNumber.SetValue(line);
>+        columnNumber.SetValue(column);
>+      }
>+
>+      MaybeReportDeprecation(window, aOperation,
>+                             NS_ConvertUTF8toUTF16(fileName), lineNumber,
>+                             columnNumber);
>     }
> 
>     return;
>   }
> 
>   WorkerPrivate* workerPrivate = GetWorkerPrivateFromContext(aGlobal.Context());
>   if (!workerPrivate) {
>     return;
>   }
> 
>+  nsAutoCString fileName;
>+  Nullable<uint32_t> lineNumber;
>+  Nullable<uint32_t> columnNumber;
>+  uint32_t line = 0;
>+  uint32_t column = 0;
>+  if (nsJSUtils::GetCallingLocation(aGlobal.Context(), fileName,
>+                                    &line, &column)) {
>+    lineNumber.SetValue(line);
>+    columnNumber.SetValue(column);
>+  }
>+
this code duplication is rather annoying, but I don't see easy way to avoid it. Though, it is unclear to me why we do worker reporting the way the patch does it.



r- because I don't understand why worker reports are sent to the main thread.
Should ReportingObserver  be exposed to workers too perhaps?
Attachment #9021792 - Flags: review?(bugs) → review-

Updated

6 months ago
Attachment #9021879 - Flags: review?(bugs) → review+
(Assignee)

Comment 46

6 months ago
Attachment #9021767 - Attachment is obsolete: true
Attachment #9023228 - Flags: review?(bugs)
(Assignee)

Comment 47

6 months ago
I filed https://github.com/w3c/reporting/issues/131 to expose ReportingObserver to workers. In the meantime, let's ignore deprecate reports for workers.
Attachment #9021792 - Attachment is obsolete: true
Attachment #9023235 - Flags: review?(bugs)

Updated

5 months ago
Attachment #9023228 - Flags: review?(bugs) → review+

Updated

5 months ago
Attachment #9023235 - Flags: review?(bugs) → review+

Comment 48

5 months ago
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e8d5d9f9ecc8
Reporting API - part 1 - WebIDL, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/a599388cbfe7
Reporting API - part 2 - WebIDL for DeprecationReportBody, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/a86e9cde89bb
Reporting API - part 3 - Deprecate reports, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/c200472121fd
Reporting API - part 4 - Reporting, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/3e311bf1af40
Reporting API - part 5 - tests, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed3b5f933b97
Reporting API - part 6 - FeaturePolicy, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/6119216d9038
Reporting API - part 7 - WPTs, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/2751ee1bdac6
Reporting API - part 8 - memory-pressure, r=smaug
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/14065 for changes under testing/web-platform/tests
Whiteboard: [domsecurity-backlog1] → [domsecurity-backlog1][wptsync upstream]
Can't merge web-platform-tests PR due to failing upstream checks:
Github PR https://github.com/web-platform-tests/wpt/pull/14065
* continuous-integration/travis-ci/pr (https://travis-ci.org/web-platform-tests/wpt/builds/455264711?utm_source=github_status&utm_medium=notification)
Whiteboard: [domsecurity-backlog1][wptsync upstream] → [domsecurity-backlog1][wptsync upstream error]
Whiteboard: [domsecurity-backlog1][wptsync upstream error] → [domsecurity-backlog1][wptsync upstream]
Upstream PR merged
Note to MDN writer's team:

This API is not enabled in release yet, so I'm not adding a note to the Fx65 rel notes. The API should be documented, however, and an entry should be added to our experimental features page.
You need to log in before you can comment on or make changes to this bug.