Closed Bug 2003244 Opened 11 months ago Closed 10 months ago

Crash in [@ IPCError-content | RecvConstructBrowser BrowserChild::Init failed]

Categories

(Core :: DOM: Navigation, defect)

Product:
Core
Component:
DOM: Navigation
Platform:
Unspecified
Windows 11
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
148 Branch
Tracking Flags:
Tracking Status
firefox-esr115 --- unaffected
firefox-esr140 --- unaffected
firefox145 --- unaffected
firefox146 --- unaffected
firefox147 + disabled
firefox148 + fixed

People

(Reporter: gsvelto, Assigned: vhilla)

References

(Regression)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(2 external files, 2 obsolete files)

Crash report: https://crash-stats.mozilla.org/report/index/b2ef1937-361d-4802-a0b7-bd3ca0251130

MOZ_CRASH Reason:

MOZ_CRASH(Content child abort due to IPC error)

Top 10 frames:

0  xul.dll  mozilla::dom::ContentChild::ProcessingError(mozilla::ipc::HasResultCodes::Res...  dom/ipc/ContentChild.cpp:2314
1  xul.dll  mozilla::ipc::IPCResult::FailImpl(mozilla::NotNull<mozilla::ipc::IProtocol*>,...  ipc/glue/ProtocolUtils.cpp:61
2  xul.dll  mozilla::ipc::IPCResult::Fail(mozilla::NotNull<mozilla::ipc::IProtocol*>, con...  ipc/glue/ProtocolUtils.h:375
2  xul.dll  mozilla::dom::ContentChild::RecvConstructBrowser(mozilla::ipc::ManagedEndpoin...  dom/ipc/ContentChild.cpp:1966
2  xul.dll  mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&)  ipc/ipdl/PContentChild.cpp:8681
3  xul.dll  mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecyc...  ipc/glue/MessageChannel.cpp:1793
3  xul.dll  mozilla::ipc::MessageChannel::DispatchMessage(mozilla::ipc::ActorLifecyclePro...  ipc/glue/MessageChannel.cpp:1719
3  xul.dll  mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::ActorLifecycleProxy*, ...  ipc/glue/MessageChannel.cpp:1508
3  xul.dll  mozilla::ipc::MessageChannel::MessageTask::Run()  ipc/glue/MessageChannel.cpp:1610
4  xul.dll  mozilla::RunnableTask::Run()  xpcom/threads/TaskController.cpp:705

The first buildid aligns with the landing of bug 543435, Henri could this be a regression?

Flags: needinfo?(hsivonen)

This looks a lot like a regression from bug 543435. I don't access to extra info that users may have added to crash reports. Is there extra info from users that could point to steps to reproduce?

Flags: needinfo?(hsivonen) → needinfo?(gsvelto)

Marking as regressed by bug 543435 due to the high probability that that's the case.

Keywords: regression
Regressed by: sync-about-blank

There are no comments in the crash reports yet, and the crashing URLs are all over the place, there doesn't seem to be any pattern in there. The only thing that we can tell is that this call failed. Maybe we should gather more detailed info on the failure and stuff it in a crash annotation?

Flags: needinfo?(gsvelto)

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

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

For more information, please visit BugBot documentation.

status-firefox145: --- → unaffected
status-firefox146: --- → unaffected
status-firefox147: --- → affected
status-firefox-esr140: --- → unaffected
Flags: needinfo?(vhilla)

Unless vhilla has better ideas, my first suggestion forward is that we change bool nsDocShell::Initialize to nsresult nsDocShell::Initialize and try to see which error code we fail with. Notably, we have an identifiable error code at https://searchfox.org/firefox-main/rev/7032b912c282da6b9b8219e779b2a441a27aaec9/docshell/base/nsDocShell.cpp#6767 vs. a more generic ones earlier in that method.

padenot suggested surfacing the nsresult value using MOZ_CRASH_UNSAFE_FMT. (The comment for that macro says it needs a data steward review, which hopefully shouldn't be a big deal for an exposed nsresult.)

Do you happen to recall why we do an IPC_FAIL instead of propagating the failure to the parent process?

Flags: needinfo?(nika)

:hsivonen can you explain what you mean by nsDocShell::Initialize I'm trying to find where it's being called, but I can't find it, I only see BrowserChild::Init being the culprit that ultimately makes the IPC_FAIL happen?

Flags: needinfo?(hsivonen)

(In reply to Simon Farre [:sfarre] from comment #8)

:hsivonen can you explain what you mean by nsDocShell::Initialize I'm trying to find where it's being called, but I can't find it, I only see BrowserChild::Init being the culprit that ultimately makes the IPC_FAIL happen?

Its a bit further in the call stack. RecvConstructBrowser -> browserChild->Init -> nsWebBrowser::Create -> docShell->InitWindow -> nsDocShell::Initialize.

Flags: needinfo?(hsivonen)
Flags: needinfo?(vhilla)

(In reply to Henri Sivonen (:hsivonen) from comment #7)

Do you happen to recall why we do an IPC_FAIL instead of propagating the failure to the parent process?

I'm guessing this is because we considered this an unrecoverable error. We should never be failing to initialize a new BrowserChild unless something has gone very wrong, and we don't really have great ways to recover from this failure in the parent process right now, other than crashing the content process and showing the tab crashed message.

I'm assuming this is fall-out from the initial about:blank work, which means the most likely failure stack is (with greatly increasing uncertainty as we get deeper):

I'm unsure what past that point is causing a problem, unfortunately.

Flags: needinfo?(nika)
Severity: -- → S2

(In reply to Vincent Hilla [:vhilla] from comment #9)

(In reply to Simon Farre [:sfarre] from comment #8)

:hsivonen can you explain what you mean by nsDocShell::Initialize I'm trying to find where it's being called, but I can't find it, I only see BrowserChild::Init being the culprit that ultimately makes the IPC_FAIL happen?

Its a bit further in the call stack. RecvConstructBrowser -> browserChild->Init -> nsWebBrowser::Create -> docShell->InitWindow -> nsDocShell::Initialize.

And since there a couple of layers of nullptrs there, propagating nsresult out of nsDocShell::Initialize isn't going to be sufficient. Sorry about suggesting that alone.

(In reply to Nika Layzell [:nika] (ni? for response) from comment #10)

(In reply to Henri Sivonen (:hsivonen) from comment #7)

Do you happen to recall why we do an IPC_FAIL instead of propagating the failure to the parent process?

I'm guessing this is because we considered this an unrecoverable error. We should never be failing to initialize a new BrowserChild unless something has gone very wrong, and we don't really have great ways to recover from this failure in the parent process right now, other than crashing the content process and showing the tab crashed message.

Thanks.

I'm unsure what past that point is causing a problem, unfortunately.

Me, too. Even if we figured out which of https://searchfox.org/firefox-main/rev/5ccf4a7d77a329f237d3a41e400049f9c47dc71f/docshell/base/nsDocShell.cpp#6680,6733,6738,6767,6879 returned the failure, it's unclear to me what we'd do about that.

Steps to repro would help a lot with understanding what kind of remedy would be appropriate.

I wonder if it's safe to assume that this is happening during some stage of shut down (as it returns early in some cases for that)?

Can we find some way of potentially writing a test that is being spammy with creating about:blank and shutting them down, maybe?

Could we perhaps

  • add MOZ_CRASH_UNSAFE_FMT to nsWebBrowser::Create instead of propagating nsresult through several calls
  • change a few failures to be more unique, e.g. this to NS_ERROR_ABORT
  • Use nsXULAppInfo::AnnotateCrashReport along the failure stack

I think figuring out where the error occurs is still valuable.

(In reply to Vincent Hilla [:vhilla] from comment #13)

Could we perhaps

  • add MOZ_CRASH_UNSAFE_FMT to nsWebBrowser::Create instead of propagating nsresult through several calls

Let's do this only if aInitialWindowChild is not nullptr to avoid crashing early for the other caller.

  • change a few failures to be more unique, e.g. this to NS_ERROR_ABORT

Makes sense.

  • Use nsXULAppInfo::AnnotateCrashReport along the failure stack

If we do the above two items, what info did you have in mind for this one?

I think figuring out where the error occurs is still valuable.

Yes.

Going through the changes, this part sticks out to me:

nsWebBrowser::Create

-  nsresult rv = docShell->InitWindow(docShellParentWidget, 0, 0, 0, 0);
+  nsresult rv = docShell->InitWindow(docShellParentWidget, 0, 0, 0, 0,
+                                     aOpenWindowInfo, aInitialWindowChild);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return nullptr;
   }
 
-  if (aInitialWindowChild) {
-    docShell->CreateDocumentViewerForActor(aInitialWindowChild);
-  }
-

As InitWindow now creates the viewer, we started propagating errors from there. Failing to create the viewer wouldn't have crashed previously.

The crashes almost always seem to have a third-party password manager extension. (Often Bitwarden, but also Proton, iCloud, 1Password)

It probably makes sense to see what Bitwarden injects into about:blank. Bug 2003255 comment 2 is possibly relevant.

The bug is marked as tracked for firefox147 (nightly). We have limited time to fix this, the soft freeze is in 2 days. However, the bug still isn't assigned.

:hsinyi, could you please find an assignee for this tracked bug? Given that it is a regression and we know the cause, we could also simply backout the regressor. If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Flags: needinfo?(htsai)

(In reply to BugBot [:suhaib / :marco/ :calixte] from comment #18)

The bug is marked as tracked for firefox147 (nightly). We have limited time to fix this, the soft freeze is in 2 days. However, the bug still isn't assigned.

The plan for 147 is to back bug 543435 out of beta in bug 2003720.

Flags: needinfo?(htsai)

Let's add diagnostics with a distinct bug number: bug 2003759

::BrowserChild::Init () at BrowserChild.cpp:471
nsWebBrowser::Create () at nsWebBrowser.cpp:142
nsDocShell::InitWindow () at nsDocShell.cpp:449
nsDocShell::Initialize () at nsDocShell.cpp:478
nsDocShell::CreateInitialDocumentViewer () at nsDocShell.cpp:6628
nsDocShell::CreateAboutBlankDocumentViewer () at nsDocShell.cpp:6880
nsDocShell::Embed () at nsDocShell.cpp:5630
nsDocShell::SetupNewViewer () at nsDocShell.cpp:8193
nsDocumentViewer::Init () at nsDocumentViewer.cpp:661
nsDocumentViewer::InitInternal () at nsDocumentViewer.cpp:895
nsAutoScriptBlocker::~nsAutoScriptBlocker () at nsContentUtils.h:3847
nsContentUtils::RemoveScriptBlocker () at nsContentUtils.cpp:6900
::RunnableMethodImpl<>::Run () at nsThreadUtils.h:1132
::RunnableMethodArguments<>::apply<> () at nsThreadUtils.h:1081
std::apply<> () at tuple:1751
_ZSt12__apply_implIZN7mozilla6detail23RunnableMethodArgumentsIJEE5applyI19nsGlobalWindowOuterMS5_FvvEEEDcPT_T0_EUlDpOT_E_RSt5tupleIJEETpTnmJEEDcOS8_OSA_St16integer_sequenceImJXspT1_EEE () at tuple:1740
std::__invoke<> () at invoke.h:95
std::__invoke_impl<> () at invoke.h:60
::RunnableMethodArguments<>::apply<>(nsGlobalWindowOuter*, void (nsGlobalWindowOuter::*)())::{lambda(auto:1&&)#1}::operator()<> const () at nsThreadUtils.h:1083
nsGlobalWindowOuter::DispatchDOMWindowCreated () at nsGlobalWindowOuter.cpp:2618

... might be of interest.

(In reply to Henri Sivonen (:hsivonen) from comment #16)

The crashes almost always seem to have a third-party password manager extension. (Often Bitwarden, but also Proton, iCloud, 1Password)

:robwu can you confirm that document_start is the earliest an extension can interact with the initial document of a new window? There is a pattern with the crashes we're seeing that they involve the use of web extensions. But afaik, the crashes occur at document creation while document_start scripts run later off of CompleteInitialAboutblankLoad nsContentUtils::AddScriptRunner.

Flags: needinfo?(rob)

(In reply to Vincent Hilla [:vhilla] from comment #23)

(In reply to Henri Sivonen (:hsivonen) from comment #16)

The crashes almost always seem to have a third-party password manager extension. (Often Bitwarden, but also Proton, iCloud, 1Password)

:robwu can you confirm that document_start is the earliest an extension can interact with the initial document of a new window?

Yes, and document_start is connected to the document-element-inserted notification (source). We compile content scripts script on the first execution, asynchronously, and temporarily block the DOM parser until completion (source). The compiled script is cached and when a new document is loaded, the scripts execute synchronously at document-element-inserted.

Flags: needinfo?(rob)
Attachment #9530642 - Attachment is obsolete: true
Attachment #9530789 - Attachment is obsolete: true

The widest script blocking is the most promising change. That one with more tests:
https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=166963

Assignee: nobody → hsivonen
Attachment #9530790 - Attachment description: WIP: Bug 2003244 - Defer the notification for the global creation for the initial about:blank. → Bug 2003244 - Defer the notification for the global creation for the initial about:blank.
Status: NEW → ASSIGNED
Pushed by hsivonen@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/c6acc0cb971f https://hg.mozilla.org/integration/autoland/rev/71ae26400a95 Defer the notification for the global creation for the initial about:blank. r=smaug

Bug 543435 was reverted from Fx147. Updating the flags accordingly.

status-firefox147: affected → disabled
status-firefox148: --- → affected
tracking-firefox148: --- → +

https://hg.mozilla.org/mozilla-central/rev/71ae26400a95

Status: ASSIGNED → RESOLVED
Closed: 11 months ago
status-firefox148: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → 148 Branch

There are still crashes, so evidently they are caused something other than code running from observers for the notifications that got deferred.

It's known that in the same-process iframe creation case nsDocShell::Initialize can return a failure due to reaching the JS recursion limit, but the stacks after the landed fix don't show a nested event loop, so this IPC case can't be explained by the recursion limit.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
status-firefox148: fixed → affected
Target Milestone: 148 Branch → ---
See Also: → 2003759

Per team discussion: S3. The crash stacks show that this is not about the recursion limit. However, the crash rate is low enough that the situation does not appear to be too awful.

Severity: S2 → S3
Blocks: 2006305

There are a few crashes on try with the new annotations, e.g. this. So we can see that mDocumentViewer->Init(widget, bounds, aWindowActor) fails. Edit: Ah, but this is the known same-process iframe creation case where the JS recursion limit is reached.

Doing multiple TV runs, I reproduced this crash on Linux debug with test layout/base/crashtests/378682.html.

Adding some logging, the nsresult is 2147549183, which corresponds to NS_ERROR_UNEXPECTED and what failed is this !mBrowsingContext->AncestorsAreCurrent() || (mozilla::SessionHistoryInParent() && mBrowsingContext->IsInBFCache()) check. It is mBrowsingContext->AncestorsAreCurrent() = 0, mozilla::SessionHistoryInParent() = 1, mBrowsingContext->IsInBFCache() = 0. Looking further, AncestorsAreCurrent() is false because mBrowsingContext->IsDiscarded() on the second iteration.

I'm confused how the code of this specific crashtest relates to a crash in RecvConstructBrowser.

(deleted)

The crashtest is done, the test framework loads BLANK_URL_FOR_CLEARING on top of crashtests/378682.html to clear it out, this discards the window global. While this is ongoing, a timeout handler executes and reloads a nested about:blank iframe. It is located within a data:text/html, iframe. This reload causes a process switch because the remote type changes from web to file, which is how we get to RecvConstructBrowser.

:hsivonen, I don't know how to continue, can you take a look?

In this specific case, maybe the remote type of the initial about:blank was incorrect? Reload causing a remote type change seems unexpected.

More generally, should content js be allowed to run after we discarded the window global? Should we accept that nsDocShell::Initialize can fail in such ways and change RecvConstructBrowser to not crash if initializing the docshell fails?

Flags: needinfo?(hsivonen)

Perhaps we can add an early-return somewhere if an ancestor window global is discarded, though I'm unsure where. And I worry that Initialize might fail for other reasons too and crashes continue.

(In reply to Vincent Hilla [:vhilla] from comment #45)

Reload causing a remote type change seems unexpected.

Is this a case of data: URL in navigation from a page inheriting a principal but data: URL as reload being treated as lacking such principal context?

More generally, should content js be allowed to run after we discarded the window global?

It seems to me that it should not be allowed to run.

Should we accept that nsDocShell::Initialize can fail in such ways and change RecvConstructBrowser to not crash if initializing the docshell fails?

I think I need to redirect needinfo to Nika.

Flags: needinfo?(hsivonen) → needinfo?(nika)

It seems to me that it should not be allowed to run.

Per discussion with smaug, I was wrong on that point.

Next step from the meeting: Checking in the Pernosco trace if reload loses the precursor principal for the data: URL.

I think before bug 543435 we didn't go through this ProcessIsolation code in the parent.

  • The crashtest is loaded as file://.../378682.html with a file remote type.
  • It contains an iframe with data: URI that is loaded in a different process. This happens because ProcessIsolation determines it's remote type as ForceWebRemoteType due to this browser.tabs.remote.dataUriInDefaultWebProcess pref.
  • The data: URI causes the creation of an about:blank iframe. This iframe inherits the principal from the data: URI iframe.
  • When reloading this about:blank iframe, it's not the initial load anymore, we go through ProcessIsolation in the parent and due to the file URI precursor, a process switch occurs.

What confuses me is that loading a top level data:text/html,<iframe></iframe> with a Firefox version before bug 543435 also doesn't isolate the about:blank iframe from the data: parent. I think the parent process isn't involved at all. Here is a pernosco trace: https://pernos.co/debug/qJXit0GJLRT_rJWXP-DMPA/index.html (the trace doesn't include a reload, but reloading the iframe doesn't cause a process switch)

And I doubt that this specific case of reloading a nested about:blank within a data: URI iframe, right after shutdown is what we see crash reports for.

( Oh, and there is no principal mismatch. Because process selection issues sounds similar to bug 2004165 c7 )

Unassigning from self, since vhilla has investigated further in the most recent comments.

Assignee: hsivonen → nobody

This is a reminder regarding comment #18!

The bug is marked as tracked for firefox148 (nightly). We have limited time to fix this, the soft freeze is in 8 days. However, the bug still isn't assigned.

Next week is the final week of nightly for Fx148.

:vhilla, what is the current status of the investigation/priority here? Can we expect a fix in time for Fx148?

Flags: needinfo?(vhilla)

Process switch is a pre-existing issue

I disabled this BLANK_URL_FOR_CLEARING load and ran the layout/base/crashtests/378682.html test with ProcessIsolation logging for a build pre-bug 543435. After reload, the logs show "Process Switch: Changing Remoteness from 'web' to 'file'".

I think it's a bug that the reload causes a process switch, but it's a pre-existing issue.

Crash reproducible before bug 543435

See comment 15. I took the same pre-bug 543435 build and returned null when CreateDocumentViewerForActor fails. That way, I can get a crash with layout/base/crashtests/378682.html before bug 543435.

So at least for layout/base/crashtests/378682.html, the reason we start crashing is that bug 543435 started propagating a pre-existing failure.

Suggestion

(In reply to Henri Sivonen (:hsivonen) (away from Bugzilla until 2026-01-05) from comment #47)

(In reply to Vincent Hilla [:vhilla] from comment #45)

Should we accept that nsDocShell::Initialize can fail in such ways and change RecvConstructBrowser to not crash if initializing the docshell fails?

I think I need to redirect needinfo to Nika.

Nika is ooo so I don't think she'll get to this in time.

I'm not familiar enough with this process isolation code to resolve the pre-existing issue and expect there to be more ways to cause this crash. We already don't crash if viewer creation fails in nsFrameLoader::MaybeCreateDocShell. Bug 2005094 just recently depended on that. I'll put up a patch as a prototype.

Flags: needinfo?(vhilla)

Bug 543435 changed nsDocShell::Initialize from always returning true to
returning whether CreateInitialDocumentViewer succeeded. If this fails,
it is propagated up to ContentChild::RecvConstructBrowser where we crash.
The call to CreateDocumentViewerForActor that existed before bug 543435 in
nsWebBrowser::Create would've failed silently.

This commit restores avoids a crash by restoring the silent failure.

Assignee: nobody → vhilla
See Also: → 2008366

:hsivonen, do you agree we should land my patch as explained in comment 54?

Flags: needinfo?(hsivonen)

(In reply to Vincent Hilla [:vhilla] from comment #56)

:hsivonen, do you agree we should land my patch as explained in comment 54?

Let's land it. Thank you.

Flags: needinfo?(hsivonen)
Pushed by abutkovits@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/7a8a3e1a81cd https://hg.mozilla.org/integration/autoland/rev/b1c5621757a0 Revert "Bug 2003244 - Don't crash when failing to initialize docshell. r=dom-core,mccr8" for causing failures at nsGlobalWindowOuter.cpp.

testing/crashtest/final/1419902.html tests exceeding the recursion limit causing something within window.open to fail. The call stack goes through OpenWindowInternal, ProvideWindow and the same nsWebBrowser::Create were we don't want to propagate the failure anymore.

Assuming we always succeed to eagerly create a document, I changed SetInitialPrincipal to assert mDoc in bug 2004407. But before bug 543435, all the parts of window.open being permissive of no document existing was what caused the test to pass.

Let's try reverting that part of bug 2004407: https://treeherder.mozilla.org/jobs?repo=try&revision=8480569613378a2cbff4e88e0037885de914eb61

Flags: needinfo?(vhilla)

https://hg.mozilla.org/mozilla-central/rev/3c99a43d1d94

Status: REOPENED → RESOLVED
Closed: 11 months ago → 10 months ago
status-firefox148: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → 148 Branch

(In reply to Henri Sivonen (:hsivonen) from comment #47)

(In reply to Vincent Hilla [:vhilla] from comment #45)

Reload causing a remote type change seems unexpected.

Is this a case of data: URL in navigation from a page inheriting a principal but data: URL as reload being treated as lacking such principal context?

Reload causing a process switch seems pretty normal to me. We are doing a fresh load, and could make a different decision about where the URL is present.

This could even be explicitly requested by a website which was previously not serving a Cross-Origin-Opener-Policy header, and then after a reload starts serving that header.

Should we accept that nsDocShell::Initialize can fail in such ways and change RecvConstructBrowser to not crash if initializing the docshell fails?

I think I need to redirect needinfo to Nika.

I think nsDocShell::Initialize failing is pretty darn catastrophic. How would we even begin to try to recover in that situation? The entire BrowserChild is completely broken after that point, and I expect a number of things would start falling over at that point.

AFAICT the current patch just throws away the error if Initialize fails, and throws it away early enough in the process that it also changes the behaviour for nsFrameLoader::Show, AppWindow::Initializem, and nsAppShellService::CreateWindowlessBrowser which concerns me a bit, as those codepaths weren't the ones leading to problems.

If we're going to be ignoring errors, I'd prefer to do this only for the BrowserChild case, and ideally I'd prefer to recover as well as we can in that case, and make sure that the parent process does a reasonable thing.

Flags: needinfo?(nika) → needinfo?(vhilla)
Blocks: 2009931

I was focused on fixing the regression, i.e. avoiding the crash by restoring behavior prior to bug 543435. That should correspond to ignoring the error. But I agree that isn't optimal. Even more so because we now enforce eager viewer creation and a call to EnsureDocumentViewer cannot be used to recover. Though that wouldn't have worked anyway if being discarded as in crashtests/378682.html.

(In reply to Nika Layzell [:nika] (ni? for response) from comment #64)

Reload causing a process switch seems pretty normal to me. We are doing a fresh load, and could make a different decision about where the URL
is present.

I'm referring to bug 2008366 which doesn't reproduce in Chromium. But I think it's minor and shouldn't cause a crash.

AFAICT the current patch just throws away the error if Initialize fails, and throws it away early enough in the process that it also changes the behaviour for nsFrameLoader::Show, AppWindow::Initializem, and nsAppShellService::CreateWindowlessBrowser which concerns me a bit, as those codepaths weren't the ones leading to problems.

I added checks to some of these call sites. From what I see, CreateWindowlessBrowser is the only behavior change. And there's added fragility in case future calls are added.

If we're going to be ignoring errors, I'd prefer to do this only for the BrowserChild case, and ideally I'd prefer to recover as well as we can in that case, and make sure that the parent process does a reasonable thing.

I agree and filed bug 2009931 as follow-up.

Flags: needinfo?(vhilla)
QA Whiteboard: [qa-triage-done-c149/b148]
Regressions: 2013289
See Also: → 2013828
See Also: → 2014692
See Also: → 2032336
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: