Debug Assertion on page already registered in the profiler browsing to youtube.com

RESOLVED FIXED in Firefox 68

Status

()

defect
P2
normal
RESOLVED FIXED
3 months ago
Last month

People

(Reporter: jesup, Assigned: canaltinova)

Tracking

(Depends on 1 bug, Regression, {assertion})

52 Branch
mozilla68
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox67 wontfix, firefox68 fixed)

Details

Attachments

(3 attachments)

STR: with a fairly new profile with a Debug non-opt build, browse to a site (I used mozilla.github.io), then start the profiler (already installed) and browse to youtube.com.

Hit an assertion at tools/profiler/core/platform.cpp:323 due to a page already being in the page cache. Dumps of the 3 pages in the cache and the new page are attached -- the collision was on HistoryId 13, which one has as about:blank, and the other has as "https://accounts.google.com/ServiceLogin?service=youtube&continue=https%3A%2F%2Fwww.youtube.com%2Fsignin%3Ffeature%3Dpassive%26hl%3Den%26action_handle_signin%3Dtrue%26next%3D%252Fsignin_passive%26app"... (truncated).

They compare as equal in PageInformation::Equals() (same docshell, id, and subframe).

Unsure if this is repeatable. Note: browsing with a non-opt debug build is Very Slow....

I presume this was introduced by bug 1417976

Posted file docshell_problem
Flags: needinfo?(canaltinova)

If you remove the ifdef, and make it MOZ_RELEASE_ASSERT(), it failed on the the first pageload of youtube for me (after loading https://mozilla.github.io/webrtc-landing). Trying again and 1 or two other URLs didn't hit, but clearly there's a hole here.

Posted file assertion2

Assertion with opt build (modified to include the assert) on a CNN page.

Thanks for filing this Randell. I'm aware of that issue and actually I have a fix for that here: https://phabricator.services.mozilla.com/D18331.
But I wasn't working on that for a while after publishing the patch because of other works that I need to finish. I came back to it and finishing up/writing some tests now. It will be fixed after that.

Depends on: 1512500
Flags: needinfo?(canaltinova)
Priority: -- → P2

Nazim, we need to disable this, or fix it - it makes debug builds die with the profiler way too often. Currently I build with patches to make that code an #if 0 to avoid crashing

Flags: needinfo?(canaltinova)

Ok, how about disabling it until we fix it? Will submit a patch that disables that for now. Sorry about that.

Flags: needinfo?(canaltinova)
Pushed by canaltinova@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/326391a0e32a
Disable the assertion in profiler page registration until we fix it r=jesup
Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Assignee: nobody → canaltinova
You need to log in before you can comment on or make changes to this bug.