103250 - Highlighting text during page load - trunk topcrash [@ IsScriptEnabled ]

Status: VERIFIED FIXED

(Core :: DOM: Selection, defect, P1)

Description

[Jan Carpenter]

This bug is a topcrasher for the Trunk
Added [@ IsScriptEnabled ] for tracking.  

Here are some URLs & Comments from recent crashes:
     (36238953)	Comments: Selecting text while a new page is being loaded
     (36238396)	URL: http://www.redhat.com
     (36238396)	Comments: Select text while a document is being loaded
     (36238344)	URL: http://www.redhat.co
     (36216086)	URL: http://www.limi.net/projects/cmf
     (36245574) copying url from url bar to chatzilla

Here is a recent stack trace:


         IsScriptEnabled()
	 nsHTMLCopyEncoder::Init()
	 nsCopySupport::HTMLCopy()
	 nsAutoCopyService::NotifySelectionChanged()
	 nsTypedSelection::NotifySelectionListeners()
	 nsSelection::NotifySelectionListeners()
	 nsSelection::SetMouseDownState()
	 nsFrame::HandleRelease()
	 nsFrame::HandleEvent()
	 PresShell::HandleEventInternal()
	 PresShell::HandleEvent()
	 nsView::HandleEvent()
	 nsViewManager::DispatchEvent()
	 HandleEvent()
	 nsWidget::DispatchEvent()
	 nsWidget::DispatchWindowEvent()
	 nsWidget::DispatchMouseEvent()
	 nsWidget::OnButtonReleaseSignal()
	 nsWindow::HandleGDKEvent()
	 dispatch_superwin_event()
	 handle_gdk_event()
	 libgdk-1.2.so.0 + 0x17b7f (0x4033fb7f)
	 libglib-1.2.so.0 + 0x11987 (0x40373987)
	 libglib-1.2.so.0 + 0x12001 (0x40374001)
	 libglib-1.2.so.0 + 0x121cc (0x403741cc)
	 libgtk-1.2.so.0 + 0x93843 (0x40289843)
	 nsAppShell::Run()
	 nsAppShellService::Run()
	 main1()
	 main()
	 libc.so.6 + 0x1bf31 (0x404b7f31)

Comment 1

[Jan Carpenter]

adding keywords

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This code was added for bug 97687 and the crash started at the exact same time.

->peterv

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

OK, the checkin that almost definitely caused this crash is on the branch,
according to comments in bug 97687.  For the past 2 days this is the #1 Linux
topcrash on the *trunk* (it's Linux-only), so it's probably overwhelming for the
branch, unless the branch hasn't gotten stable yet.

Comment 4

[Peter Van der Beken [:peterv]]

I 
haven't 
been 
able 
to 
reproduce 
this 
yet. 
The 
code 
is 
at 
http://lxr.mozilla.org/seamonkey/source/content/base/src/nsDocumentEncoder.cpp#2
02. I don't see anything obvious from a quick look at the code.
Looking.

Comment 5

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Well, it seems one null check was forgotten, this might be the cause. We don't
check if we got globalObject before using it.

+  nsCOMPtr<nsIScriptGlobalObject> globalObject;
+  aDoc->GetScriptGlobalObject(getter_AddRefs(globalObject));
+
+  nsCOMPtr<nsIScriptContext> scriptContext;
+  globalObject->GetContext(getter_AddRefs(scriptContext));

Comment 6

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Attachment: Probable fix

Comment 7

[Jaime Rodriguez, Jr.]

Pls get the reviews, and let's look at it in today's PDT meeting.

Comment 8

[vidur (gone)]

Comment on attachment 52562 [details] [diff] [review]
Probable fix

sr=vidur

The NS_ENSURE_TRUE will assert in a debug build. If that happens frequently enough, hopefully we'll be pushed into figuring out why there's no global object in this case.

Comment 9

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Checked in on the trunk. Those that were able to reproduce, please test and
report...

I have emailed [e-mail] to get permission to check this onto the
0.9.5 branch.

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Looking at the disassembly in talkback incident 36357853 (as an example), the
crash occurs here:

40b04272 8b3e             mov     edi,[esi]         <====CRASH HERE
40b04274 8d45e8           lea     eax,[ebp-0x18]
40b04277 50               push    eax
40b04278 e8072aeaff       call    409a6c84
40b0427d 83c404           add     esp,0x4
40b04280 50               push    eax
40b04281 56               push    esi
40b04282 8b4718           mov     eax,[edi+0x18]
40b04285 ffd0             call    eax
40b04287 83c408           add     esp,0x8
40b0428a 837de800         cmp     dword ptr [ebp-0x18],0x0
40b0428e 7470             jz      40b04300

where ESI is 0.  This looks like an attempt to make a virtual function call on a
null pointer to the function at vtable index 0x18, where (in egcs vtables) the
0th function is at vtable index 0x8, which fits the position of GetContext.  So
I think this patch should fix the problem.

Does a document have a null global object during the paint suppression period
when it is about to be replaced?  Could that be when this is happening?

Comment 11

[Asa Dotzler [:asa]]

Comment on attachment 52562 [details] [diff] [review]
Probable fix

a=asa (on behalf of drivers) for checkin to 0.9.5 branch.

Comment 12

[Jaime Rodriguez, Jr.]

this looks like something we'd like for the 094 branch. clayton, can we talk
about this one in the PDT tomrrow.

Comment 13

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Checked in on 0.9.5.

Comment 14

[Jaime Rodriguez, Jr.]

can someone, pls check this into the 094 branch as soon as you can - PDT+

Comment 15

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Checked in on 0.9.4 as well, marking fixed.

Comment 16

[Terri Preston]

Verified fix checked into lxr.mozilla.org