With the bug 1323671 patches, we can end up resolving a style context that is destroyed by the end of Element::GetBindingURL, thereby destroying the URLValue that we return a weak reference to. We should make this return a strong reference.

I don't really like the API that returns a RefPtr via a pointer out param, but looking at the function and its callsite, I cannot figure out any better way, so probably it's fine. Strictly speaking I'm not a DOM peer, so passed it to smaug.

Comment on attachment 8822142 [details] Bug 1326023 - Make Element::GetBindingURL return a strong reference. https://reviewboard.mozilla.org/r/101140/#review101646 ::: dom/base/Element.h:390 (Diff revision 1) > if (aNotify) { > UpdateState(true); > } > } > > - bool GetBindingURL(nsIDocument *aDocument, css::URLValue **aResult); > + bool GetBindingURL(nsIDocument* aDocument, RefPtr<css::URLValue>* aResult); This is weird. Why not just use the normal style where out params are always addrefed, and then callers use getter_AddRefs. Element::GetBindingURL needs to of course AddRef aResult when it points to non-null. No need to use any unusual style here.

Comment on attachment 8822142 [details] Bug 1326023 - Make Element::GetBindingURL return a strong reference. https://reviewboard.mozilla.org/r/101140/#review101798 Hopefully the addref/release don't show up badly in the performance profiles or microbenchmarks (for XUL).

Pushed by cmccormack@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bf998120b814 Make Element::GetBindingURL return a strong reference. r=smaug

Backed out for hazard failure: https://hg.mozilla.org/integration/autoland/rev/a3174cbb505663f5665c1fc79ef7c24e61a01ef5 Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=bf998120b81440e4d3809d2b5332e89e0831f30b Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=8492465&repo=autoland [task 2016-12-29T12:13:32.934364Z] Found 1 hazards and 92 unsafe references [task 2016-12-29T12:13:32.938581Z] + check_hazards /home/worker/workspace/analysis [task 2016-12-29T12:13:32.938823Z] + set +e [task 2016-12-29T12:13:32.939175Z] ++ grep -c 'Function.*has unrooted.*live across GC call' /home/worker/workspace/analysis/rootingHazards.txt [task 2016-12-29T12:13:32.940576Z] + NUM_HAZARDS=1 [task 2016-12-29T12:13:32.940880Z] ++ grep -c '^Function.*takes unsafe address of unrooted' /home/worker/workspace/analysis/refs.txt [task 2016-12-29T12:13:32.942005Z] + NUM_UNSAFE=92 [task 2016-12-29T12:13:32.942258Z] ++ grep -c '^Function.* has unnecessary root' /home/worker/workspace/analysis/unnecessary.txt [task 2016-12-29T12:13:32.944807Z] + NUM_UNNECESSARY=1170 [task 2016-12-29T12:13:32.944839Z] + set +x [task 2016-12-29T12:13:32.944859Z] TinderboxPrint: rooting hazards<br/>1 [task 2016-12-29T12:13:32.944883Z] TinderboxPrint: unsafe references to unrooted GC pointers<br/>92 [task 2016-12-29T12:13:32.944907Z] TinderboxPrint: unnecessary roots<br/>1170 [task 2016-12-29T12:13:32.944927Z] TEST-UNEXPECTED-FAIL 1 hazards detected

Shuffled things around a bit to avoid the hazard: https://treeherder.mozilla.org/#/jobs?repo=try&revision=172b88c68394ce738c3aa18d3b576c2247fb93f3

Pushed by cmccormack@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/04dff9adc703 Make Element::GetBindingURL return a strong reference. r=smaug