531551 - Firefox 3.6 topcrash [@ UserCallWinProcCheckWow] due to old Acrobat Plugin (nppdf32.dll) [@ nppdf32.dll@0x5e14 ] [@ xpcom.dll@0x80 ]

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[David Baron :dbaron:]

In Firefox 3.6 betas, about 50% of the crashes at the signature UserCallWinProcCheckWow are correlated with and have a stack going through nppdf32.dll.  The correlations are only with old versions of the Acrobat plugin, primarily 8.1 and 9.0.  Acrobat 9.1 is not a problem.  So this seems related to bug 434593, but it's also not clear why this just started showing up as a topcrash in Firefox 3.6 betas when it wasn't a topcrash in Firefox 3.5.

(Bug 501429 covers another topcrash at this signature that does show up in Firefox 3.5.)

Comment 1

[d]

Does this occur when a PDF document is being loaded/opened or just randomly? And, before the release of Firefox 3.6, can we blocklist versions of Acrobat Reader lower than 9.1 for Firefox versions 3.6+?

Comment 2

[Jo Hermans]

I got it several times when trying to go back to the previous page, thus unloading the plugin. But Firefox 3.6 doesn't really unload the plugin from memory anymore, maybe this is the reason why it appears in 3.6 ?

Comment 3

[David Baron :dbaron:]

(In reply to comment #2)
> I got it several times when trying to go back to the previous page, thus
> unloading the plugin. But Firefox 3.6 doesn't really unload the plugin from
> memory anymore, maybe this is the reason why it appears in 3.6 ?

Was it reproducable using certain steps?  (At least some of the time?)  If so, could you share those steps with us?

Comment 4

[Jo Hermans]

No, not on a specific webpage, I just saw it several times going back on a webpage by pressing the back-button on the toolbar. (they all used Flash though). But not when I tried to repeat it. Gmail was the most common case, but that's filed under bug 501429.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

UserCallWinProcCheckWow|EXCEPTION_ACCESS_VIOLATION (264 crashes) 
  66% (173/264) vs.   5% (539/11574) nppdf32.dll
    15% (40/264) vs.   1% (74/11574) 8.1.0.137
    19% (49/264) vs.   1% (66/11574) 8.1.3.187 
    14% (38/264) vs.   1% (78/11574) 9.0.0.332
     1% (2/264) vs.   2% (201/11574) 9.1.0.163

The correlation of interested modules also shows up version 9.1. But it's only 1%.

Comment 7

[David Baron :dbaron:]

(In reply to comment #5)
> UserCallWinProcCheckWow|EXCEPTION_ACCESS_VIOLATION (264 crashes) 
>   66% (173/264) vs.   5% (539/11574) nppdf32.dll
>     15% (40/264) vs.   1% (74/11574) 8.1.0.137
>     19% (49/264) vs.   1% (66/11574) 8.1.3.187 
>     14% (38/264) vs.   1% (78/11574) 9.0.0.332
>      1% (2/264) vs.   2% (201/11574) 9.1.0.163
> 
> The correlation of interested modules also shows up version 9.1. But it's only
> 1%.

No, that not showing that 9.1 is contributing to the crash.  It's saying that 1% of the users who hit this crash have 9.1 loaded, against 2% in our whole crash sample.  (They probably have one of the other causes of UserCallWinProcCheckWow crashes.)

Comment 8

[David Baron :dbaron:]

It's worth watching nightly data to see if bug 520639's fix fixed this.  It looks quite possible so far.

Comment 9

[(not currently active) Ted Mielczarek]

I have a friend who's hitting a repeatable crash with this signature:
http://crash-stats.mozilla.com/report/index/5fabde48-5124-47de-9f06-332ad2091221

Looks pretty similar to the other crashes with this signature.

He crashes going to a site (requires login) that uses some plugin for displaying CAD models. I've pointed him at 3.6b5 and he's promised to test with that to see if it's fixed there.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Ted, which type of plugin does it use? Could you ask him please?

Comment 11

[(not currently active) Ted Mielczarek]

It appears to be npViewpoint.dll - "MetaStream 3 Plugin".

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #11)
> It appears to be npViewpoint.dll - "MetaStream 3 Plugin".

Alright. This is covered by bug 501429 - at least for now.

Comment 13

[David Baron :dbaron:]

The Acrobat-related crashes are about 60% of the UserCallWinProcCheckWow crashes (the #1 topcrash) in Firefox 3.6b5.

Comment 14

[David Baron :dbaron:]

Nominating the #1 and #2 topcrashes so they're on people's radar, although I personally don't think they're hard blockers.

Comment 15

[Mike Beltzner [:beltzner, not reading bugmail]]

Agreed with dbaron's assessment.

Comment 16

[d]

Why isn't anything happening here? This crash is on the top of the 3.6 crash-list. Shouldn't the old Adobe plugin/dll be blocked?

Comment 17

[timeless]

re comment 16, we'd need a bug filed in addons.mozilla.org:blocklisting, i'm not quite sure what dbaron wants done here, but since he filed it in Core:plugins, it seems he isn't specifically asking for blocklisting. I'm slowly trying to get other stuff blocklisted, so I might poke this eventually.

Comment 18

[David Baron :dbaron:]

Well, we don't know why it's crashing in 3.6 when it wasn't crashing in 3.5; that's why I filed it in Core::Plugins.  That said, there may be other good reasons to blacklist old Acrobat plugins, and this may be an additional one, so filing a bug in blocklisting would make sense.

Comment 19

[d]

Yes, Adobe Reader has had lots of security holes recently, upgrading would benefit the user in more ways than just resolving the Firefox crashes.

Comment 20

[Marcia Knous [:marcia]]

I am able to reproduce this crash in the lab 100% using a Windows Vista machine and FF 3.6. https://crash-stats.mozilla.com/report/index/bp-08c7fba0-f193-4fb9-8287-18c392100201.

STR:
1. Install the Viewpoint plugin 3.6.0.59 - for some reason even though it is blocklisted I can still install.
2. Visit http://tinyurl.com/y9t33dn
3. Crash.

Here is my profile info:

        Microsoft .NET Framework Assistant
        1.1
        true
        {20a82645-c095-46ed-80e3-08825760534b}

        Bing Bar
        5.0
        true
        [e-mail]

        Search Helper Extension
        1.0
        true
        {27182e60-b5f3-411c-b545-b44205977502}

        Skype extension for Firefox
        3.3.0.3971
        true
        {B13721C7-F507-4982-B2E5-502A71474FED}

        Move Media Player
        1.0.0.%(version)s
        true
        [e-mail]

  Modified Preferences

        accessibility.typeaheadfind.flashBar
        0

        browser.places.smartBookmarksVersion
        2

        browser.startup.homepage_override.mstone
        rv:1.9.2

        dom.max_script_run_time
        1800

        extensions.lastAppVersion
        3.6

        general.useragent.extra.microsoftdotnet
        (.NET CLR 3.5.30729)

        network.cookie.prefsMigrated
        true

        privacy.sanitize.migrateFx3Prefs
        true

Comment 21

[David Baron :dbaron:]

Can we please keep this bug about the problem with the Acrobat plugin?

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+1]]

David, for what are we waiting for to get old versions blocked?

Comment 23

[David Baron :dbaron:]

Somebody should probably file a bug in the blocklisting component to start that process.

There was still a regression between 3.5 and 3.6; that's what's covered by this bug.

Comment 24

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Filed as bug 548002.

Comment 26

[chris hofmann]

I'm seeing about 200-270 crashes per day from
https://qtwu2.turbotaxonline.intuit.com/secure/ttonline.htm ,
turbotax.intuit.com and turbotax.com with this stack signature  and general
users comments about what looks like pdf interaction to view and print out US
tax returns.  I think there might be several parts of their tax filing app that
might tickle pdf interaction bugs.

an example is

https://qtwu2.turbotaxonline.intuit.com/tto-web/ttaxol/TaxReturn.pdf?cr=printType%3D1%26returnType%3D2%26miniwks%3Dfalse%26watermark%3Dfalse&cmd=PRINT&uid=XXXXXXXXX%3A0&prodid=512
&csrc=3468337910&app=ttwprodapp22&pt=63770&D=TaxReturn.pdf 

UserCallWinProcCheckWow
http://crash-stats.mozilla.com/report/index/52bcce39-98db-4d78-ba24-8b63a2100328

I suspect volume on this could increase as the April 15th tax filing deadline
in the US approaches.

I can't think of any ideas other than maybe getting intuit to publish warnings
about installing the latest version of acrobat before starting the filing
process.   maybe we can also contact them to see if there is a way create
reproducible test cases with the pdf interaction.

As mentioned above the UserCallWinProcCheckWow crashes are mostly 3.6.x.

checking --- 20100328-crashdata.csv UserCallWinProcCheckWow
release total-crashes
              UserCallWinProcCheckWow crashes

3.5.8   34753   271     0.00779789
3.0.18  10964   91      0.00829989
3.6.2  212098  10906   0.0514196

that seems to hold up for these turbotax UserCallWinProcCheckWow crashes. 
about 99% of them are 3.6 or 3.6.2.

Comment 27

[barryk]

In Bug 552093 I posted some HTML/Javascript to reproduce the crash (or one of them at least) with Acrobat < 9.1 and FF 3.6.x. It seems if you have an iframe with a PDF and then destroy the iframe via javascript the browser crashes. An embed rather than an iframe does not cause a crash.

Comment 28

[chris hofmann]

I'm noticing that 70% of UserCallWinProcCheckWow crashes are windows 95

os breakdown
3435    0.38397 Windows NT5.1.2600 Service Pack 3
1642    0.183546        Windows NT5.1.2600 Service Pack 2
39      0.00435949      Windows NT5.1.2600 Szervizcsomag 3
37      0.00413593      Windows NT5.1.2600 Dodatek Service Pack 3
36      0.00402414      Windows NT5.1.2600 Service Pack 1
28      0.00312989      Windows NT5.1.2600 Dodatek Service Pack 2
24      0.00268276      Windows NT5.1.2600 Szervizcsomag 2
16      0.00178851      Windows NT5.2.3790 Service Pack 2
16      0.00178851      Windows NT5.1.2600
3       0.000335345     Windows NT5.1.2600 Service Pack 3, v.5913
3       0.000335345     Windows NT5.1.2600 Service Pack 3, v.3264

1730    0.193383        Windows NT6.0.6002 Service Pack 2
977     0.109211        Windows NT6.0.6001 Service Pack 1
384     0.0429242       Windows NT6.0.6000

566     0.0632685       Windows NT6.1.7600

and top url's in the crash data for this signature might be attempts to download various localized versions of the reader from the adobe and other sites.

  39 http://ardownload.adobe.com/pub/adobe/reader/win/9.x/9.3/enu/AdbeRdr930_en_US.exe
  15 http://ardownload.adobe.com/pub/adobe/reader/win/9.x/9.3/deu/AdbeRdr930_de_DE.exe
 10 http://ardownload.adobe.com/pub/adobe/reader/win/9.x/9.3/fra/AdbeRdr930_fr_FR.exe
  8 http://ardownload.adobe.com/pub/adobe/reader/win/9.x/9.3/esp/AdbeRdr930_es_ES.exe
   5 http://ardownload.adobe.com/pub/adobe/reader/win/9.x/9.3/ptb/AdbeRdr930_pt_BR.exe
   2 http://ardownload.adobe.com/pub/adobe/reader/win/9.x/9.3/suo/AdbeRdr930_fi_FI.exe
   2 http://ardownload.adobe.com/pub/adobe/reader/win/9.x/9.3/nld/AdbeRdr930_nl_NL.exe
   2 http://ardownload.adobe.com/pub/adobe/reader/win/9.x/9.3/ita/AdbeRdr930_it_IT.exe
   1 https://owa.pplweb.com/pub/adobe/reader/win/9.x/9.3/enu/,DanaInfo=ardownload.adobe.com+AdbeRdr930_en_US.exe

Anyone have a win95 set up to test on?

Comment 29

[Samuel Sidler (:ss)]

We don't support Windows 95 and haven't since Firefox 2, which doesn't support Breakpad. The data you pasted shows "Windows NT5.1.2600 Service Pack 3" which is Windows XP, Service Pack 3. Unless I'm missing something?

Comment 30

[Carsten Book [:Tomcat]]

(In reply to comment #29)
> We don't support Windows 95 and haven't since Firefox 2, which doesn't support
> Breakpad. The data you pasted shows "Windows NT5.1.2600 Service Pack 3" which
> is Windows XP, Service Pack 3. Unless I'm missing something?

Yep its XP. Windows NT 5 was Windows 2000 and Windows 98/95/ME have different version numbers: 

Windows 95 retail, OEM     4.00.950                    
Windows 95 retail SP1      4.00.950A                    
OEM Service Release 2      4.00.1111* (4.00.950B)       
OEM Service Release 2.1    4.03.1212-1214* (4.00.950B)  
OEM Service Release 2.5    4.03.1214* (4.00.950C)      
Windows 98 retail, OEM     4.10.1998                    
Windows 98, Security CD    4.10.1998A 
Windows 98 Second Edition  4.10.2222A                   
Windows 98 SE Security CD  4.10.2222B
Windows Me                 4.90.3000                   
Windows Me Security CD     4.90.3000A

Comment 31

[chris hofmann]

this signature that ends up in a stack overflow also looks related


Firefox 3.6.3 Crash Report [@ nppdf32.dll@0x5e14 ]

http://crash-stats.mozilla.com/report/index/87662843-2570-467a-94e5-ee2e52100407

0  	nppdf32.dll  	nppdf32.dll@0x5e14  	
1 	user32.dll 	InternalCallWinProc 	
2 	user32.dll 	UserCallWinProcCheckWow 	
3 	user32.dll 	CallWindowProcAorW 	
4 	user32.dll 	CallWindowProcA 	

5 	nppdf32.dll 	nppdf32.dll@0x624e 	
6 	user32.dll 	InternalCallWinProc 	
7 	user32.dll 	UserCallWinProcCheckWow 	
8 	user32.dll 	CallWindowProcAorW 	
9 	user32.dll 	CallWindowProcA 	
10 	nppdf32.dll 	nppdf32.dll@0x624e

...rinse and repeat numerous times...

19295  	nppdf32.dll  	nppdf32.dll@0x624e  	
19296 	user32.dll 	InternalCallWinProc 	
19297 	user32.dll 	UserCallWinProcCheckWow 	
19298 	user32.dll 	CallWindowProcAorW 	
19299 	user32.dll 	CallWindowProcA 	

19300 	nppdf32.dll 	nppdf32.dll@0x624e 	
19301 	user32.dll 	InternalCallWinProc 	
19302 	user32.dll 	UserCallWinProcCheckWow 	
19303 	user32.dll 	CallWindowProcAorW 	
19304 	user32.dll 	CallWindowProcA 	
19305 	nppdf32.dll 	nppdf32.dll@0x624e 	

19306 	user32.dll 	InternalCallWinProc 	
19307 	user32.dll 	UserCallWinProcCheckWow 	
19308 	user32.dll 	DispatchClientMessage 	
19309 	user32.dll 	__fnDWORD 	
19310 	ntdll.dll 	KiUserCallbackDispatcher 	
19311 	nppdf32.dll 	nppdf32.dll@0x6147 	
19312 	user32.dll 	NtUserPeekMessage 	
19313 	xul.dll 	nsAppShell::ProcessNextNativeEvent 	widget/src/windows/nsAppShell.cpp:172
19314 	xul.dll 	nsBaseAppShell::OnProcessNextEvent 	widget/src/xpwidgets/nsBaseAppShell.cpp:278
19315 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:508
19316 	xul.dll 	nsBaseAppShell::Run 	widget/src/xpwidgets/nsBaseAppShell.cpp:170
19317 	xul.dll 	nsAppStartup::Run 	toolkit/components/startup/src/nsAppStartup.cpp:183
19318 	nspr4.dll 	nspr4.dll@0xbdcf 	
19319 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp:120
19320 	firefox.exe 	__tmainCRTStartup 	obj-firefox/memory/jemalloc/crtsrc/crtexe.c:591
19321 	kernel32.dll 	BaseProcessStar

Comment 32

[chris hofmann]

here is another signature running though similar code and 100% crash on winxp

Firefox 3.6.3 Crash Report [@ xpcom.dll@0x80 ]

http://crash-stats.mozilla.com/report/index/bbdb9a99-4081-459c-97e4-fcea32100405

Frame  	Module  	Signature [Expand]  	Source
0 	xpcom.dll 	xpcom.dll@0x80 	
1 	user32.dll 	UserCallWinProcCheckWow 	
2 	firefox.exe 	firefox.exe@0x1c63e 	
3 	user32.dll 	CallWindowProcA 	
4 	nppdf32.dll 	nppdf32.dll@0x6e90 	
5 	user32.dll 	InternalCallWinProc 	
6 	user32.dll 	UserCallWinProcCheckWow 	
7 	user32.dll 	DispatchClientMessage 	
8 	user32.dll 	__fnDWORD 	
9 	ntdll.dll 	KiUserCallbackDispatcher 	
10 	nppdf32.dll 	nppdf32.dll@0x6d86 	
11 	user32.dll 	NtUserPeekMessage 	
12 	xul.dll 	nsAppShell::ProcessNextNativeEvent 	widget/src/windows/nsAppShell.cpp:172
13 	xul.dll 	nsBaseAppShell::OnProcessNextEvent 	widget/src/xpwidgets/nsBaseAppShell.cpp:284
14 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:508
15 	xul.dll 	nsBaseAppShell::Run 	widget/src/xpwidgets/nsBaseAppShell.cpp:170
16 	xul.dll 	nsAppStartup::Run 	toolkit/components/startup/src/nsAppStartup.cpp:183
17 	nspr4.dll 	nspr4.dll@0xbdcf 	
18 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp:120
19 	firefox.exe 	__tmainCRTStartup 	obj-firefox/memory/jemalloc/crtsrc/crtexe.c:591
20 	kernel32.dll 	BaseProcessStart

Comment 33

[Zane U. Ji]

Steps to reproduce it:
1. Open http://users.ece.gatech.edu/~bonnie/book3/
2. Click "Worked Problems" in left panel
3. Click to open a problems (or solutions) link in right panel
4. Navigate back or repeat step 2.
5. Repeat step 3 and 4 until FF crashes.

Comment 34

[timeless]

it sounds like they didn't unregister their window proc, and then the new plugin instance registers again, and they call themselves.

i wonder if this relates to us having fewer widgets in our native widget tree

Comment 35

[chris hofmann]

I've got some new toys that help to show top source line of any signature.

They show /nsAppShell.cpp as the leading top source line associated with this signature like the stacks in comment 31 and 32.

5604 widget/src/windows/nsAppShell.cpp  UserCallWinProcCheckWow
 652 view/src/nsView.cpp  UserCallWinProcCheckWow
 627 widget/src/windows/nsWindow.cpp  UserCallWinProcCheckWow
 282 layout/generic/nsObjectFrame.cpp  UserCallWinProcCheckWow
 265 dom/base/nsFocusManager.cpp  UserCallWinProcCheckWow
 225 widget/src/windows/nsToolkit.cpp  UserCallWinProcCheckWow
 190 layout/base/nsDocumentViewer.cpp  UserCallWinProcCheckWow
 180 widget/src/xpwidgets/nsBaseAppShell.cpp  UserCallWinProcCheckWow
 145 xpcom/io/nsLocalFileWin.cpp  UserCallWinProcCheckWow
  64 xpfe/appshell/src/nsXULWindow.cpp  UserCallWinProcCheckWow
  63 modules/plugin/base/src/nsPluginHost.cpp  UserCallWinProcCheckWow
<long tail of other top source lines>

Its possible that this bug is the biggest part of #1 UserCallWinProcCheckWow problem.

Rudi,  can you help find someone to look at this from the Adobe Reader side?

It would be good to confirm that latest versions have been fixed.

Comment 36

[Zane U. Ji]

(In reply to comment #34)
> i wonder if this relates to us having fewer widgets in our native widget tree
It used to be:
	Window 00081A16 "Fundamentals Signals Systems - Shiretoko" MozillaUIWindowClass
		Window 000B19E0 "" MozillaWindowClass
			Window 00061A1E "" MozillaWindowClass
				Window 00061A20 "" MozillaWindowClass
Window consists of 3 panels:
					Window 00061A22 "" MozillaContentWindowClass
						Window 001205D0 "" MozillaWindowClass
Right panel:
							Window 00030A90 "" MozillaContentFrameWindowClass
								Window 00131E7C "" MozillaWindowClass
									Window 00051E68 "" MozillaWindowClass
										Window 00071E64 "" MozillaWindowClass
											Window 00031E6A "http://users.ece.gatech.edu/~bonnie/book3/worked_problems/Chap1_periodic.pdf - Adobe Reader" Static
Left panel:
							Window 00080A94 "" MozillaContentFrameWindowClass
								Window 00011DA6 "" MozillaWindowClass
									Window 00011DA8 "" MozillaWindowClass
Top panel:
							Window 000C080E "" MozillaContentFrameWindowClass
								Window 00030A8C "" MozillaWindowClass


Now it is:
	Window 001A1D46 "Fundamentals Signals Systems - SeaMonkey {Build ID: 20100417005239}" MozillaUIWindowClass
		Window 001C1D42 "" MozillaWindowClass
			Window 00211D34 "" MozillaContentWindowClass
Window consist of 3 panels:
				Window 001D0C42 "" MozillaWindowClass
Right panel:
					Window 000C1DFE "" MozillaWindowClass
						Window 00061DD4 "http://users.ece.gatech.edu/~bonnie/book3/worked_problems/Chap1_periodic.pdf - Adobe Reader" Static
					Window 00081DDE "FAKETRACKPOINTSCROLLBAR" ScrollBar
				Window 00091DD0 "FAKETRACKPOINTSCROLLBAR" ScrollBar
			Window 001E1D2E "" MozillaContentWindowClass
				Window 00061E5E "" MozillaWindowClass
					Window 000E1DBA "FAKETRACKPOINTSCROLLBAR" ScrollBar
				Window 000B1DD6 "FAKETRACKPOINTSCROLLBAR" ScrollBar
			Window 00241D44 "FAKETRACKPOINTSCROLLBAR" ScrollBar
		Window 00111D54 "FAKETRACKPOINTSCROLLBAR" ScrollBar

And there are some warnings:
WARNING: NS_ENSURE_TRUE(browserChrome) failed: file e:/mozilla-build/src/mozilla/docshell/base/nsDocShell.cpp, line 10290
WARNING: Something wrong when creating the docshell for a frameloader!: file e:/mozilla-build/src/mozilla/content/base/src/nsFrameLoader.cpp, line 1043
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file e:/mozilla-build/src/mozilla/content/base/src/nsFrameLoader.cpp, line 1067
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file e:/mozilla-build/src/mozilla/content/base/src/nsFrameLoader.cpp, line 220
pldhash: for the table at address 079F22B8, the given entrySize of 48 probably favors chaining over double hashing.
++DOCSHELL 079F2250 == 9
++DOMWINDOW == 10 (079F2D80) [serial = 10] [outer = 00000000]
search "file" not found - skipping.
WARNING: Subdocument container has no frame: file e:/mozilla-build/src/mozilla/layout/base/nsDocumentViewer.cpp, line 2337

Left panel seems to be destroyed along with the plugin window.

Comment 37

[Zane U. Ji]

(In reply to comment #36)
> Left panel seems to be destroyed along with the plugin window.
Typo. Should be right panel.

Comment 38

[Rudi Sherry]

This has been fixed for a while in Reader 8 and 9.  This is related to tabs and windows, and specifically changing the parent HWND that contains the PDF HWND.  For technical reasons that predate me (and some are still valid) we need to "subclass" the top-level window in order to capture various keystrokes and menu commands that otherwise Firefox would not send us, even if we are the focus.

We subclass by replacing the wndproc and calling the previous one if it's not something we think we need to handle.  When the PDF is closed, we un-subclass.  We find the parent HWND by, of course, GetParent(). However, the original code makes the assumption that for the life of the PDF, the parent HWND won't change so we assumed, when it's time to un-subclass, we can call GetParent again and we'll get the same one.  Unfortunately, when this was different, we left the wndproc alone... that means the wndproc of the parent HWND pointed into our plug-in code.

Shortly after that, the plug-in was unloaded, and the parent HWND now has a wndproc pointing to garbage... hence the crasher.

This means that changes in Firefox can actually affect this: I don't know what makes the parent HWND change, but that started happening in 3.0 (IIRC); that is, in Firefox 2 we never saw this issue.  I'm guessing that the later version of Firefox are now manipulating HWNDs in a way that is making it much more likely that the parent has chnaged, and so tickling this bug much more often.

Again, the fix for users is to update to the latest version.

We now (finally!) have the version number in the plug-in description so the plugin version checking code can be completed: if there is no version, the user needs to update (currently latest versions are 8.2.2 and 9.3.2, and both have the version in the plug-in description).

Comment 39

[Zane U. Ji]

(In reply to comment #38)
Thank you for your information.

> This means that changes in Firefox can actually affect this: I don't know what
> makes the parent HWND change, but that started happening in 3.0 (IIRC); that
> is, in Firefox 2 we never saw this issue.  I'm guessing that the later version
> of Firefox are now manipulating HWNDs in a way that is making it much more
> likely that the parent has chnaged, and so tickling this bug much more often.
> 
Because FF resets pulg-in window's parent before destroy it.

The parent window that has been subclassed used to be destroyed soon after plug-in is unloaded. So it won't have a chance to process any message. However, it remains in this case. Destroying that parent window should fix the problem.

Comment 40

[Zane U. Ji]

Attachment: Patch

Do cleanup for PDF plug-in because we don't give it a chance to do that. It is detached before being destroyed.

Comment 41

[Josh Aas]

Comment on attachment 440472 [details] [diff] [review]
Patch

Why are you moving the code that sets mPluginType?

+  // Old PDF plug-ins subclass parent window, bug 531551

Please document the version of the plugin that has the fix in the code.

It seems to me that this might not fix the problem for out-of-process plugins. Am I correct, and if so can you fix that?

Comment 42

[Josh Aas]

When we find out exactly what version of the plugin has the fix, and if it is old enough, we might want to consider just blocklisting everything prior to the version with the fix.

Comment 43

[Josh Aas]

Comment on attachment 440472 [details] [diff] [review]
Patch

Also, I'm not a very good reviewer for this type of Windows code. Moving review to jmathies.

Comment 44

[Rudi Sherry]

This was fixed in 9.1, quite some time ago, and a bit later in 8 (I don't know the exact version).  Does blocklisting give a URL where the user can go to get an explanation of updating?  This could work the same way as the plugincheck code...

Comment 45

[Zane U. Ji]

Attachment: Patch

(In reply to comment #41)
> Please document the version of the plugin that has the fix in the code.
> 
Done.

> It seems to me that this might not fix the problem for out-of-process plugins.
> Am I correct, and if so can you fix that?

I found out that FF v3.7a5pre (20100419), which uses plugin-container.exe to load plug-ins, works fine. PDF plug-in does subclass that window. However, FF somehow subclasses it again.

(In reply to comment #42)
> we might want to consider just blocklisting everything prior to the
> version with the fix.

Sounds right to me.

Comment 46

[Rudi Sherry]

The plugins for 9.3.2 and 8.2.2 now have the version in plugin description; you
can assume that any plug-in without a version number is out-of-date.

The URL to go to for non-current versions is:

http://www.adobe.com/go/acrobat_reader_updates

Comment 48

[Jim Mathies [:jimm]]

Comment on attachment 441055 [details] [diff] [review]
Patch

> +  // check plugin mime type and cache whether it is Flash or not
> +  // Flash will need special treatment later

Update these comments to better reflect what the code is doing.

>  nsresult nsPluginNativeWindowWin::CallSetWindow(nsCOMPtr..
>  {
> 
>    if (!aPluginInstance) {
>      UndoSubclassAndAssociateWindow();
>      mPrevWinProc = NULL;
>    }
>  
> ..
> 
> +
> +  // PDF plug-in v7.0.9, v8.1.3, and v9.0 subclass parent window, bug 531551
> +  // V8.2.2 and V9.1 don't have such problem.
> +  if (nsPluginType_PDF == mPluginType) {
> +    HWND parent = ::GetParent((HWND)window);
> +    if (mParentWnd != parent) {
> +      NS_ASSERTION(!mParentWnd, "Plug-in's parent window changed");
> +      mParentWnd = parent;
> +      mParentProc = ::GetWindowLongPtr(mParentWnd, GWLP_WNDPROC);
> +    }
> +  }
> 

I think you need |if (!aPluginInstance)| wrapping around this. You end up resetting the parent window procedure twice, the first time you reset pdf's subclass, the second time you try to set the desktop's window procedure to null in UndoSubclassAndAssociateWindow. This happens when !aPluginInstance, UndoSubclassAndAssociateWindow is called clearing the pdf's subclass, then you fall through and set mParentWnd to the desktop HWND. 

> nsresult nsPluginNativeWindowWin::UndoSubclassAndAssociateWindow()
> 
> ..
> 
> +  if (nsPluginType_PDF == mPluginType && mParentWnd) {
> +    ::SetWindowLongPtr(mParentWnd, GWLP_WNDPROC, mParentProc);
> +    mParentWnd = NULL;
> +    mParentProc = NULL;
> +  }
> +

Please reverse |nsPluginType_PDF == mPluginType| in both cases.

Comment 49

[Zane U. Ji]

Attachment: Updated patch

Jim, thanks for the review.

Comment 50

[Jim Mathies [:jimm]]

Comment on attachment 443370 [details] [diff] [review]
Updated patch

Thanks!

Comment 52

[Mitchell Field [:Mitch]]

What's going on here? Does this topcrash-fixing patch still need to be checked-in?

Comment 55

[Benjamin Smedberg]

jmathies, can you shepherd this to completion?

Comment 57

[Daniel Kirsch]

I don't really know what the patch really fix, so just for the record:
While it is easy for most private users to update their acrobat plugin, it is usually not for users in a company. We encouraged a lot of our customers to use Firefox and for them changing anything (even just updating an existing version) often means a tremendous effort. They don't really much care about why Firefox crashes, they will switch back to a browser which does not.
So I hope that old plugin version will not just be blocked but the cause for the crash will be fixed or a workaround can be found.

Comment 58

[Fernando Hartmann]

I arrived here from a constantly crash (id 7a02ddeb-36c3-4c68-9b6b-f05132100825) that I have opening pages with Java applets, as I'm using FF 4b4, I'm no sure if it started on the java update (u20 to u21) or in a FF beta update. But this happens almost all the time when I access a page with java applet.
I saw int crash comments that are other peoples that complains with about java applets pages.
There is another bug specific to Java ?
There are some test that I can do to help to understand what is the problem ?

Comment 59

[Fernando Hartmann]

One more information, I use Win 7 and the crash happens with UAC activated, with UAC deactivated the browser hangs for some time (timeout) and shows the page with the plugin crash information, but the FF steal working well.

Comment 60

[Kevin Brosnan]

This particular bug report is about outdated versions of Adobe Reader. If you are seeing this crash visit https://www.mozilla.com/en-US/plugincheck/ and make sure all the installed plugins you have are up to date and disable any unknown plugins you are not using via tools > addons > plugins.

Comment 61

[Jim Mathies [:jimm]]

still applies cleanly. I'll land this after we reopen the tree post beta 5.

Comment 62

[Jim Mathies [:jimm]]

Attachment: merged to tip

Comment 63

[Jim Mathies [:jimm]]

http://hg.mozilla.org/mozilla-central/rev/5da673421ec0

Comment 64

[Daniel Veditz [:dveditz]]

Comment on attachment 472646 [details] [diff] [review]
merged to tip

missed the 1.9.2.11 release, maybe next time. Did the trunk check-in make a measurable difference in the crash rate for this signature?

Comment 65

[chris hofmann]

> Did the trunk check-in make a measurable difference in the crash rate for this signature?

hard to say since volume is low  and bumpy for UserCallWinProcCheckWow signatures on the trunk.

running about 6-21 crashes per day both before and after sept 7.

Comment 66

[LegNeato]

a=LegNeato for 1.9.2.12. Please land only on the mozilla-1.9.2 default branch, *not* the relbranch.

Comment 67

[Reed Loden [:reed]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/463cdfd0371c