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

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect, P2)

Description

[Randell Jesup [:jesup] (needinfo me)]

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

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Attachment: docshell_problem

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

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.

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

Attachment: assertion2

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

Comment 4

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

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.

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

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

Comment 6

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

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

Comment 7

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1542918 - Disable the assertion in profiler page registration until we fix it r?jesup

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/326391a0e32a
Disable the assertion in profiler page registration until we fix it r=jesup

Comment 9

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/326391a0e32a