Closed Bug 265599 Opened 17 years ago Closed 16 years ago

crash in [@ GtkPromptService::Prompt]

Categories

(Core Graveyard :: Embedding: GTK Widget, defect)

x86
Linux
defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: chpe, Assigned: chpe)

References

()

Details

(Keywords: fixed1.7.13)

Attachments

(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.2) Gecko/20040803 Galeon/1.3.17.99
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.3) Gecko/20041019 Firefox/1.0

GtkPromptService::Prompt assumes that if aCheckMsg != nsnull then also
aCheckValue != nsnull. But on aviary branch, nsSingleSignonPrompt::Prompt passes
a .get() on an nsAutoString but only sets aCheckValue conditionally,
http://lxr.mozilla.org/aviarybranch/source/toolkit/components/passwordmgr/base/nsSingleSignonPrompt.cpp#64

Reproducible: Always
Steps to Reproduce:
1. Load the testcase
2. Move mouse over the "Open this link in new tab" link
3. Wait 6 seconds for the testcase's timeout

Actual Results:  
Crash.

Expected Results:  
Prompt pops up.

Trace, unfortunately no debug build:

#0  0x001b6e54 in GtkPromptService::Prompt () from ./libgtkembedmoz.so
#1  0x00278447 in nsPrompt::Prompt () from
/opt/firefox/lib/firefox-1.0/components/libembedcomponents.so
#2  0x00e9e571 in nsSingleSignonPrompt::Prompt () from
/opt/firefox/lib/firefox-1.0/components/libtoolkitcomps.so
#3  0x01457f28 in GlobalWindowImpl::Prompt () from
/opt/firefox/lib/firefox-1.0/components/libgklayout.so
#4  0x014582b4 in GlobalWindowImpl::Prompt () from
/opt/firefox/lib/firefox-1.0/components/libgklayout.so
#5  0x006ee20f in XPTC_InvokeByIndex () from ./libxpcom.so
#6  0x002da477 in XPCWrappedNative::CallMethod () from
/opt/firefox/lib/firefox-1.0/components/libxpconnect.so
#7  0x002e0572 in XPC_WN_CallMethod () from
/opt/firefox/lib/firefox-1.0/components/libxpconnect.so
#8  0x0094fd0e in js_Invoke () from ./libmozjs.so
#9  0x00959508 in js_Interpret () from ./libmozjs.so
#10 0x00950352 in js_Execute () from ./libmozjs.so
#11 0x009304b2 in JS_EvaluateUCScriptForPrincipals () from ./libmozjs.so
#12 0x0144e518 in nsJSContext::EvaluateString () from
/opt/firefox/lib/firefox-1.0/components/libgklayout.so
#13 0x0145f21f in GlobalWindowImpl::RunTimeout () from
/opt/firefox/lib/firefox-1.0/components/libgklayout.so
#14 0x0145f99f in GlobalWindowImpl::TimerCallback () from
/opt/firefox/lib/firefox-1.0/components/libgklayout.so
#15 0x006d9dae in nsTimerImpl::Fire () from ./libxpcom.so
#16 0x006d9e1b in handleTimerEvent () from ./libxpcom.so
#17 0x006d5fc7 in PL_HandleEvent () from ./libxpcom.so
#18 0x006d5ef9 in PL_ProcessPendingEvents () from ./libxpcom.so
#19 0x006d7832 in nsEventQueueImpl::ProcessPendingEvents () from ./libxpcom.so
#20 0x00c6c63e in event_processor_callback () from
/opt/firefox/lib/firefox-1.0/components/libwidget_gtk2.so
#21 0x0599fa2f in g_vasprintf () from /usr/lib/libglib-2.0.so.0
#22 0x05978e9b in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
#23 0x0597aac4 in g_main_context_acquire () from /usr/lib/libglib-2.0.so.0
#24 0x0597ad8a in g_main_loop_run () from /usr/lib/libglib-2.0.so.0
#25 0x00a9ec33 in gtk_main () from /usr/lib/libgtk-x11-2.0.so.0
#26 0x08049ebb in main ()

The crash happens in GtkPromptService.cpp on this line
http://lxr.mozilla.org/aviarybranch/source/embedding/browser/gtk/src/GtkPromptService.cpp#176
:
176         prompter.SetCheckValue(*aCheckValue);

because aCheckValue is 0.
I forgot to mention that you have to load the testcase in TestGtkEmbed, not in
firefox.

As for the fix, should we
- fix the assumption in GtkPromptService, or
- fix nsSingleSignonPrompt to pass nsnull instead of "" when aCheckValue is also
nsnull ?
-> marco
Assignee: blizzard → marco
Attached patch fix (obsolete) — Splinter Review
Attachment #163301 - Flags: superreview?(blizzard)
Attachment #163301 - Flags: review?(marco)
See also bug 199421 where the same change was made to ::ConfirmEx.
Attached patch updated patch (obsolete) — Splinter Review
Also make sure aCheckValue isn't 0 in ::AlertCheck and ::ConfirmCheck.
Attachment #163301 - Attachment is obsolete: true
Attachment #163425 - Flags: superreview?(blizzard)
Attachment #163425 - Flags: review?(marco)
Attachment #163301 - Flags: superreview?(blizzard)
Attachment #163301 - Flags: review?(marco)
Attachment #163425 - Flags: review?(marco) → review+
Comment on attachment 163425 [details] [diff] [review]
updated patch

Nitpick of the day, you should probably put a new line between NS_ENSURE_ARG
and EmbedPrompter (AlertCheck and ConfirmCheck)
Attached patch nit fixedSplinter Review
Attachment #163425 - Attachment is obsolete: true
Attachment #163425 - Flags: superreview?(blizzard)
Comment on attachment 163429 [details] [diff] [review]
nit fixed

Transferring r=marco, and asking for sr.
Attachment #163429 - Flags: superreview?(blizzard)
Attachment #163429 - Flags: review+
Still happening, so confirming. Can we get an sr=, please? :)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attachment #163429 - Flags: superreview?(blizzard) → superreview+
Attachment #163429 - Flags: approval1.8b4?
Attachment #163429 - Flags: approval1.8b4?
Attachment #163429 - Flags: approval1.7.11?
Attachment #163429 - Flags: approval-aviary1.0.7?
Comment on attachment 163429 [details] [diff] [review]
nit fixed

This is also needed on trunk. Fixes an embedding-only crash, no risk.
Attachment #163429 - Flags: approval1.8b4?
Attachment #163429 - Flags: approval1.8b4? → approval1.8b4+
Summary: crash in GtkPromptService::Prompt on aviary branch → crash in [@ GtkPromptService::Prompt]
Checking in embedding/browser/gtk/src/GtkPromptService.cpp;
/cvsroot/mozilla/embedding/browser/gtk/src/GtkPromptService.cpp,v  <-- 
GtkPromptService.cpp
new revision: 1.8; previous revision: 1.7
done
Assignee: mpgritti → chpe
Marking fixed.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Attachment #163429 - Flags: approval1.7.11? → approval1.7.11-
Attachment #163429 - Flags: approval1.7.12?
Comment on attachment 163429 [details] [diff] [review]
nit fixed

a=dveditz for drivers
Attachment #163429 - Flags: approval1.7.13?
Attachment #163429 - Flags: approval1.7.13+
Attachment #163429 - Flags: approval-aviary1.0.8?
Attachment #163429 - Flags: approval-aviary1.0.8+
Comment on attachment 163429 [details] [diff] [review]
nit fixed

mozilla/embedding/browser/gtk/src/GtkPromptService.cpp 	1.6.16.1 	MOZILLA_1_7_BRANCH
mozilla/embedding/browser/gtk/src/GtkPromptService.cpp 	1.6.32.1 	AVIARY_1_0_1_20050124_BRANCH
verified fixed on the 1.0.1 branch using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.12) Gecko/20060212 Firefox/1.0.7. Running the testcase URL produces no crash. Adding verified-aviary1.0.8 keyword.
verified fixed on the 1.0.1 branch using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.12) Gecko/20060212 Firefox/1.0.7. Running the testcase URL produces no crash. Adding verified-aviary1.0.8 keyword.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.