2036774 - MOZ_RELEASE_ASSERT(IsTop()); in GetDownloadFolderOverride when enumerating properties of a child BrowsingContext

Status: RESOLVED FIXED

(Remote Protocol :: WebDriver BiDi, defect, P3)

Description

[Rob Wu [:robwu]]

When I tried to examine properties of a child BrowsingContext in the Browser Console, the whole browser crashed before I hit enter. This appears to be caused by eager evaluation while typing gBrowser.selectedBrowser.browsingContext.children[0], a self-contained test case is included in the STR below.

STR:

1. Open a page with iframes: https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/iframe
2. Open the Browser Console and run:
   bc=gBrowser.selectedBrowser.browsingContext.children[0];for(let k in bc)void bc[k]

Expected:

• No crash, maybe a runtime error at most.

Actual:

• Crash, with stack:

  1. mozilla::dom::CanonicalBrowsingContext::GetDownloadFolderOverride
  2. mozilla::dom::CanonicalBrowsingContext_Binding::get_downloadFolderOverride

This is due to the MOZ_RELEASE_ASSERT(IsTop()) at https://searchfox.org/firefox-main/rev/a353242aeafd56b2c21a2c0672ecb51ea1b81142/docshell/base/CanonicalBrowsingContext.h#395-396

I see that the code may be inspired by similar code above those lines, but the difference between the previously existing code and the new one is that IsPriorityActive is C++ only, whereas the new GetDownloadFolderOverride getter is also exposed to JS. We shouldn't crash here.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 2017252

:Sasha, since you are the author of the regressor, bug 2017252, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 2

[Alexandra Borovova [:Sasha]]

Attachment: Bug 2036774 - Do not assert for top browsing context to avoid causing the crash.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/4776e412dc9a
https://hg.mozilla.org/integration/autoland/rev/46479b6e565f
Do not assert for top browsing context to avoid causing the crash. r=dom-core,smaug

Comment 4

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/46479b6e565f