Closed Bug 1590748 Opened 3 years ago Closed 3 years ago

crash [@ mozilla::dom::ClientHandle::Control ] hitting MOZ_RELEASE_ASSERT(ClientMatchPrincipalInfo(mClientInfo.PrincipalInfo(), aServiceWorker.PrincipalInfo()))

Categories

(Core :: DOM: Service Workers, defect)

defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox-esr68 --- unaffected
firefox67 --- unaffected
firefox68 --- unaffected
firefox69 --- unaffected
firefox70 --- unaffected
firefox71 --- unaffected
firefox72 blocking fixed

People

(Reporter: dbaron, Assigned: mattwoodrow, NeedInfo)

References

(Regression)

Details

(Keywords: crash, regression, topcrash)

Crash Data

Attachments

(1 file)

There are a bunch of crashes starting in the 20191023094816 nightly (so this is the regression window) with the signature mozilla::dom::ClientHandle::Control. One of them (bp-0faa8c5a-5f80-46fa-8837-bfee20191023) was mine, possibly the result of clicking a link to https://www.cnbc.com/2019/10/16/reuters-america-u-s-safety-agency-to-jumpstart-revamp-of-new-car-rating-program.html from a Slack instance (though it didn't crash the second time after I restarted, so it could have just been something happening in the background).

All the crashes are hitting MOZ_RELEASE_ASSERT(ClientMatchPrincipalInfo(mClientInfo.PrincipalInfo(), aServiceWorker.PrincipalInfo())).

Skimming the list of commits, bug 1583700 is the one that jumps out as a possible cause, although I'm far from certain.

[Tracking Requested - why for this release]: The number of crashes in nightly so far pretty clearly makes this a top crash.

Flags: needinfo?(nika)
Flags: needinfo?(matt.woodrow)
Flags: needinfo?(bugmail)
Crash Signature: [@ mozilla::dom::ClientHandle::Control ]
Crash Signature: [@ mozilla::dom::ClientHandle::Control ] → [@ mozilla::dom::ClientHandle::Control]

I can consistently reproduce this crash on macOS when clicking a link to https://www.theatlantic.com/technology/archive/2019/10/dont-play-the-goose-game/600472/ from a Slack window.

I cannot reproduce the crash when visiting that link from a non-Slack window.

(In reply to Josh Matthews [:jdm] from comment #3)

I can consistently reproduce this crash on macOS when clicking a link to https://www.theatlantic.com/technology/archive/2019/10/dont-play-the-goose-game/600472/ from a Slack window.

STR works for me as well, but requires browser.tabs.documentchannel=true.

https://hg.mozilla.org/mozilla-central/rev/4946569ae95bbf189dea8bc8dfff8d839b496f4b is another possible cause. Eden, could that patch make a registration start controlling a cross-origin client?

Flags: needinfo?(echuang)

(In reply to Perry Jiang [:perry] from comment #5)

STR works for me as well, but requires browser.tabs.documentchannel=true.

That's the default on nightly as of d5768c2f8124 (the bug I cited).

Assignee: nobody → matt.woodrow
Flags: needinfo?(matt.woodrow)

Julien, FYI, this is a new 72 top crash.

Flags: needinfo?(jcristau)
Severity: normal → major
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ca11dd7809bb
Setup result principal URI on the content process LoadInfo as well as the parent process one, since ServiceWorkers depends on it there. r=nika
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72

(In reply to Perry Jiang [:perry] from comment #6)

https://hg.mozilla.org/mozilla-central/rev/4946569ae95bbf189dea8bc8dfff8d839b496f4b is another possible cause. Eden, could that patch make a registration start controlling a cross-origin client?

I think it should not be, but I assumed that all the needed information of LoadInfo should be passed/propagated to the child process to create a channel. However, according to patch, the assumption seems not to be true.

Flags: needinfo?(echuang)
Flags: needinfo?(jcristau)
Flags: needinfo?(nika)

In therms of manual testing, anything we could verify for this issue?

Flags: qe-verify+

Please specify a root cause for this bug. See :tmaity for more information.

Root Cause: --- → ?
Root Cause: ? → Coding: Logical Error

This might also be caused by similar architecture problems as bug 1535699. Matt, Andrew, maybe you want to talk shortly about the root cause we want to prevail here?

Flags: needinfo?(matt.woodrow)
Flags: needinfo?(matt.woodrow)
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.