Closed Bug 1630323 Opened 2 years ago Closed 2 years ago

Do not override user preferences when clicking on a service worker notification to open a new document

Categories

(Core :: DOM: Service Workers, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla78
Tracking Status
firefox-esr68 --- unaffected
firefox76 --- wontfix
firefox77 --- wontfix
firefox78 --- fixed

People

(Reporter: annyG, Assigned: annyG)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file, 2 obsolete files)

No description provided.
Assignee: nobody → agakhokidze
Status: NEW → ASSIGNED
Regressed by: 1622749
Has Regression Range: --- → yes
Priority: -- → P2
Attached file Bug 1630323 - WIP (obsolete) —
Attachment #9141055 - Attachment is obsolete: true
Attachment #9143784 - Attachment description: Bug 1630323 - Do not override user preferences when clicking on a service worker notification to open a new document → Bug 1630323 - Do not override user preferences when clicking on a service worker notification to open a new document, r?Gijs!

NeedInfo'ing Andrew on this as I need help answering one of the review comments.

Some backstory:
The need for this bug stemmed out of my hacky solution for crash happening in bug 1622749 (the crash was happening in code landed in bug 1578070).

I have a patch above with hopefully a complete description of how the crash came to be and how I’m trying to fix the regression properly instead of the hacky solution.

I was wondering you could help me answer Gij’s questions in the one comment linked here https://phabricator.services.mozilla.com/D72745#inline-427631 ? Is it a problem that WaitForLoad will be invoked via event loop now? If not, I guess I could dispatch a runnable (calling WaitForLoad) to the proper thread in the resolve case of the promise somehow… I’m also not sure about the second question - is it possible that the load succeeds before WaitForLoad?

Flags: needinfo?(bugmail)

Responded on the review, with the caveat that I would likely consider :nika and :smaug as likely to have more authoritative guidance about the load edge cases.

Flags: needinfo?(bugmail)
Attachment #9143784 - Attachment description: Bug 1630323 - Do not override user preferences when clicking on a service worker notification to open a new document, r?Gijs! → Bug 1630323 - Do not override user preferences when clicking on a service worker notification to open a new document, r?Gijs!,nika!
Attachment #9146666 - Attachment is obsolete: true
Attachment #9143784 - Attachment description: Bug 1630323 - Do not override user preferences when clicking on a service worker notification to open a new document, r?Gijs!,nika! → Bug 1630323 - Do not override user preferences when clicking on a service worker notification to open a new document, r?Gijs!,nika!,snorp!
Pushed by agakhokidze@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f0de352a865b
Do not override user preferences when clicking on a service worker notification to open a new document, r=Gijs,nika,geckoview-reviewers,snorp
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla78

I have seen crash reports like this:
https://crash-stats.mozilla.org/report/index/ccb26a4d-fb18-46ac-b5cf-e72e60210214

Frame Module Signature Source Trust
0 libpthread.so.0 __pthread_mutex_lock /build/glibc-vjB4T1/glibc-2.28/nptl/../nptl/pthread_mutex_lock.c:67 context
1 firefox-esr mozilla::detail::MutexImpl::lock() build-browser/mozglue/misc/mozglue/misc/Mutex_posix.cpp:118 cfi
2 libxul.so nsBrowsingContextReadyCallback::BrowsingContextReady(mozilla::dom::BrowsingContext*) cfi
3 libxul.so nsFrameLoader::InvokeBrowsingContextReadyCallback() build-browser/dom/base/dom/base/nsFrameLoader.cpp:3578 cfi
4 libxul.so nsFrameLoader::TryRemoteBrowserInternal() cfi
5 libxul.so nsFrameLoader::TryRemoteBrowser() build-browser/dom/base/dom/base/nsFrameLoader.cpp:2657 cfi
6 libxul.so nsFrameLoader::GetBrowsingContext() build-browser/dom/base/dom/base/nsFrameLoader.cpp:3195 cfi
7 libxul.so mozilla::dom::FrameLoader_Binding::get_browsingContext(JSContext*, JS::Handle<JSObject*>, void*, JSJitGetterCallArgs) build-browser/dom/bindings/build-browser/dom/bindings/FrameLoaderBinding.cpp:163 cfi
8 libxul.so bool mozilla::dom::binding_detail::GenericGetter<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) build-browser/dom/bindings/dom/bindings/BindingUtils.cpp:3079 cfi
9 libxul.so js::jit::CallNativeGetter(JSContext*, JS::Handle<JSFunction*>, JS::Handle<JSObject*>, JS::MutableHandle<JS::Value>) build-browser/js/src/jit/js/src/jit/VMFunctions.cpp:1478 cfi

Depends on: 1693946
You need to log in before you can comment on or make changes to this bug.