Closed Bug 1317990 Opened 3 years ago Closed 3 years ago
We should use CallerType instead.
Ben, could you look at the serviceworker bits?
Comment on attachment 8811300 [details] [diff] [review] Remove Event::IsChrome in favor of passing CallerType arguments Review of attachment 8811300 [details] [diff] [review]: ----------------------------------------------------------------- r=me with comments addressed. ::: dom/workers/ServiceWorkerEvents.cpp @@ +772,5 @@ > mPromises.AppendElement(&aArg); > } > > void > +FetchEvent::PreventDefault(JSContext* aCx, CallerType aCallerType) You could actually assert this is not a system caller. We explicitly don't support system principal service workers. But I guess probably not necessary. ::: dom/workers/ServiceWorkerEvents.h @@ +6,5 @@ > > #ifndef mozilla_dom_workers_serviceworkerevents_h__ > #define mozilla_dom_workers_serviceworkerevents_h__ > > +#include "mozilla/dom/BindingDeclarations.h" nit: This header should not be needed here. The signature is defined in the parent class. ::: dom/workers/ServiceWorkerPrivate.cpp @@ +1542,5 @@ > + // one? The former lies about cases when system code (e.g. default > + // handling) does the preventDefault and claims default was _not_ > + // prevented... Keep calling the "scriptable" one for now. > + CallerType callerType = aWorkerPrivate->UsesSystemPrincipal() ? > + CallerType::System : CallerType::NonSystem; Please just MOZ_ASSERT(!aWorkerPrivate->UsesSystemPrincipal()) and pass CallerType::NonSystem. We don't allow system principal service workers and have no plans to add them. I think this means the XXX comment can go as well?
Attachment #8811300 - Flags: review?(bkelly) → review+
> I think this means the XXX comment can go as well? Yes. Applied the other review comments.
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/482ed3f75e5b Remove Event::IsChrome in favor of passing CallerType arguments. r=smaug,bkelly
You need to log in before you can comment on or make changes to this bug.