452689 - Crash in [@ nsPrefBranch::SetComplexValue] passing RSS feed URL to FeedDemon

Status: VERIFIED FIXED

(Firefox Graveyard :: RSS Discovery and Preview, defect)

Description

[Stephen Donner [:stephend] Not actively reading bugmail]

Build ID: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.17) Gecko/2008082714 Firefox/2.0.0.17 (1st candidate)

Summary: Crash in [@ nsPrefBranch::SetComplexValue] passing RSS feed URL to Newsgator

Steps to Reproduce:

1. Install FeedDemon 2.7 from http://www.newsgator.com/Individuals/Downloads.aspx?product=FeedDemon
2. Load http://www.bloglines.com
3. Click on the RSS feed icon in the URL bar
4. From the dropdown, select "Choose application..." from the pulldown
5. Choose the Bloglines executable
6. Click "Subscribe"

Crash...

Comment 1

[Samuel Sidler (old account; do not CC)]

$10 says it's a regression from bug 360529. This is a regression that we should fix for 1.8.1.17.

--

Incident ID: 49144368
Stack Signature	nsPrefBranch::SetComplexValue c27d20f8
Product ID	Firefox2
Build ID	2008082714
Trigger Time	2008-08-28 14:52:06.0
Platform	Win32
Operating System	Windows NT 5.1 build 2600
Module	firefox.exe + (000dd510)
URL visited	
User Comments	Crashing with newsgator
Since Last Crash	124 sec
Total Uptime	124 sec
Trigger Reason	Access violation
Source File, Line No.	c:/builds/tinderbox/Fx-Mozilla1.8-Release/WINNT_5.2_Depend/mozilla/modules/libpref/src/nsPrefBranch.cpp, line 390
Stack Trace 	
nsPrefBranch::SetComplexValue  [mozilla/modules/libpref/src/nsPrefBranch.cpp, line 390]
nsPrefService::SetComplexValue  [mozilla/modules/libpref/src/nsPrefService.h, line 61]
XPCWrappedNative::CallMethod  [mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 2169]
XPC_WN_CallMethod  [mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp, line 1455]
js_Invoke  [mozilla/js/src/jsinterp.c, line 1387]
js_Interpret  [mozilla/js/src/jsinterp.c, line 3966]
js_Invoke  [mozilla/js/src/jsinterp.c, line 1406]
nsXPCWrappedJSClass::CallMethod  [mozilla/js/src/xpconnect/src/xpcwrappedjsclass.cpp, line 1453]
nsXPCWrappedJS::CallMethod  [mozilla/js/src/xpconnect/src/xpcwrappedjs.cpp, line 468]
SharedStub  [mozilla/xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp, line 147]
nsEventListenerManager::HandleEventSubType  [mozilla/content/events/src/nsEventListenerManager.cpp, line 1655]
nsEventListenerManager::HandleEvent  [mozilla/content/events/src/nsEventListenerManager.cpp, line 1762]
nsXULElement::HandleDOMEvent  [mozilla/content/xul/content/src/nsXULElement.cpp, line 2237]
PresShell::HandleDOMEventWithTarget  [mozilla/layout/base/nsPresShell.cpp, line 6641]
nsButtonBoxFrame::DoMouseClick  [mozilla/layout/xul/base/src/nsButtonBoxFrame.cpp, line 181]
nsButtonBoxFrame::MouseClicked  [mozilla/layout/xul/base/src/nsButtonBoxFrame.h, line 61]
PresShell::HandleEventInternal  [mozilla/layout/base/nsPresShell.cpp, line 6583]
PresShell::HandleEventWithTarget  [mozilla/layout/base/nsPresShell.cpp, line 6439]
nsEventStateManager::CheckForAndDispatchClick  [mozilla/content/events/src/nsEventStateManager.cpp, line 3207]
nsEventStateManager::PostHandleEvent  [mozilla/content/events/src/nsEventStateManager.cpp, line 2170]
PresShell::HandleEventInternal  [mozilla/layout/base/nsPresShell.cpp, line 6614]
PresShell::HandleEvent  [mozilla/layout/base/nsPresShell.cpp, line 6377]
nsViewManager::HandleEvent  [mozilla/view/src/nsViewManager.cpp, line 2566]
nsViewManager::DispatchEvent  [mozilla/view/src/nsViewManager.cpp, line 2253]
HandleEvent  [mozilla/view/src/nsView.cpp, line 174]
nsWindow::DispatchEvent  [mozilla/widget/src/windows/nsWindow.cpp, line 1319]
nsWindow::DispatchMouseEvent  [mozilla/widget/src/windows/nsWindow.cpp, line 6329]
ChildWindow::DispatchMouseEvent  [mozilla/widget/src/windows/nsWindow.cpp, line 6576]
nsWindow::WindowProc  [mozilla/widget/src/windows/nsWindow.cpp, line 1507]
USER32.dll + 0x8734 (0x7e418734)
USER32.dll + 0x8816 (0x7e418816)
USER32.dll + 0x89cd (0x7e4189cd)
USER32.dll + 0x8a10 (0x7e418a10)
nsAppShell::Run  [mozilla/widget/src/windows/nsAppShell.cpp, line 159]
nsAppStartup::Run  [mozilla/toolkit/components/startup/src/nsAppStartup.cpp, line 152]
main  [mozilla/browser/app/nsBrowserApp.cpp, line 61]
kernel32.dll + 0x16fd7 (0x7c816fd7)

Comment 2

[Samuel Sidler (old account; do not CC)]

Note: this is 100% reproducible on Windows but not reproducible on Mac.

Comment 3

[Stephen Donner [:stephend] Not actively reading bugmail]

s/Bloglines executable/NewsGator executable.

Comment 4

[Stephen Donner [:stephend] Not actively reading bugmail]

 Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.17pre) Gecko/2008082503 BonEcho/2.0.0.17pre is fine, implicating bug 360529; I've set a dependency.

Comment 5

[Daniel Veditz [:dveditz]]

Gavin's testing a patch

Comment 6

[Martijn Wargers (dead)]

I guess backporting the patch for bug 352666 might help fix the crash on Firefox 2 branch, but I don't know if there is an underlying issue here that needs to be fixed.

Comment 7

[:Gavin Sharp [email: [e-mail]]]

As far as I can tell there are two problems with the branch patch in bug 360529:
1) it's missing parts of attachment 311366 [details] [diff] [review] (the parts that remove selectedApplicationItemWrapped/defaultSystemReaderItemWrapped).

2) Part of the merge doesn't account for bug 400064 not having landed there (it adds references to getPrefAppForType, which doesn't exist on the branch).

I thought that 1) was the original cause of this bug, since we get try to get ".file" off selectedApplicationItemWrapped, but had never set it there, but when I fixed that to use _selectedApp as on the trunk, things were still crashing.

It turns out that the trunk and branch are both broken here, because of a problem with the original patch. _setSelectedHandler is called from the pref observer that monitors both the PREF_SELECTED_APP and PREF_SELECTED_READER prefs, and it resets _selectedApp. So when we call setCharPref here:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/components/feeds/src/FeedWriter.js&rev=1.56&mark=1299#1299

the pref observer calls _setSelectedHandler, which fails to get the pref for the default app handler (because it doesn't exist by default), which means _selectedApp is cleared, so when we next try to setComplexValue using _selectedApp, we fail. The easy way to fix this is to set the selected app pref first, to avoid having other pref changes wipe out _selectedApp.

Comment 8

[:Gavin Sharp [email: [e-mail]]]

Attachment: branch patch

This brings the branch into sync with the trunk. It doesn't fix the crash because bug 352666 isn't landed there (bug 352666 is the only reason we're not crashing on trunk).

Comment 9

[:Gavin Sharp [email: [e-mail]]]

Attachment: trunk&branch patch

(This applies on top of the previous patch, on the branch.)

On the trunk, the symptoms of this bug are that you can't open RSS feeds in external readers, unless you've previously configured the external app as a default in the preferences pane.

Comment 10

[:Gavin Sharp [email: [e-mail]]]

Seems like we should fix the functionality regression on the trunk, too (even though we shipped 3.0 with it - how'd that get missed!?).

Comment 11

[:Gavin Sharp [email: [e-mail]]]

As far as I can tell this is a cross platform bug. I have no idea why people couldn't reproduce the crash on Mac.

Comment 12

[Asaf Romano (gone)]

Comment on attachment 335990 [details] [diff] [review]
branch patch

r=mano, thanks.

Comment 13

[Asaf Romano (gone)]

Note that I intentionally didn't do comment 7 part 1, but given that's it's not part of the API, I don't mind.

Comment 14

[Samuel Sidler (old account; do not CC)]

Comment on attachment 335991 [details] [diff] [review]
trunk&branch patch

Let's get this landed on the 1.9.0 branch and the 1.9.0.2 relbranch asap.

Comment 15

[Samuel Sidler (old account; do not CC)]

Comment on attachment 335990 [details] [diff] [review]
branch patch

And... let's get this landed on the 1.8 branch and the 1.8.1.17 relbranch asap.

Comment 16

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 335991 [details] [diff] [review]
trunk&branch patch

Need this patch on the branch too.

Comment 17

[Samuel Sidler (old account; do not CC)]

Comment on attachment 335991 [details] [diff] [review]
trunk&branch patch

Oh, right. Approval all around!

Comment 18

[:Gavin Sharp [email: [e-mail]]]

Branch (and branch relbranch):
mozilla/browser/components/feeds/src/FeedWriter.js 	1.2.2.34.2.2
mozilla/browser/components/feeds/src/FeedWriter.js 	1.2.2.36
mozilla/browser/components/feeds/src/FeedWriter.js 	1.2.2.34.2.1
mozilla/browser/components/feeds/src/FeedWriter.js 	1.2.2.35

Comment 19

[:Gavin Sharp [email: [e-mail]]]

Trunk and trunk relbranch:
mozilla/browser/components/feeds/src/FeedWriter.js 	1.56.10.1
mozilla/browser/components/feeds/src/FeedWriter.js 	1.57

Comment 20

[:Gavin Sharp [email: [e-mail]]]

http://bonsai.mozilla.org/cvsquery.cgi?branch=&branchtype=match&date=explicit&mindate=2008-08-29+08%3A24&maxdate=2008-08-29+08%3A50

Comment 21

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED on the 2.0.0.x branch, using:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.17) Gecko/2008082909 Firefox/2.0.0.17

-and-

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.17) Gecko/2008082909 Firefox/2.0.0.17

(never could repro on OS X 10.4, for some inane reason.)

Replacing fixed1.8.1.17 with verified1.8.1.17.

Comment 22

[Tracy Walker [:tracy]]

verified: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.2) Gecko/2008082911 Firefox/3.0.2

Comment 23

[Daniel Veditz [:dveditz]]

This bug is not resolved FIXED, but has been verified on the 1.8.1 and 1.9.0 branches. Does it also need to be checked in to mozilla-central, or is it fully resolved now?

Comment 24

[:Gavin Sharp [email: [e-mail]]]

I confirmed yesterday that mozilla-central also needs the second fix (attachment 335991 [details] [diff] [review]) - I need to check it in there.

Comment 25

[:Gavin Sharp [email: [e-mail]]]

http://hg.mozilla.org/mozilla-central/rev/b9f6e51be24f

Comment 26

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED on trunk using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1pre) Gecko/20080904035000 Minefield/3.1b1pre

Comment 27

[Al Billings [:abillings - ex-MoCo]]

Trying to verify this (again?) for 1.8.1.18 and the repro steps don't seem to work. I see no RSS icon on bloglines. Stephen, do you have up to date steps or can you verify this with a 1.8 nightly?

Comment 28

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.18pre) Gecko/2008102103 BonEcho/2.0.0.18pre

Comment 29

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.4pre) Gecko/2008102405 GranParadiso/3.0.4pre

Replacing fixed1.9.0.4 keyword with verified1.9.0.4.