662186 - "###!!! ABORT: Expecting to be paused for pagehide before disconnect: 'mPauseState & nsSMILTimeContainer::PAUSE_PAGEHIDE', file content/smil/nsSMILAnimationController.cpp"...)

Status: RESOLVED WORKSFORME

(Core :: SVG, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

The abort happens at least when using the patch for strong parent nodes.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

The strong parent node is bug 335998

Comment 2

[Daniel Holbert [:dholbert]]

(This ABORT_IF_FALSE was added in bug 654015; marking dependency)

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

layout/reftests/svg/as-image/limeInRed-noSVGDimensions-animViewBox.svg
calls the Disconnect() where the abort happens.

Comment 4

[Daniel Holbert [:dholbert]]

smaug initially asked in IRC if the assertion was bogus, given this comment in 
nsDocument::GetAnimationController():
> 5515   // [...](Skip this check for SVG-as-an-image documents, though,
> 5516   // because they don't get OnPageShow / OnPageHide calls).
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDocument.cpp#5514

That comment is actually slightly misleading -- while it's true that SVG-as-an-image don't get the same OnPageShow/OnPageHide calls that other documents would get, we *do* actually fire one OnPageHide notification at image tear-down time, here:
>  99 SVGDocumentWrapper::DestroyViewer()
> 100 {
> 101   if (mViewer) {
> 102     mViewer->GetDocument()->OnPageHide(PR_FALSE, nsnull);
> 103     mViewer->Close(nsnull);
> 104     mViewer->Destroy();
http://mxr.mozilla.org/mozilla-central/source/modules/libpr0n/src/SVGDocumentWrapper.cpp#99

So I *think* this bug's ABORT_IF_FALSE should still hold for SVG-as-an-image documents...

I haven't been able to reproduce this, FWIW, with the latest patch (WIP v4) from bug 335998 applied.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Aha, the failing document is used as a background image
http://mxr.mozilla.org/mozilla-central/source/layout/reftests/svg/as-image/background-resize-4.html?force=1#17

Comment 6

[Daniel Holbert [:dholbert]]

Yup, that's the testcase I was using.  I loaded & reloaded &  shift-reloaded it repeatedly before comment 4, and I wasn't able to reproduce.  (though I did hit some "healthy" Disconnect() calls that satisfied the ABORT_IF_FALSE condition)

Can you reproduce from loading / reloading / shift-reloading background-resize-4?

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

I think I found the problem in my patch. I had wrong assumption that
I could unlink animationcontroller. I do need to do that, but apparently
I need to be more careful how.

I should still understand why pagehide isn't called before unlinking.

Comment 8

[Daniel Veditz [:dveditz]]

Is this then a security bug, or just a problem in your patch? Can you give it a security rating if the former?

Assuming this "blocks" bug 335998, one way or another you need to deal with it.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

I think this is just a bug in my patch, and I've actually fixed it.
Sorry for the noise.