Closed Bug 744992 Opened 12 years ago Closed 12 years ago

crash in mozilla::widget::TaskbarPreview::MainWindowHook mainly with toolbar@web.de

Categories

(Core :: Widget: Win32, defect)

12 Branch
x86
Windows 7
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla15
Tracking Status
firefox12 + ---
firefox13 + verified
firefox14 --- verified

People

(Reporter: marcia, Assigned: mkaply)

References

Details

(4 keywords, Whiteboard: [startupcrash][qa+])

Crash Data

Attachments

(3 files)

This bug was filed from the Socorro interface and is 
report bp-e89ae536-d019-47b8-bc77-ebd582120412 .
============================================================= 

Seen while looking at Beta 4 crash stats. This was the #9 overall crash in Beta 4 but did not seem to have a bug. https://crash-stats.mozilla.com/report/list?signature=mozilla::widget::TaskbarPreview::MainWindowHook%28void*,%20HWND__*,%20unsigned%20int,%20unsigned%20int,%20long,%20long*%29

All of the comments seem to be in German. 

Going to hunt down addon/module correlations but it might be related to toolbar@web.de.

Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::widget::TaskbarPreview::MainWindowHook 	widget/windows/TaskbarPreview.cpp:456
1 	xul.dll 	mozilla::widget::WindowHook::CallbackData::Invoke 	widget/windows/WindowHook.cpp:153
2 	xul.dll 	nsWindow::ProcessMessage 	
3 	xul.dll 	nsWindow::WindowProcInternal 	widget/windows/nsWindow.cpp:4450
4 	xul.dll 	CallWindowProcCrashProtected 	xpcom/base/nsCrashOnException.cpp:65
5 	xul.dll 	nsWindow::WindowProc 	widget/windows/nsWindow.cpp:4392
6 	user32.dll 	InternalCallWinProc 	
7 	user32.dll 	UserCallWinProcCheckWow 	
8 	user32.dll 	DispatchMessageWorker 	
9 	user32.dll 	DispatchMessageW 	
10 	xul.dll 	nsAppShell::ProcessNextNativeEvent 	widget/windows/nsAppShell.cpp:344
11 	xul.dll 	nsBaseAppShell::OnProcessNextEvent 	widget/xpwidgets/nsBaseAppShell.cpp:306
12 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:619
13 	nspr4.dll 	PR_Unlock 	nsprpub/pr/src/threads/combined/prulock.c:347
14 	xul.dll 	MessageLoop::RunHandler 	ipc/chromium/src/base/message_loop.cc:201
15 	xul.dll 	_SEH_epilog4 	
16 	xul.dll 	MessageLoop::Run 	ipc/chromium/src/base/message_loop.cc:175
17 	xul.dll 	mozilla::scache::canonicalizeBase 	startupcache/StartupCacheUtils.cpp:155
18 		@0x32336c64 	
19 	xul.dll 	nsAppStartup::Run 	toolkit/components/startup/nsAppStartup.cpp:220
20 	xul.dll 	XRE_main 	toolkit/xre/nsAppRunner.cpp:3537
21 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp:107
22 	firefox.exe 	firefox.exe@0x4033 	
23 	firefox.exe 	__tmainCRTStartup 	crtexe.c:594
24 	firefox.exe 	_SEH_epilog4 	
25 	kernel32.dll 	BaseThreadInitThunk 	
26 	ntdll.dll 	__RtlUserThreadStart 	
27 	ntdll.dll 	WinSqmSetIfMaxDWORD 	
28 	ntdll.dll 	_RtlUserThreadStart 	
29 	firefox.exe 	pre_c_init 	crtexe.c:304
30 	firefox.exe 	pre_c_init 	crtexe.c:304
31 		@0xfffddfff
Nominating for tracking since there seems to be a significant spike from B3->B4 in this signature. B3 had 55 crashes, B4 had 1095 crashes. This crash appeared in Firefox 11 and it looks like in the last week there were 2255 crashes in FF 11 in this signature.
Keywords: topcrash
I went looking for the webde toolbar, and I found http://produkte.web.de/browser/ which seems to be a customized version of Firefox that has a version of the webde toolbar installed.
It happens only on Windows 7.
OS: Windows NT → Windows 7
Whiteboard: [startupcrash]
#6 on 11.*, #3 on 12.*, #7 on 13.*, #3 on 14.* in yesterday's data. This appeared out of nowhere across the board. Is is this a website thing, an add-on, or some other third party triggering that crash?
Crash Signature: [@ mozilla::widget::TaskbarPreview::MainWindowHook(void*, HWND__*, unsigned int, unsigned int, long, long*)] → [@ mozilla::widget::TaskbarPreview::MainWindowHook(void*, HWND__*, unsigned int, unsigned int, long, long*)] [@ mozilla::widget::TaskbarPreview::MainWindowHook(void*, HWND__*, unsigned int, unsigned __int64, __int64, __int64*)]
It's correlated to several versions of toolbar@web.de on 11.0:
  mozilla::widget::TaskbarPreview::MainWindowHook(void*, HWND__*, unsigned int, unsigned int, long, long*)|EXCEPTION_ACCESS_VIOLATION_WRITE (2986 crashes)
    100% (2972/2986) vs.   3% (4056/139826) toolbar@web.de
          0% (0/2986) vs.   0% (3/139826) 1.7
          0% (0/2986) vs.   0% (4/139826) 1.7.1
          0% (0/2986) vs.   0% (16/139826) 1.7.2
          0% (0/2986) vs.   0% (2/139826) 1.7.3
         35% (1041/2986) vs.   1% (1714/139826) 1.7.5
          1% (18/2986) vs.   0% (30/139826) 2.0.3
          0% (1/2986) vs.   0% (26/139826) 2.0.6
         64% (1912/2986) vs.   2% (2261/139826) 2.1

I think it's related to the Windows patch Tuesday on April 10th.
Summary: crash in mozilla::widget::TaskbarPreview::MainWindowHook → crash in mozilla::widget::TaskbarPreview::MainWindowHook mainly with toolbar@web.de
Firefox B5 has not been officially shipped yet and this is already the top crash on that version from early adopters, so I am bit concerned about how this is rising even in the latest version.
When I installed the Firefox browser from http://produkte.web.de/browser/ on Mac, it has the 1.7.5 version of the webde toolbar installed. So far I was not able to find a separate version of the toolbar that I could install and I am not sure how they are getting Version 2.1.
Tracking for releases until we figure out whether or not FF12/13 is any more susceptible to this crash than FF11.
I was able to reproduce this in the QA lab using a Windows 7 x64 machine. Here are the steps I followed:

1. Start with a fresh profile and one of the 12 betas (I believe I used B3)
2. Download web.de toolbar V 1.51 from http://web-de-toolbar.softonic.de/
3. After installing, go to Firefox addons manager and perform an "check for update" to update the plugin to the latest version - I believe Version 2.1
4. After I restarted Firefox I crash repeatedly on startup.
5. One other possible factor is that while I was testing I had downloaded an update to the latest beta and after installing the extension update, at the same time a software update to the latest beta was being applied. I will retest with a static beta to see if the same thing happens after installing the extension or whether software update plays into it as well.

https://crash-stats.mozilla.com/report/index/bp-4eba28df-cdaa-483e-81fc-321892120413
Keywords: reproducible
(In reply to Marcia Knous [:marcia] from comment #10)
> I was able to reproduce this in the QA lab using a Windows 7 x64 machine.
> Here are the steps I followed:

I'm including Jorge and Kev on this bug since it sounds like we may need to blocklist. A couple of questions:

* Does FF11 run into the same startup crash given the STR in Comment 10?
* What version of Win7 did you reproduce on? We should confirm this isn't related to Patch Tuesday.
* I couldn't find any mention of v2.1 of the toolbar on Google. When does version 2.1 first show up in crash reports?
* Why is version 1.7.5 similarly affected? That version was presumably released before 2.1, but this startup crash didn't show up on our radars prior to this week.

We basically need to figure out if the spike in crashes is related to Patch Tuesday, a specific FF release, or a new version of the add-on.
I am still testing to get some of the answers to the the questions Alex poses in Comment 11.

I left out one additional step that I performed in Comment 10. After the Toolbar installs I did check off all the boxes (make web.de default search, etc) on the startup page that launches after you install the extension.

Version 2.1 of the toolbar did not seem to be readily available in a Google search. I found V 1.5 and V 1.7 and was able to update to V 2.1 using the addon software update.

I reproduced it on Windows 7 machine running Win 7 Professional, SP1 x64 bit OS. 

I have http://support.microsoft.com/kb/2656368 installed which was released on the 4/10 Patch Tuesday, so it appears this machine has all of the Patch Tuesday update.
I haven't yet been able to reproduce the crash in a stock Firefox 11 build with no update involved. I am also having trouble reproducing it on the original machine that I reproduced it on to begin with.

I noticed during some additional testing that when I check for extension updates it also finds an extension update for the Feedback extension (Latest version 1.2.1). So I know for a fact when testing in Comment 10, there was a perfect storm of:

(1) web.de toolbar updated from V 1.5.1 to V 2.1
(2) feedback extension updated to V 1.2.1
(3) software updated from an earlier version of beta

When the browser tried to restart after those steps it got in the startup crash death loop.
Note that even if we block the add-on, it's likely that the block won't apply to those already experiencing startup crashes. This would only protect those using it who aren't crashing.
reaching out to my webde contact, but note that web.de is very popular in Germany, so blocklisting this too fast here would have affect on a large user base in germany
(In reply to Marcia Knous [:marcia] from comment #8)
> So far I was not able to find a separate version of the toolbar that I could 
> install and I am not sure how they are getting Version 2.1.
(In reply to Marcia Knous [:marcia] from comment #12)
> Version 2.1 of the toolbar did not seem to be readily available in a Google
> search.
Here is the download link:
https://produkte.web.de/mailcheck/firefox/?mc=freemail@mailfooter@intern@mailcheck.produkte@mailcheck
(In reply to Carsten Book [:Tomcat] from comment #15)
> reaching out to my webde contact, but note that web.de is very popular in
> Germany, so blocklisting this too fast here would have affect on a large
> user base in germany

If we block too slowly, we're at risk of losing all of our Win7 web.de toolbar users. From [1] it's obvious that this is affecting FF11 users as well, so I don't think there's anything we can do on our side to resolve the issue. In the past week alone, we've seen 33652 crashes. Of those, 70% are startup crashes. When Marcia was able to repro, she couldn't get out of the crash cycle. Let's be conservative and say that each user tried open the browser 5 times. That means that we've probably already lost (.7*33652)/5~=4711 users (not taking into consideration any throttling we do on crashes).

Let's make sure to notify the web.de guys, and strongly consider the blocklist as of Monday if this is still a top crasher at that time.

[1] https://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=contains&query=mozilla%3A%3Awidget%3A%3ATaskbarPreview%3A%3AMainWindowHook&reason_type=contains&date=04%2F14%2F2012%2018%3A27%3A51&range_value=1&range_unit=weeks&hang_type=any&process_type=any&do_query=1&signature=mozilla%3A%3Awidget%3A%3ATaskbarPreview%3A%3AMainWindowHook%28void*%2C%20HWND__*%2C%20unsigned%20int%2C%20unsigned%20int%2C%20long%2C%20long*%29
Just looked at crash stats - This is the #2 top crash on Firefox 11 with over 31K in crashes in the last week, and it is #2 on Beta 5 which just shipped Friday. It also appears in Firefox 10.0.2 data as the #2 top crash in the last week.
got a first reply from my contact and they are investigating this issue
Is there someone who can reliably reproduce this bug and provide repro steps?
(In reply to mozdev from comment #20)
> Is there someone who can reliably reproduce this bug and provide repro steps?

hi, comment#10 contains some steps to reproduce, will try this also now
Thanks for notifying us.

A few responses to the above:
* Download of toolbar:
  * Public website:
    http://produkte.web.de/mailcheck/firefox/?mc=freemail@hilfe@toolbar.produkte@browser
    http://dl.web.de/toolbar/firefox/download_w/webde_toolbar.xpi
  * Update RDF
    https://dl.web.de/toolbar/firefox/toolbar.rdf
    (in response to this bug, we have already rolled the update.rdf to 1.7.5)
    2.0 branch: https://dl.web.de/toolbar/firefox/toolbar20.rdf
  * Direct version-specific download URLs, gathered from update.rdf:
    https://dl.web.de/toolbar/firefox/update_w/webde_toolbar.1.7.2.xpi
    https://dl.web.de/toolbar/firefox/update_w/webde_toolbar.1.7.5.xpi
    https://dl.web.de/toolbar/firefox/update_w/webde_toolbar.2.0.6.xpi
    https://dl.web.de/toolbar/firefox/update_w/webde_toolbar.2.1.xpi
* We have 2 branches: 1.7.x for one user group, and 2.0 and 2.1 for users of our new webmail frontend, which should be pretty much everybody by now.

* I am unclear from the comments above whether FF11 or only FF12 is affected. Most comments talk about betas, but then some people say that FF11 is also affected. Then again, the relative low number of crashes compared to the huge number of installations suggests that there is some limiting factor to the bug.

* One person from our group says that he saw the bug on his machine only when he also had Firebug installed.

* Given that this is reportedly a crash on startup, and a restart loop: How would we ever be able to update these users to a fixed version of the addon?

* Given that our extension is pure JS, this must be a Firefox bug that we merely trigger.

* You can find the taskbar code when you unzip the XPI, and look in content/email/taskbar.js .
I will attach the file for convenience. It is based on <https://github.com/sid0/overlay-extension>.
sid0, see above.

FWIW, I heard about the bug 2 hours ago, so please excuse that we respond only now.
(In reply to Dev Web.de Firefox Addon from comment #22)
> * I am unclear from the comments above whether FF11 or only FF12 is
> affected.
It affects Fx 9, 10, 11, 12 Beta, Aurora, and Nightly. It first appeared in these versions around April, 11th.

> * Given that our extension is pure JS, this must be a Firefox bug that we merely > trigger.
It would have appeared with a new Firefox version in that case. In addition, there's no JS code in the stack.
(In reply to Scoobidiver from comment #25)
> > * Given that our extension is pure JS, this must be a Firefox bug that we merely > trigger.
> It would have appeared with a new Firefox version in that case. In addition,
> there's no JS code in the stack.

Well, we're crashing in a callback from Windows, so I wonder what's going on. Looking.
(In reply to Dev Web.de Firefox Addon from comment #22)
> * I am unclear from the comments above whether FF11 or only FF12 is
> affected.

As Scoobidiver has pointed out already, all versions from Firefox 9 through 14 are affected, including 11.

> Then again, the relative low number of crashes compared to
> the huge number of installations suggests that there is some limiting factor
> to the bug.

On releases, you need to multiply the listed numbers by 10 as we only process 10% of the Firefox release crashes.

> * Given that our extension is pure JS, this must be a Firefox bug that we
> merely trigger.

That's very probable, and we should find out what it is and fix it (looks like Siddarth is looking into that). In the meantime, it probably still would be a good idea if you guys could work around it so that our common users have less pain with it. :)
> On releases, you need to multiply the listed numbers by 10 as we only
> process 10% of the Firefox release crashes.

OK, that's an important bit of info, thanks.
Also, while there might be several reports from one user, they are often marked duplicate. OTOH, users might choose not to submit.
So, we're somewhere in the 6-digit range.

> It first appeared in these versions around April, 11th.

Our version 2.1 was released on April 12. Also, while most of the reports are with web.de toolbar, not all of them are. This further suggests that this is a general problem.

> good idea if you guys could work around it

Definitely! I've already made a 2.1.1, which is identical to 2.1, but with the new taskbar code disabled. It's in our QA right now. We might release it tonight CET.
I can't reproduce this at all with Firefox 12.0b5 and toolbar version 2.1...
(In reply to Dev Web.de Firefox Addon from comment #28)
> Our version 2.1 was released on April 12. Also, while most of the reports
> are with web.de toolbar, not all of them are. This further suggests that
> this is a general problem.
When the crash happens too soon, Breakpad is not enable to detect installed extensions, so the correlation could be 100%.

> Definitely! I've already made a 2.1.1, which is identical to 2.1, but with
> the new taskbar code disabled. It's in our QA right now. We might release it
> tonight CET.
35% of crashes happen with version 1.7.5. Are you going to release version 1.7.5.1 also?
Replace enable by able in comment 30.
> I can't reproduce this at all with Firefox 12.0b5 and toolbar version 2.1...

2 people here could reproduce this only with:
* Windows 7 (x64)
* Web.de Toolbar 2.1
* Firebug
As soon as they disabled Firebug, the crash went away.

I don't have a Win7 VM to test with, but I'll set one up now.
(In reply to Scoobidiver from comment #6)
> It's correlated to several versions of toolbar@web.de on 11.0:
>   mozilla::widget::TaskbarPreview::MainWindowHook(void*, HWND__*, unsigned
> int, unsigned int, long, long*)|EXCEPTION_ACCESS_VIOLATION_WRITE (2986
> crashes)
>     100% (2972/2986) vs.   3% (4056/139826) toolbar@web.de

Please give me the URL where you saw this. I cannot interpret these number (e.g. "vs.").
(In reply to Dev Web.de Firefox Addon from comment #32)
> > I can't reproduce this at all with Firefox 12.0b5 and toolbar version 2.1...
> 
> 2 people here could reproduce this only with:
> * Windows 7 (x64)
> * Web.de Toolbar 2.1
> * Firebug
> As soon as they disabled Firebug, the crash went away.
> 
> I don't have a Win7 VM to test with, but I'll set one up now.

i was also not able to reproduce with a new Windows 7 VM and fresh profile etc and the steps to reproduce here. Was this Firebug a specific version or just the latest?
And in case it wasn't obvious, Siddarth, we pretty much took this code directly from your example.

https://github.com/sid0/overlay-extension
OK, so I haven't managed to repro yet, but here's my guess about what's happening.

At http://hg.mozilla.org/releases/mozilla-release/annotate/b967d9c07377/widget/src/windows/TaskbarPreview.cpp#l455, window is null.

Here's a possible scenario in which this could have happened:

1. A window is opened.

2. Your code creates the icon controller...

3. ... which causes the constructor at http://hg.mozilla.org/releases/mozilla-release/annotate/b967d9c07377/widget/src/windows/TaskbarWindowPreview.cpp#l83 to be called.

4. Since the taskbar icon isn't ready yet, we can't make taskbar calls. So we add a hook to Windows itself at line 103.

5. The window's closed/closing before the taskbar icon is ready, or something else happens that either
a) causes SetNSWindowPtr to be called with null: http://hg.mozilla.org/releases/mozilla-release/file/6504535d557b/widget/src/windows/nsWindow.cpp#l989, or
b) causes preview->mWnd to be set to null.

6. Windows finally fires the notification for which we added the hook.

7. window is null at TaskbarPreview.cpp#l455, hence we crash.

I guess we should be a bit more resilient and skip executing the hook when window = null at line 455. Note that NS_ASSERTION only works in debug mode.

Rob, Rob, what do you think?
or probably even when preview->mWnd is null.
(In reply to Dev Web.de Firefox Addon from comment #33)
> Please give me the URL where you saw this. I cannot interpret these number
> (e.g. "vs.").
vs. means versus. It means there were 2986 crashes with this crash signature in 11.0 on April 13rd, among them 2972 crashes with Web.de toolbar installed. There were also 139826 crashes this day, among them 4056 crashes with Web.de toolbar installed.
Ans as was pointed out, version 1.7.5 does not have any of the code to do task bar preview.

Are you sure that:

         35% (1041/2986) vs.   1% (1714/139826) 1.7.5

Is exactly the same crash?

The only change in the 1.7.5 release was to remove a button. And it's been out for a while.
(In reply to Carsten Book [:Tomcat] from comment #34)
> (In reply to Dev Web.de Firefox Addon from comment #32)
> > > I can't reproduce this at all with Firefox 12.0b5 and toolbar version 2.1...
> > 
> > 2 people here could reproduce this only with:
> > * Windows 7 (x64)
> > * Web.de Toolbar 2.1
> > * Firebug
> > As soon as they disabled Firebug, the crash went away.
> > 
> > I don't have a Win7 VM to test with, but I'll set one up now.
> 
> i was also not able to reproduce with a new Windows 7 VM and fresh profile
> etc and the steps to reproduce here. Was this Firebug a specific version or
> just the latest?

I'm able to reproduce (not on VM).
Steps to repro:
OS: Windows 7 professional 64bit
Installed Firefox 11
Installed Firebug 1.9.1 => Firebug: active on all websites, all modules active
Installed WEB.DE toolbar 2.1, restart => crash
(In reply to Michael Kaply (mkaply) from comment #39)
> Are you sure that:
>          35% (1041/2986) vs.   1% (1714/139826) 1.7.5
> Is exactly the same crash?
Yes. It's the same crash signature:
https://crash-analysis.mozilla.com/crash_analysis/20120416/20120416_Firefox_11.0-interesting-addons-with-versions.txt.gz
and the same stack: bp-db26f590-95fa-43d4-8a09-d73c82120416.
With respect to the Firebug crash, I have verified that if I turn off the taskbar code specifically in webde, we don't crash.

The specific panel in Firebug that causes the crash is the Script panel.
I can reproduce now, but only with Firebug *and* Firebug enabled (all modules, all websites). I crashed only with Firebug. This is important for reproduction and investigation, but most crash reports do not list Firebug, so this is not a required factor, there's something else that triggers it, too.
There must be something wrong with the data.

On 20120412 there were no crashes in TaskbarPreview::MainWindowHook at all.

When the crashes show up on 20120413, there are 1.7.5 crashes, but 1.7.5 was not updated between April 12 and April 13. Only 2.1 was. 

1.7.5 users were updated to 2.1 (I think).

And 1.7.5 definitely does not contain the taskbar code.
I have a 2.1.1 with taskbar code disabled that fixes the crash for me. It passed our QA. We will probably want to ship this to end-users in 3 hours.

However, given that I crash on startup, the only way to install the fix was to start firefox from the commandline with "firefox.exe -safe-mode".

We don't know how we can deliver the fix (without user interaction) to the users. If anybody knows a trick, please let us know. We urgently need this in the next 2 hours.
We have found another piece of information.

the crash does not happen if you run Firefox as an administrator.

To me, this is looking more and more like something that Microsoft changed with their April 12 security update that has broken Firefox.
(In reply to Dev Web.de Firefox Addon from comment #45)
> We don't know how we can deliver the fix (without user interaction) to the
> users. If anybody knows a trick, please let us know. We urgently need this
> in the next 2 hours.

i guess the first step would be blocklisting the "old" crashing extension and then letting users updating the extension via the update feature. Also this would prevent startup crashes i guess
We're releasing the 2.1.1 without taskbar code right now. This should get at least those users in safe waters who are not yet crashing at startup.

For those users who are currently crashing at startup, *neither* ext update *nor* blocklist *nor* firefox self-update will be fetched and none of that will help them.

For those users, we currently have the following options:
* Start Firefox as admin. Bug doesn't appears in this case. Then update extensions manually. Restart Firefox. We would not like to recommend this to end-users.
* Start Firefox in safe mode by pressing SHIFT while clicking on the Firefox icon. Disable extensions in the safe mode dialog. Then update extensions manually. Restart Firefox. We will write a tech doc with this for users.
* We are also publishing an (official) bundle of full "Firefox WEB.DE Edition" with toolbar as installer EXE. There, we can include any fix we want. Then ask affected users to download and install the EXE.

Problem with all these solutions is that they require manual user interaction, which means we need to reach them first. We don't have exact records of which users have this toolbar. Also, the users will not understand that WEB.DE is somehow related to this, they blame Firefox, so they won't search for "firefox web.de crash", but "firefox crash", which makes it harder to publish instructions.

We are still looking for a way to fix the profiles of users who are currently crashing at startup, without user interaction.
> We're releasing the 2.1.1 without taskbar code right now. This should get at least
> those users in safe waters who are not yet crashing at startup.

https://dl.web.de/toolbar/firefox/toolbar.rdf
https://dl.web.de/toolbar/firefox/update_w/webde_toolbar.2.1.1.xpi
On April/16/ at 18.00 (GMT+1) the version 2.1.1 (without the taskbar code) was staged and shipped to end users. This version can be downloaded under
http://dl.web.de/toolbar/firefox/download_w/webde_toolbar.xpi
(In reply to Scoobidiver from comment #6)
>     100% (2972/2986) vs.   3% (4056/139826) toolbar@web.de
>          35% (1041/2986) vs.   1% (1714/139826) 1.7.5
>          64% (1912/2986) vs.   2% (2261/139826) 2.1
> 
> I think it's related to the Windows patch Tuesday on April 10th.

(The other versions of our toolbar are marginal in the crash stats, because they have no userbase anymore.)

What's puzzling me is that the crash also happens on 1.7.5 (which still has some population). We didn't have any taskbar code in that version, it was introduced in 2.1.

This suggests that some other factor is triggering this, too. I have no clue why our 1.7.5 toolbar would make this crash more likely.

It could be a severe error in the crash reporting, but I'd rather not go with that assumption.
Adding Honza, given that Firebug appears to be a factor triggering this crash.
Comment on attachment 615671 [details] [diff] [review]
fix - doesn't fix the crash

I think there should be a comment here because the assertion above the if check implies the only reason you would get a null window is because you are in an embedded context.
Comment on attachment 615671 [details] [diff] [review]
fix - doesn't fix the crash

I can't really vouch for the correctness, but it looks good to me.
Attachment #615671 - Flags: review+
Should the nsIWinTaskbar ID be revved so that a developer can determine if they have the "safe" API?

@mozilla.org/windows-taskbar;1

to

@mozilla.org/windows-taskbar;2
> @mozilla.org/windows-taskbar;2

+1

This allows developers to use the API without checking for a Firefox version to avoid crashing.
I don't think many ext are using this yet, and most or all of them would need to pay attention for this bug, so they have to act anyway.
So I believe there is something wrong with your data, and I'm not even sure how we would go about debugging it. Looking at today's numbers:

  mozilla::widget::TaskbarPreview::MainWindowHook(void*, HWND__*, unsigned int, unsigned int, long, long*)|EXCEPTION_ACCESS_VIOLATION_WRITE (8782 crashes)

   100% (8750/8782) vs.   7% (10378/146527) toolbar@web.de
          0% (0/8782) vs.   0% (2/146527) 1.7
          0% (8/8782) vs.   0% (9/146527) 1.7.1
          0% (16/8782) vs.   0% (29/146527) 1.7.2
          0% (3/8782) vs.   0% (4/146527) 1.7.3
         19% (1698/8782) vs.   1% (2049/146527) 1.7.5
          0% (27/8782) vs.   0% (32/146527) 2.0.3
          0% (0/8782) vs.   0% (28/146527) 2.0.6
         80% (6997/8782) vs.   6% (8174/146527) 2.1
          0% (1/8782) vs.   0% (51/146527) 2.1.1

We have verified that there is no task bar code in any 1.7 version that we shipped.

Yet you show crashes at this address in 1.7.1,1.7.2,1.7.3 and especially 1.7.5.

So somehow crash data for our 2.X products are getting confused with crashes with 1.7.

I understand that you can point me at a report (https://crash-stats.mozilla.com/report/index/db26f590-95fa-43d4-8a09-d73c82120416) that shows 1.7.5 crashing, but to be blunt it's simply impossible.

How does crash reporter grab the extensions that are installed on the system?
Try run for 4fda8f968b34 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=4fda8f968b34
Results (out of 2 total builds):
    success: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mozilla.BenB@bucksch.org-4fda8f968b34
Try run for 3bf02f2bb85b is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=3bf02f2bb85b
Results (out of 1 total builds):
    success: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mozilla.BenB@bucksch.org-3bf02f2bb85b
Comment on attachment 615671 [details] [diff] [review]
fix - doesn't fix the crash

Our QA ran tests with the try build above, and confirms that even the toolbar version 2.1 doesn't crash with this fix.
Comment on attachment 615671 [details] [diff] [review]
fix - doesn't fix the crash

> Our QA ran tests with the try build above, and confirms that even the toolbar version 2.1
> doesn't crash with this fix.

Sorry, there was a misunderstanding, I have to retract that.
The build with the fix here with toolbar 2.1 *does* crash.

So, this patch does *not* fix the bug. Unfortunately, I'm also at a loss as to why it doesn't fix it.
Comment on attachment 615671 [details] [diff] [review]
fix - doesn't fix the crash

Based on last comment, revoking review.
Attachment #615671 - Flags: review+ → review-
Attachment #615671 - Attachment description: fix → fix - doesn't fix the crash
Checking for the null hwnd is not enough.

We need to not go into the preview code if it is null.

I also removed the assertion because it's not a correct statement (this code happens in a non embedded case as well)
Attachment #617101 - Flags: review?(sagarwal)
Comment on attachment 617101 [details] [diff] [review]
Complete fix for problem

This looks fine but I'm not qualified to review it.
Attachment #617101 - Flags: review?(tellrob)
Attachment #617101 - Flags: review?(sagarwal)
Attachment #617101 - Flags: review?(robert.bugzilla)
Attachment #617101 - Flags: feedback+
Oh, you might also want to check if preview->mWnd is null.
Comment on attachment 617101 [details] [diff] [review]
Complete fix for problem

Jim, has worked on this code before
Attachment #617101 - Flags: review?(robert.bugzilla) → review?(jmathies)
Can we please have a review for this? I want this on FF13 at least, given that we missed FF12. :(
Attachment #617101 - Flags: review+
Comment on attachment 617101 [details] [diff] [review]
Complete fix for problem

[Approval Request Comment]
Regression caused by (bug #):
Taskbar API

User impact if declined:
If anything uses the taskbar icon API, browser crashes on startup, leaving the user with a broken, non-updateable Firefox.
Attachment #617101 - Flags: approval-mozilla-beta?
Attachment #617101 - Flags: approval-mozilla-aurora?
Comment on attachment 617101 [details] [diff] [review]
Complete fix for problem

[Triage Comment]
Definitely seems like we should get this in early and verify the fix as well as resolve the user pain here.
Attachment #617101 - Flags: approval-mozilla-beta?
Attachment #617101 - Flags: approval-mozilla-beta+
Attachment #617101 - Flags: approval-mozilla-aurora?
Attachment #617101 - Flags: approval-mozilla-aurora+
Looks like we're still waiting for an r+ from jmathies. Assigning to him. This approval may have been premature.
Assignee: nobody → jmathies
Keywords: checkin-needed
Attachment #617101 - Flags: approval-mozilla-beta?
Attachment #617101 - Flags: approval-mozilla-beta+
Attachment #617101 - Flags: approval-mozilla-aurora?
Attachment #617101 - Flags: approval-mozilla-aurora+
Keywords: checkin-needed
Comment on attachment 617101 [details] [diff] [review]
Complete fix for problem

Review of attachment 617101 [details] [diff] [review]:
-----------------------------------------------------------------

sorry, been on pto. Added null checks are always ok with me. :)

::: widget/windows/TaskbarPreview.cpp
@@ +454,3 @@
>  
> +      if (preview->mVisible)
> +        preview->UpdateTaskbarProperties();

nit - please wrap this.
Attachment #617101 - Flags: review?(tellrob)
Attachment #617101 - Flags: review?(jmathies)
Attachment #617101 - Flags: review+
Keywords: checkin-needed
Comment on attachment 617101 [details] [diff] [review]
Complete fix for problem

Approving now that jmathies' review is in.
Attachment #617101 - Flags: approval-mozilla-beta?
Attachment #617101 - Flags: approval-mozilla-beta+
Attachment #617101 - Flags: approval-mozilla-aurora?
Attachment #617101 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/27ade28a07ca
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [startupcrash] → [startupcrash][qa+]
(In reply to mozdev from comment #40)
> I'm able to reproduce (not on VM).
> Steps to repro:
> OS: Windows 7 professional 64bit
> Installed Firefox 11
> Installed Firebug 1.9.1 => Firebug: active on all websites, all modules
> active
> Installed WEB.DE toolbar 2.1, restart => crash

I was able to reproduce the problem both on Win 7 32/64-bit.
I see no crashes on FF 13b3:
Mozilla/5.0 (Windows NT 6.1; rv:13.0) Gecko/20100101 Firefox/13.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20100101 Firefox/13.0

Marking the bug as verified fixed.
Verified fixed on FF 14b6:
Mozilla/5.0 (Windows NT 6.1; rv:14.0) Gecko/20100101 Firefox/14.0 
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20100101 Firefox/14.0
Status: RESOLVED → VERIFIED
Depends on: 800498
There are still crashes in this function. I filed bug 1053839.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: