1280505 - [e10s] browser/base/content/test/general/browser_save_link-perwindowpb.js asserts and leaks nsNativeDragTarget

Status: RESOLVED INCOMPLETE

(Core :: DOM: Content Processes, defect)

Description

[Neil Deakin]

This test is causing a leak (see bug 1268756) and has a pile of assertions suggesting a real bug of some sort:

The logs are at https://treeherder.mozilla.org/#/jobs?repo=try&revision=8e1376f3138b10fa9c870aa443d3fcd06e82186b  which show where nsNativeDragTarget is allocated and freed. For the two nsNativeDragTargets that are not freed, there is an assert immediately after the allocation line:

20:02:52     INFO -  [Parent 4968] ###!!! ASSERTION: This should never be switched out from under us!: 'currentWndProc == (LONG_PTR)NeuteredWindowProc', file c:/builds/moz2_slave/try-w64-d-00000000000000000000/build/src/ipc/glue/WindowsMessageLoop.cpp, line 686

And other assertions occur before the allocation:

20:02:52     INFO -  [Parent 4968] ###!!! ASSERTION: Received "nonqueued" ui message "WM_NCCREATE" during a synchronous IPC message for window 0x1280076 ("MozillaDropShadowWindowClass"), sending it to DefWindowProc instead of the normal window procedure.: 'Error', file c:/builds/moz2_slave/try-w64-d-00000000000000000000/build/src/ipc/glue/WindowsMessageLoop.cpp, line 265
20:02:52     INFO -  [Parent 4968] ###!!! ASSERTION: Received "nonqueued" ui message "WM_CREATE" during a synchronous IPC message for window 0x1280076 ("MozillaDropShadowWindowClass"), sending it to DefWindowProc instead of the normal window procedure.: 'Error', file c:/builds/moz2_slave/try-w64-d-00000000000000000000/build/src/ipc/glue/WindowsMessageLoop.cpp, line 265
20:02:52     INFO -  [Parent 4968] ###!!! ASSERTION: Received "nonqueued" ui message "WM_SIZE" during a synchronous IPC message for window 0x1280076 ("MozillaDropShadowWindowClass"), sending it to DefWindowProc instead of the normal window procedure.: 'Error', file c:/builds/moz2_slave/try-w64-d-00000000000000000000/build/src/ipc/glue/WindowsMessageLoop.cpp, line 265
20:02:52     INFO -  [Parent 4968] ###!!! ASSERTION: Received "nonqueued" ui message "WM_MOVE" during a synchronous IPC message for window 0x1280076 ("MozillaDropShadowWindowClass"), sending it to DefWindowProc instead of the normal window procedure.: 'Error', file c:/builds/moz2_slave/try-w64-d-00000000000000000000/build/src/ipc/glue/WindowsMessageLoop.cpp, line 265

Bugs for these assertions are tracked by 1052362.

Comment 1

[:Gijs (he/him)]

Does the same leak happen when manually doing something similar to the test (in other words, are users likely to encounter this leak 'in the wild')? If so, should we track this for a release?

Comment 2

[Neil Deakin]

I don't know. I didn't look at the test.

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Aaron, do you know what these errors mean?

Comment 4

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to Bill McCloskey (:billm) from comment #3)
> Aaron, do you know what these errors mean?

I think that these are all related; I've seen this before in other contexts.

(In reply to Neil Deakin from comment #0)
> 
> 20:02:52     INFO -  [Parent 4968] ###!!! ASSERTION: This should never be
> switched out from under us!: 'currentWndProc ==
> (LONG_PTR)NeuteredWindowProc', file
> c:/builds/moz2_slave/try-w64-d-00000000000000000000/build/src/ipc/glue/
> WindowsMessageLoop.cpp, line 686

During synchronous IPC calls, we replace ("subclass" in Windows parlance) a window's event handler (aka WndProc) with NeuteredWindowProc. Once the sync IPC finishes, we restore each window's WndProc from NeuteredWindowProc back to its original. When this assertion fires it means that something else has unexpectedly subclassed a window during the IPC call.

> 
> And other assertions occur before the allocation:
> 
> 20:02:52     INFO -  [Parent 4968] ###!!! ASSERTION: Received "nonqueued" ui
> message "WM_NCCREATE" during a synchronous IPC message for window 0x1280076
> ("MozillaDropShadowWindowClass"), sending it to DefWindowProc instead of the
> normal window procedure.: 'Error', file
> c:/builds/moz2_slave/try-w64-d-00000000000000000000/build/src/ipc/glue/
> WindowsMessageLoop.cpp, line 265
> 20:02:52     INFO -  [Parent 4968] ###!!! ASSERTION: Received "nonqueued" ui
> message "WM_CREATE" during a synchronous IPC message for window 0x1280076
> ("MozillaDropShadowWindowClass"), sending it to DefWindowProc instead of the
> normal window procedure.: 'Error', file
> c:/builds/moz2_slave/try-w64-d-00000000000000000000/build/src/ipc/glue/
> WindowsMessageLoop.cpp, line 265
> 20:02:52     INFO -  [Parent 4968] ###!!! ASSERTION: Received "nonqueued" ui
> message "WM_SIZE" during a synchronous IPC message for window 0x1280076
> ("MozillaDropShadowWindowClass"), sending it to DefWindowProc instead of the
> normal window procedure.: 'Error', file
> c:/builds/moz2_slave/try-w64-d-00000000000000000000/build/src/ipc/glue/
> WindowsMessageLoop.cpp, line 265
> 20:02:52     INFO -  [Parent 4968] ###!!! ASSERTION: Received "nonqueued" ui
> message "WM_MOVE" during a synchronous IPC message for window 0x1280076
> ("MozillaDropShadowWindowClass"), sending it to DefWindowProc instead of the
> normal window procedure.: 'Error', file
> c:/builds/moz2_slave/try-w64-d-00000000000000000000/build/src/ipc/glue/
> WindowsMessageLoop.cpp, line 265
> 

These ones are caused because they are not being handled by ProcessOrDeferMessage(). Sometimes this is a bug, sometimes this isn't. :-)

bent once told me that the set of messages that are handled in that function was essentially developed by trial and error: we'd keep adding new messages to that handler until there were no more of those assertions. OTOH, blindly adding new messages to that set is not always the correct thing to do: some messages are intended for scenarios that we do not expect to occur during a sync IPC call. This bug is a case of the latter situation.

Given the message sequence shown above (WM_NCCREATE, WM_CREATE, WM_SIZE, WM_MOVE), that is telling us that a new MozillaDropShadowWindowClass window was created in the midst of a synchronous IPC call. In general our code is not really prepared to handle that case (and in the past when I've seen it, it was clearly a bug elsewhere in our code).

I'd suggest that the first step here is to determine how/why a new window is being created in the midst of sync IPC.

Comment 5

[Jens Stutte [:jstutte]]

It seems this test is disabled under e10s/windows currently.