1495554 - Call StopObserving in SVGRenderingObserver's dtor instead of individual subclasses' dtors

Status: RESOLVED INVALID

(Core :: SVG, enhancement, P3)

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: Bug 1495554. Call StopObserving in SVGRenderingObserver's dtor instead of individual subclasses' dtors. r?longsonr

Comment 2

[Robert Longson [:longsonr]]

Comment on attachment 9013406 [details]
Bug 1495554. Call StopObserving in SVGRenderingObserver's dtor instead of individual subclasses' dtors. r?longsonr

Robert Longson [:longsonr] has approved the revision.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/28792d9adea2
Call StopObserving in SVGRenderingObserver's dtor instead of individual subclasses' dtors. r=longsonr

Comment 4

[Narcis Beleuzu [:NarcisB]]

Backed out for multiple failures like:

1. Crashes on [@ CrashReporter::TerminateHandler()]
Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=202747489&repo=mozilla-inbound&lineNumber=883

2. wpt failures : https://treeherder.mozilla.org/logviewer.html#?job_id=202746623&repo=mozilla-inbound&lineNumber=1025

3. bc failures : https://treeherder.mozilla.org/logviewer.html#?job_id=202746618&repo=mozilla-inbound&lineNumber=1441

4. mochitest crashes on [@ 0x0] : https://treeherder.mozilla.org/logviewer.html#?job_id=202744535&repo=mozilla-inbound&lineNumber=1591

Comment 5

[Jonathan Watt [:jwatt]]

Oops! StopObserving() itself is not virtual, but it calls the virtual function GetReferencedElementWithoutObserving and of course calling virtual methods during destruction is hazardous.

Specifically what happens here I believe is that when the SVGRenderingObserver dtor is called the vtable in use is the vtable for SVGRenderingObserver, not the vtable for the original, fully derived subclass type of the object being destroyed. That vtable has no entry for GetReferencedElementWithoutObserving (which is pure virtual at that level in the type hierarchy). I'm not sure what behavior the spec defines at that point, but crashing immediately in SVGRenderingObserver::StopObserving at the GetReferencedElementWithoutObserving call does indeed seem like the best course of action.