1368327 - Assertion failure: aDocShell, at dom/base/Location.cpp:57

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[Bob Clary [:bc] (inactive)]

1. http://www.yad2.co.il/Nadlan/ViewImage.php?CatID=2&SubCatID=5&RecordID=1625606

2. Assertion failure: aDocShell, at /mozilla/builds/nightly/mozilla/dom/base/Location.cpp:57

Assertion failure: aDocShell, at /home/worker/workspace/build/src/dom/base/Location.cpp:57
#01: nsGlobalWindow::GetLocation [mfbt/RefPtr.h:53]
#02: mozilla::dom::WindowBinding::get_location [obj-firefox/dom/bindings/WindowBinding.cpp:1352]
#03: mozilla::dom::WindowBinding::genericCrossOriginGetter [obj-firefox/dom/bindings/WindowBinding.cpp:15834]

Comment 1

[Tyson Smith [:tsmith]]

We are seeing this a lot while fuzzing

Comment 2

[Samael Wang [:freesamael] (away for now)]

That would indicate the outer nsGlobalWindow::mDocShell was nullptr, which should imply it was after nsDocShell::Destroy(). So... script can still be run after docshell destroyed?

Comment 3

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

Sure. Just have an iframe, take reference to iframe.contentWindow.location in the parent window, then remove iframe and try to use location.
Or take reference to iframe.contentWindow, remove iframe, and then try to access contentWindow.location.
Looks like a wrong assertion, but need to check that Location can handle null docshell.

Comment 4

[Samael Wang [:freesamael] (away for now)]

Attachment: Bug 1368327 - Do not assert aDocShell in Location, since it's actually possible to be nullptr.

Review commit: https://reviewboard.mozilla.org/r/173162/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/173162/

Comment 5

[Samael Wang [:freesamael] (away for now)]

(In reply to Olli Pettay [:smaug] from comment #3)
> need to check that Location can handle null docshell.

Location is using a nsWeakPtr to hold docshell, and I checked that all references to it has nullptr check on docshell and/or the queryInterface'd webNav:

http://searchfox.org/mozilla-central/search?q=symbol:F_%3CT_mozilla%3A%3Adom%3A%3ALocation%3E_4&redirect=false

Comment 6

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

Comment on attachment 8901731 [details]
Bug 1368327 - Do not assert aDocShell in Location, since it's actually possible to be nullptr.

https://reviewboard.mozilla.org/r/173162/#review178506

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/294dae9f3f01
Do not assert aDocShell in Location, since it's actually possible to be nullptr. r=smaug

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/294dae9f3f01

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Doubtful there's a user impact here that justifies backport consideration. Feel free to set the status for 56 back to affected and nominate for approval if you feel otherwise, though.