Closed Bug 1587013 Opened 4 months ago Closed 2 months ago

Enable test_navigator_buildID.html in Fission

Categories

(Core :: DOM: Core & HTML, task)

task
Not set

Tracking

()

RESOLVED FIXED
mozilla72
Fission Milestone M4.1
Tracking Status
firefox72 --- fixed

People

(Reporter: hsivonen, Assigned: hsivonen)

Details

Attachments

(1 file, 1 obsolete file)

test_navigator_buildID.html is presently skipped.

The patch makes the property query proceed in Fission, but the property is evaluated without the pref setting taking effect in the content process. Is this a known bug (and bug this should block of the pref stuff getting fixed) or is the pref setting mechanism being used wrong? (I'd expect the IPC message to set the pref to be sent first, the IPC message to spawn the IPC evaluation to be sent later and the pref environment popping to happen even later.)

Flags: needinfo?(kmaglione+bmo)

The pref set operation doesn't work right with the patch with or without Fission.

So, apparently the problem is that navigator.buildID is cached, so the second time we access it, it returns whatever its value was the first time we accessed it. Somehow, accessing it through the sandbox manages to avoid the caching.

But I think that, given there's clearly a behavior difference when accessing it through a Sandbox, the test should avoid doing that, and should do something like reloading the iframe instead. This seems to work: https://gist.github.com/5660ff71f40bd5bc4e8f442a2ee903bb

Flags: needinfo?(kmaglione+bmo)

What's the next step here? Looks :kmag has a working test case. Do you plan to help land the test case to close this issue, after you are back, :kmag?

Flags: needinfo?(kmaglione+bmo)
Flags: needinfo?(hsivonen)
Flags: needinfo?(kmaglione+bmo)

Henri will upload a patch based on comment 4, thanks!

Tentatively moving all bugs whose summaries mention "Fission" (or other Fission-related keywords) but are not assigned to a Fission Milestone to the "?" triage milestone.

This will generate a lot of bugmail, so you can filter your bugmail for the following UUID and delete them en masse:

0ee3c76a-bc79-4eb2-8d12-05dc0b68e732

Fission Milestone: --- → ?
Fission Milestone: ? → M4.1
Attachment #9099488 - Attachment is obsolete: true
Flags: needinfo?(hsivonen)
Flags: needinfo?(hsivonen)

(In reply to Kris Maglione [:kmag] from comment #4)

So, apparently the problem is that navigator.buildID is cached, so the second time we access it, it returns whatever its value was the first time we accessed it. Somehow, accessing it through the sandbox manages to avoid the caching.

But I think that, given there's clearly a behavior difference when accessing it through a Sandbox, the test should avoid doing that, and should do something like reloading the iframe instead. This seems to work: https://gist.github.com/5660ff71f40bd5bc4e8f442a2ee903bb

Thank you. Are you OK with how the patch authorship is recorded at https://hg.mozilla.org/try/rev/d3262c81f99c72c301843d87dda5566110a6b6ec ?

Flags: needinfo?(hsivonen) → needinfo?(kmaglione+bmo)
Flags: needinfo?(kmaglione+bmo)

kmag said this was answered over IRC yesterday.

Pushed by hsivonen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2a3ba5508c7f
Enable test_navigator_buildID.html in Fission. r=smaug
Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
Assignee: nobody → hsivonen
You need to log in before you can comment on or make changes to this bug.