Closed Bug 78504 Opened 23 years ago Closed 23 years ago

Crash after submitting form

Categories

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

Product:
Core
Component:
DOM: Navigation
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla0.9.2

People

(Reporter: michel.poleur, Assigned: danm.moz)

References

(
URL
)

Details

(Keywords: crash, topcrash)

Attachments

(2 files)

make zombie docshell less zombie. doesn't work; included only for posterity
1.86 KB, patch
Details | Diff | Splinter Review
bulletproof windowwatcher as described above. this is the fix.
1.28 KB, patch
Details | Diff | Splinter Review 
The page displayed at http://www.ebanking.lu contains a link labelled "Accès
Membre". Clicking on this link opens a new little window asking a username/pass.
I used test values (u=isidore/p=isidore) and I clicked on "Login". And it crashed.
confirming with win2k build 20010501.. (CVS debug)

Stack Trace:
nsWindowWatcher::OpenWindowJS(nsWindowWatcher * const 0x00d17734, nsIDOMWindow * 
0x04d6467c, const char * 0x00000000, const char * 0x04cd16e0, const char * 
0x00000000, int 0, unsigned int 0, long * 0x03e8585c, nsIDOMWindow * * 
0x0012f350) line 467 + 62 bytes
GlobalWindowImpl::OpenInternal(GlobalWindowImpl * const 0x04d64678, JSContext * 
0x04e723c0, long * 0x03e85850, unsigned int 2, int 0, nsIDOMWindowInternal * * 
0x0012f444) line 3046 + 126 bytes
GlobalWindowImpl::Open(GlobalWindowImpl * const 0x04d6467c, JSContext * 
0x04e723c0, long * 0x03e85850, unsigned int 2, nsIDOMWindowInternal * * 
0x0012f444) line 2118
nsURILoader::GetTarget(nsURILoader * const 0x00de50d0, nsIChannel * 0x03eaaf60, 
nsCString & {...}, nsISupports * 0x04d053c8, nsISupports * * 0x0012f614) line 
773 + 84 bytes
nsURILoader::OpenURIVia(nsURILoader * const 0x00de50d0, nsIChannel * 0x03eaaf60, 
int 7, const char * 0x0012faf8, nsISupports * 0x04d053c8, unsigned int 0) line 
826 + 51 bytes
nsURILoader::OpenURI(nsURILoader * const 0x00de50d0, nsIChannel * 0x03eaaf60, 
int 7, const char * 0x0012faf8, nsISupports * 0x04d053c8) line 513
nsDocShell::DoChannelLoad(nsDocShell * const 0x04d053c8, nsIChannel * 
0x03eaaf60, int 7, const char * 0x0012faf8, nsIURILoader * 0x00de50d0) line 3885 
+ 28 bytes
nsDocShell::DoURILoad(nsDocShell * const 0x04d053c8, nsIURI * 0x04d009e8, nsIURI 
* 0x00000000, nsISupports * 0x00000000, int 1, int 7, const char * 0x0012faf8, 
nsIInputStream * 0x05071e8c, nsIInputStream * 0x00000000) line 3668 + 41 bytes
nsDocShell::InternalLoad(nsDocShell * const 0x04d053c8, nsIURI * 0x04d009e8, 
nsIURI * 0x00000000, nsISupports * 0x00000000, int 1, int 0, const char * 
0x0012faf8, nsIInputStream * 0x05071e8c, nsIInputStream * 0x00000000, unsigned 
int 2097153, nsISHEntry * 0x00000000) line 3417 + 47 bytes
nsWebShell::HandleLinkClickEvent(nsIContent * 0x04d3c0f8, nsLinkVerb 
eLinkVerb_Replace, const unsigned short * 0x03cd36f0, const unsigned short * 
0x04e66058, nsIInputStream * 0x05071e8c, nsIInputStream * 0x00000000) line 826
OnLinkClickEvent::HandleEvent() line 685
HandlePLEvent(OnLinkClickEvent * 0x05069028) line 699
PL_HandleEvent(PLEvent * 0x05069028) line 588 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00d82138) line 518 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x000d05c4, unsigned int 49389, unsigned int 0, 
long 14164280) line 1069 + 9 bytes
USER32! 77e048dc()
USER32! 77e04aa7()
USER32! 77e166fd()
nsAppShellService::Run(nsAppShellService * const 0x00d13e48) line 408
main1(int 2, char * * 0x003576c8, nsISupports * 0x00000000) line 1005 + 32 bytes
main(int 2, char * * 0x003576c8) line 1306 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e892a6()
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: crash
Over to embedding.
Assignee: asa → adamlock
Component: Browser-General → Embedding: Docshell
QA Contact: doronr → adamlock
Assignee: adamlock → danm
-> Dan
*** Bug 79304 has been marked as a duplicate of this bug. ***
Changing OS/Platform to all based on dupe. Also changing topic to Crash after 
submitting form used to be after identifying myself with username/password
OS: Windows 2000 → All
Hardware: PC → All
Summary: Crash after identifying myself with username/password → Crash after submitting form
Adding topcrash keyword.  This is currently the #1 topcrash with a good
stacktrace listed in:
ftp://ftp.mozilla.org/pub/data/crash-data/detailed-crash-analysis-all.html
Keywords: topcrash
i think this might be a dup of bug 81629, which was fixed on 5/18.  can qa 
please verify?
It's not crashing with today's build, so it's probably safe to take off the 
"topcrash" keyword. But neither is it exactly working. After loging in, an 
attempt to load the login.cfm script is failing because it's coming through on a 
window in the process of being deleted. Very disturbing. At least the u/p test 
values are still working; thanks for holding those open.

In ftp://ftp.mozilla.org/pub/data/crash-data/detailed-crash-analysis-all.html
there was one report of this crash on Sunday the 20th and were two from Monday
the 21st.  That's much fewer than there were on Friday the 18th and Thursday the
17th, but it doesn't make it seem like it's *completely* fixed.
Adding qawanted keyword.
Keywords: qawanted
Bug report from Tucson Beta
While reproducing error per instructions I received the following error 
message.  This message appears when clicking on login after ebtering sample 
user name and password.
NETSCP6 caused an invalid page fault in
module EMBEDCOMPONENTS.DLL at 016f:601a1611.
Registers:
EAX=00000000 CS=016f EIP=601a1611 EFLGS=00010246
EBX=0081f4c4 SS=0177 ESP=0068f140 EBP=0068f240
ECX=0068f14c DS=0177 ESI=00000000 FS=4b97
EDX=00000000 ES=0177 EDI=00000000 GS=0000
Bytes at CS:EIP:
8b 18 8d 45 fc 50 56 ff 15 18 71 1a 60 50 57 ff 
Stack dump:
00000000 034ba454 018346d8 60ebdbf8 0000000c 0000003f 00000001 00000000 
0068f164 00720070 006e0069 00690063 00610070 0065006c 00390037 00680000 
betadoc@aol.com: which build are you using ?
This seem to be fixed (the crash) for me with win2k build 20010528.. (CVS opt)

But the login doesn´t work...
The crash originally reported here is still showing up in talkback reports. 
It's now line 512 of nsWindowWatcher.cpp.

Why don't you null-check parentTreeOwner on that line?  You null-check
everywhere else and I don't think it's guaranteed to be non-null.
I hit this one today on a fresh pull while running the browser buster. It may be 
possible for the parent window to be torn down just as the new one is being 
opened.
->0.9.2
Target Milestone: --- → mozilla0.9.2
Assignee: danm → dr
Priority: -- → P1
i'll give this a try...
Yikes, no I won't. Back to danm.

The symptoms are different for me (in both my opt and debug builds): when
clicking on the link, two windows come up. One of the windows is probably
supposed to be the uname/passwd dialog, but the other is a PSM dialog prompting
me to accept a certificate from the site. The browser hangs at this point, and I
can't do anything more.

Perhaps I need to build without PSM to see the original problem...
Assignee: dr → danm
Depends on: 84572
The hang didn't happen a week or two ago. Filed dependent bug 84572 about that.

We have a testcase for the nsWindowWatcher (line 512) crash.
see bug 84592
  I'm working with the crash from bug 84592: same stack trace; different 
circumstances. I can't reproduce the crash from this bug. (Darin has posted a 
patch to bug 84572 that fixes the hang. But once past the hang, I still can't 
log in. It looks like a script from the website is closing the login window 
before I can log in:

function focus_popup() {
if ((newwin.closed==false)&(newwin!=null)) newwin.close();
}
window.onfocus = focus_popup;

I'll skip comment on the JavaScript and just say that I suspect this is why the 
login window is closing prematurely. It could be Mozilla's fault; an errant 
focus swinging by, but it's stopping me from getting there.)

  So let me reiterate that I'm really talking about the crash in bug 84592 from 
here on.
  This crashes because WindowWatcher assumes any window handed it to serve as a 
parent to a newly opened window will have a valid treeowner. If the window 
doesn't have a valid treeowner, it can't really function as a parent, so that 
seems a reasonable assumption. However, the recent change to keep the old 
docshell disconnected but alive while a new one was loading makes that 
assumption not always work.
  You might think a real fix would be to leave the disconnected docshell hooked 
up enough to serve as a proper parent. I tried that for a while (see the 
attached patch) but perhaps it's not surprising that it didn't actually work: no 
window was opened at all.
  In the end, I just decided to bulletproof the line that was crashing. In the 
situation that triggers this crash (an attempt to open a window from a parent 
window that's simultaneously being closed or reloaded) the new window will be 
opened without a parent: it'll be a new, independent window. That's not exactly 
correct, but I think we can live with it.
Status: NEW → ASSIGNED
I can hear a resounding "bite me" coming on, but:

-      } else
-        parentTreeOwner->FindItemWithName(name.GetUnicode(), nsnull,
-                                          getter_AddRefs(newDocShellItem));
+      } else {
+        /* parent is being simultaneously torn down (probably because of
+           the code that keeps an old docshell alive but disconnected while
+           we load a new one). not much to do but open the new window
+           without a parent. */
+        if (parentTreeOwner)
+          parentTreeOwner->FindItemWithName(name.GetUnicode(), nsnull,
+                                            getter_AddRefs(newDocShellItem));
+      }
     } else
       FindItemWithName(name.GetUnicode(), getter_AddRefs(newDocShellItem));

It'd be "nicer" if you just had:

else if (parentTreeOwner)
  parentTreeOwner->FindItemWithName(/* ... */);
else
  FindItemWithName(/* ... *);

(i.e., just killing as many curly braces as you can).

But whatever. r=dr.
I hate extraneous braces, too, but in my opinion the long comment warrants these. 
And I also normally do "else if ..." clauses together as you suggest, but only if 
all the ifs are related. This if is asking a different kind of question. So 
actually, I prefer it the way it is.

PS sr=hyatt

a= asa@mozilla.org for checkin to the trunk.
(on behalf of drivers)
Blocks: 83989
patch to bulletproof windowwatcher is in
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
tested on 
linux:    2001-07-26-23-0.9.2
windows:  2001-07-27-00-0.9.2
          2001-07-27-12-trunk
mac:      2001-07-26-21-0.9.2

Verified for branch, however, on the trunk build, the small login window at
ebanking.lu is empty so that there's no way to complete this test for the trunk.
Keywords: qawanted
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: