2036624 - Crash when clicking the PiP location bar button is clicked [ESET]

Status: NEW

(Core :: Audio/Video, defect)

Description

[Ryan Jones-Ward [:sciguyryan]]

I have been running into a hard crash each time the PiP button located on the location bar is clicked. This causes the app to immediately crash and exit. Here are several of the crash reports:

bp-efd14bd4-aecf-4747-9d54-c6a9b0260503
bp-1123e447-054d-432a-a767-f95c60260503
bp-b6fb7e55-0828-4df6-b36b-c32b60260422

Unfortunately, looking at those reports, I don't believe they are very helpful. This has been present for a while now, but it took me a bit to figure out exactly what was causing it. It's entirely reproducible and I'm happy to run some tests to help pinpoint the problem, if there are any suggestions.

Comment 1

[Jim Mathies [:jimm]]

Hey Ryan, what are the steps you are using to reproduce? Also, any sense as to whether this is a regression from a previous release?

Comment 2

[Jim Mathies [:jimm]]

Also, if possible, disable ESET and test a bit. This crash seems to indicate it's involved.

Comment 3

[Ryan Jones-Ward [:sciguyryan]]

Sorry for the delay here. I'm recovering from a chest infection and this has limited how much time I've had at the computer.

Initially tested by disabling ESET browser protections. That didn't change things. I then disabled all protection entirely, which also didn't change the crash. So I removed ESET entirely and that does resolve the crash. So it's definitely ESET's fault.

The steps to reproduce were simply to click the PiP icon in the address bar (to the left of the search widget). The crash would result Immediately upon doing so.

Comment 4

[Bob Owen (:bobowen)]

I've managed to reproduce this on a VM with a trial version of ESET's Home Security Premium.
It only seems to occur in Nightly and only on Official/Released builds. ESET doesn't seem to inject into other m-c or autoland builds.

Here's the bisection pushlog:
https://hg-edge.mozilla.org/mozilla-central/pushloghtml?fromchange=1d083dd7a3b058d466ed3aab07299c839bd827a5&tochange=fc89a2988ecb15ad28fdabcd660bcab2efcd4421

emilio - Bug 1993474 looks like the most likely candidate given the recursion through CallWindowProcW and the use of PiP as the trigger.

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Hmm... So ESET is calling into our wndproc, which is calling into ESET's... That seems very suspicious indeed.

I'm not on a windows machine atm but if you have the environment to repro handy, mind testing the following?

• Try a build with widget.windows.windowsappsdk.enabled=false. Does it repro there? For all windows or only for PiP windows?
• Try this diff and check if PIP windows go under the taskbar? If so we could probably just remove this special-case:

diff --git a/widget/windows/nsWindow.cpp b/widget/windows/nsWindow.cpp
index 3ccd6988e2d9..77b8d750325c 100644
--- a/widget/windows/nsWindow.cpp
+++ b/widget/windows/nsWindow.cpp
@@ -1522,13 +1522,7 @@ DWORD nsWindow::WindowExStyle() {
  **************************************************************/

 bool nsWindow::ShouldAssociateWithWinAppSDK() const {
-  // We currently don't need any SDK functionality for for PiP windows,
-  // and using the SDK on these windows causes them to go under the
-  // taskbar (bug 1995838).
-  //
-  // TODO(emilio): That might not be true anymore after bug 1993474,
-  // consider re-testing and removing that special-case.
-  return IsTopLevelWidget() && mPiPType == PiPType::NoPiP;
+  return IsTopLevelWidget();
 }

 bool nsWindow::AssociateWithNativeWindow() {

Comment 6

[Bob Owen (:bobowen)]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #5)

Hmm... So ESET is calling into our wndproc, which is calling into ESET's... That seems very suspicious indeed.

I'm not on a windows machine atm but if you have the environment to repro handy, mind testing the following?

The exact circumstances seem a little weird, Nightly stopped reproducing after an update and now if I reinstall the latest version it fails first time, but then it won't reproduce on restart.
I've never seen it on Release and I've tried the one that's just come out.

It always seems to reproduce from a Nightly zip (after that change) downloaded from archive.mozilla.org.

• Try a build with widget.windows.windowsappsdk.enabled=false. Does it repro there? For all windows or only for PiP windows?

This seems to fix the crash.

• Try this diff and check if PIP windows go under the taskbar? If so we could probably just remove this special-case:

The PiP window always seems to appear just above the taskbar in the bottom corner of the remaining screen, even with this change.
Seems to behave the same as current Nightly.
I can't tell if this fixes the crash, because ESET doesn't inject into the local build.

Comment 7

[Emilio Cobos Álvarez [:emilio]]

Ok, let's try bug 2040926?

Comment 8

[Bob Owen (:bobowen)]

If I run from the unzipped Firefox Build ID 20260521094038 (last one before bug 2040926 landed) it still crashes on my VM with the STR.
Latest unzipped Firefox Build ID 20260522093247, doesn't crash.

Hi Ryan - would you be able to retest the latest Nightly to see if this also fixes the crash for you?