The default bug view has changed. See this FAQ.

Restart after extension update check fails [@ nsContentUtils::IsCallerChrome]

RESOLVED FIXED in mozilla1.8.1beta2

Status

()

Core
Document Navigation
RESOLVED FIXED
11 years ago
6 years ago

People

(Reporter: zach, Assigned: smaug)

Tracking

(5 keywords)

1.8 Branch
mozilla1.8.1beta2
PowerPC
Mac OS X
crash, fixed1.8.0.7, regression, relnote, verified1.8.1
Points:
---
Bug Flags:
blocking1.8.1 +
blocking1.8.0.7 +

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(1 attachment)

(Reporter)

Description

11 years ago
STR - 
1. Using a 1.5.x profile, install one or more extensions not compatible with 2.0 (i.e. Live HTTP Headers from http://livehttpheaders.mozdev.org/)
2. Launch Bon Echo to upgrade your profile
3. When the Extension Update window opens up, choose "Don't Check" for updates

Instead of disabling the incompatible extensions and launching normally, the browser quits. On second launch, startup proceeds normally. 

This appears to be mac only. I can reproduce it dependably on Mac, but the issue does not occur on Windows.
It does this on all OS's and it is just noticeable on Mac OS X because of the dock and on upgrade we always perform a restart due to there being several extension install / upgrade scenarios that have to be accounted for.
Just to clarify: Zach: by "on second launch, startup proceeds normally", do you mean that it restarts automatically, or do you mean "if I launch it again myself, it works"? Sounds like this bug is describing the restart not working correctly, not the restart itself.
(Reporter)

Comment 3

11 years ago
Just talked to Rob and I'll try to clarify things a bit. What this bug is about is that the restart doesn't happen successfully: the browser quits to restart, and does not come back up again. After manually relaunching the app, it works fine.  
(Reporter)

Comment 4

11 years ago
After further investigation, the restart fails to happen whether or not we check for extension updates. 

Furthermore, it turns out that we actually crash when we're trying to restart; talkback isn't triggered, but OS X generates a CrashReporter log. Here's the stack:

Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_PROTECTION_FAILURE (0x0002) at 0x00000000

Thread 0 Crashed:
0   org.mozilla.firefox 	0x0021c0fc nsContentUtils::IsCallerChrome() + 32
1   org.mozilla.firefox 	0x001505b0 PresShell::HandleEventInternal(nsEvent*, nsIView*, unsigned, nsEventStatus*) + 508
2   org.mozilla.firefox 	0x001502a0 PresShell::HandleEvent(nsIView*, nsGUIEvent*, nsEventStatus*, int, int&) + 1300
3   org.mozilla.firefox 	0x00216698 nsViewManager::HandleEvent(nsView*, nsGUIEvent*, int) + 236
4   org.mozilla.firefox 	0x00215a88 nsViewManager::DispatchEvent(nsGUIEvent*, nsEventStatus*) + 3356
5   org.mozilla.firefox 	0x00244afc nsGlobalWindow::Deactivate() + 348
6   org.mozilla.firefox 	0x0046d9bc nsWebShellWindow::HandleEvent(nsGUIEvent*) + 816
7   org.mozilla.firefox 	0x006dc0d8 nsWindow::DispatchEvent(nsGUIEvent*, nsEventStatus&) + 172
8   org.mozilla.firefox 	0x006dc164 nsWindow::DispatchWindowEvent(nsGUIEvent&) + 32
9   org.mozilla.firefox 	0x006d64c8 nsMacEventDispatchHandler::DispatchGuiEvent(nsWindow*, unsigned) + 112
10  org.mozilla.firefox 	0x006d67e8 nsMacEventDispatchHandler::SetDeactivated(nsWindow*) + 232
11  org.mozilla.firefox 	0x006d7d98 nsMacEventHandler::HandleActivateEvent(OpaqueEventRef*) + 352
12  org.mozilla.firefox 	0x00326478 nsMacWindow::WindowEventHandler(OpaqueEventHandlerCallRef*, OpaqueEventRef*, void*) + 540
13  com.apple.HIToolbox 	0x931ea554 DispatchEventToHandlers(EventTargetRec*, OpaqueEventRef*, HandlerCallRec*) + 692
14  com.apple.HIToolbox 	0x931e9cac SendEventToEventTargetInternal(OpaqueEventRef*, OpaqueEventTargetRef*, HandlerCallRec*) + 372
15  com.apple.HIToolbox 	0x931e9b28 SendEventToEventTargetWithOptions + 40
16  com.apple.HIToolbox 	0x9322e848 PostActivateEvent + 340
17  com.apple.HIToolbox 	0x9322dd6c HiliteAndActivateWindow + 360
18  com.apple.HIToolbox 	0x9322daf4 AdjustToNewWindowActivation(WindowData*, WindowContext*, OpaqueWindowPtr*, WindowData*) + 156
19  com.apple.HIToolbox 	0x932fbb74 HideWindow + 192
20  org.mozilla.firefox 	0x00326920 nsMacWindow::Show(int) + 1096
21  org.mozilla.firefox 	0x0046fc78 nsXULWindow::Destroy() + 284
22  org.mozilla.firefox 	0x0046e93c nsWebShellWindow::Destroy() + 280
23  org.mozilla.firefox 	0x000826d8 nsAppShellService::Observe(nsISupports*, char const*, unsigned short const*) + 64
24  libxpcom_core.dylib 	0x2c00f13c nsObserverService::NotifyObservers(nsISupports*, char const*, unsigned short const*) + 432
25  libxpcom_core.dylib 	0x2c007a20 NS_ShutdownXPCOM_P + 164
26  org.mozilla.firefox 	0x0000e150 ScopedXPCOMStartup::~ScopedXPCOMStartup [unified]() + 60
27  org.mozilla.firefox 	0x00012660 XRE_main + 4280
28  org.mozilla.firefox 	0x0000d768 start + 432
29  org.mozilla.firefox 	0x0000d5e8 start + 48
Summary: "Don't Check" for extension updates causes browser to quit → Restart after extension update check fails
(Reporter)

Comment 5

11 years ago
I've narrowed this down to fallout from bug 336978--we don't crash without that patch and we do crash with it. CC'ing some relevant folks from that bug. 

This isn't an Extension Manager bug anymore...Guessing Docshell?
Component: Extension/Theme Manager → Embedding: Docshell
Keywords: regression
Product: Firefox → Core
Version: 2.0 Branch → 1.8 Branch
Flags: blocking1.8.1?

Updated

11 years ago
Assignee: nobody → Olli.Pettay
Severity: normal → critical
Keywords: crash
Summary: Restart after extension update check fails → Restart after extension update check fails [@ nsContentUtils::IsCallerChrome]
Zach, could you check whether this also affects Mac OS X using 1.5.0.5 since bug 336978 also landed on 1.8.0.5?

Updated

11 years ago
Blocks: 336978
(Reporter)

Comment 7

11 years ago
'twas already on it Rob. Interestingly enough, this does not affect 1505, just 2.0. I haven't tried the trunk yet.
So this is not Mac only?
Zach, could you test the first patch in bug 336978. Does that make
any difference comparing to the second patch which was checked in.
Severity: critical → normal
(Reporter)

Comment 9

11 years ago
Nope. We crash with the first patch as well. 

If you want me to try anything, ping me on irc (as 'zach').
I just tested this on the RC2 Windows build (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060710 Firefox/2.0b1) and did not see the problem Zach is reporting on the Mac. I used Live HTTP headers as my test extension.
Can we get a confirmation on the Mac with a clean profile? Need to know how serious this is for blocking status.
(Reporter)

Comment 12

11 years ago
Marcia and Sheppy are both seeing this on Mac, and I have been using a clean profile. Sheppy has done some diagnosis and I understand a patch is coming real soon now. 

Updated

11 years ago
Flags: blocking1.8.1? → blocking1.8.1+

Updated

11 years ago
Flags: blocking1.8.1+ → blocking1.8.1?
Created attachment 228834 [details] [diff] [review]
fix for IsCallerChrome() to check for null sSecurityManager [checked in on 1.8branch]

This patch simply makes IsCallerChrome() check for a NULL sSecurityManager -- the routine gets called after shutting down the nsContentUtils object, and that causes the sSecurityManager to be deleted.  With the patch, it just returns PR_FALSE if the pointer is null, which is just what happens if an error occurs anyway.
Attachment #228834 - Flags: review?(jst)
This is branch-only? The patch for bug 331117 should have prevented this on trunk.
I've not tried it on trunk.  That does sound like the fix for bug 331117 would prevent this problem too.
Sure enough, this crash doesn't occur on trunk.  I guess it comes down to whether we migrate the fix for bug 331117 to branch.
According to TB there are crashes @nsContentUtils::IsCallerChrome also in 1.8.0 branch, even before the patch in bug 336978 went in.
(Reporter)

Comment 18

11 years ago
*** Bug 344268 has been marked as a duplicate of this bug. ***
I originally wanted to, and dbaron was concerned about the ramifications; I'd be happy to prepare a branch version and ask for approval.
I think the patch I did isn't a bad idea to have anyway, regardless of whether the fix for bug 331117 gets migrated to the branch or not.  But one or both really ought to happen prior to release of Fx 2.
QA Contact: extension.manager → docshell

Updated

11 years ago
Flags: blocking1.8.1? → blocking1.8.1+
Keywords: relnote
Target Milestone: --- → mozilla1.8.1beta2
*** Bug 344404 has been marked as a duplicate of this bug. ***
Comment on attachment 228834 [details] [diff] [review]
fix for IsCallerChrome() to check for null sSecurityManager [checked in on 1.8branch]

r+sr=jst for this change for the branch if we chose to take this as a spot-fix for this problem on the branch.
Attachment #228834 - Flags: superreview+
Attachment #228834 - Flags: review?(jst)
Attachment #228834 - Flags: review+
Attachment #228834 - Flags: approval1.8.1?

Updated

11 years ago
Attachment #228834 - Flags: approval1.8.1? → approval1.8.1+

Updated

11 years ago
Whiteboard: [checkin needed (1.8 branch)]
Ok, bug 331117 now got fixed on the 1.8.1 branch, is the "fix for IsCallerChrome() to check for null sSecurityManager" patch still necessary for branch?
I'd like QA to verify that crashes are no longer reproducable, but I'd say this is no longer needed.
Keywords: qawanted
But the null check should be in 1.8.0 branch.
Is this crash happening on the 1.8.0 branch?
(In reply to comment #26)
> Is this crash happening on the 1.8.0 branch?
> 

Apparently not this one, but see #17
Attachment #228834 - Flags: approval1.8.0.6?
(Reporter)

Comment 28

11 years ago
Tested against 1.8 branch build 2006071803 on Mac PPC. The crashes do seem to be fixed. 

That being said, talkback data does indicate that we are crashing in IsCallerChrome() with a different stack, and adding the null check seems relatively harmless, so it seems worth landing this on the branches anyway. 
Keywords: qawanted → verified1.8
Comment on attachment 228834 [details] [diff] [review]
fix for IsCallerChrome() to check for null sSecurityManager [checked in on 1.8branch]

I've committed this additional safety patch on the 1.8 branch.
Attachment #228834 - Attachment description: fix for IsCallerChrome() to check for null sSecurityManager → fix for IsCallerChrome() to check for null sSecurityManager [checked in on 1.8branch]
Should this be marked fixed1.8.1?

Comment 31

11 years ago
I corrected the verified1.8 keyword to be verified1.8.1
Keywords: verified1.8 → verified1.8.1
Whiteboard: [checkin needed (1.8 branch)]
(Reporter)

Comment 32

11 years ago
My mistake. This should be marked verified1.8.1. It has not yet landed on 1.8.0.
Whiteboard: [checkin needed (1.8 branch)]
(Reporter)

Updated

11 years ago
Flags: blocking1.8.0.6?
Whiteboard: [checkin needed (1.8 branch)]
Comment on attachment 228834 [details] [diff] [review]
fix for IsCallerChrome() to check for null sSecurityManager [checked in on 1.8branch]

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #228834 - Flags: approval1.8.0.7? → approval1.8.0.7+
Flags: blocking1.8.0.7? → blocking1.8.0.7+
Fixed on MOZILLA_1_8_0_BRANCH. Marking fixed because I don't see this on trunk (bug 331117 should have fixed all the occurences I know of).
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Keywords: fixed1.8.0.7
Resolution: --- → FIXED
Crash Signature: [@ nsContentUtils::IsCallerChrome]
You need to log in before you can comment on or make changes to this bug.