Closed
Bug 78840
Opened 23 years ago
Closed 23 years ago
Browser crashes when the liveconnect call invoked using File->open
Categories
(Core Graveyard :: Java: OJI, defect, P1)
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: xiaobin.lu, Assigned: xiaobin.lu)
References
Details
(Whiteboard: oji_working)
Attachments
(6 files)
2.01 KB,
application/octet-stream
|
Details | |
3.71 KB,
patch
|
Details | Diff | Splinter Review | |
3.67 KB,
patch
|
Details | Diff | Splinter Review | |
3.04 KB,
patch
|
Details | Diff | Splinter Review | |
2.88 KB,
patch
|
Details | Diff | Splinter Review | |
2.97 KB,
patch
|
Details | Diff | Splinter Review |
From Bugzilla Helper: User-Agent: Mozilla/4.0 (compatible; MSIE 5.01; Windows NT) BuildID: Reproducible: Always Steps to Reproduce: 1.Download the testcase, lauch Java2JS.html 2.click the button Actual Results: Browser crashes! Expected Results: An alert box should come up. Drag the html page and drop it into the browser, it will not crash. However, crash happens when using File->open. Also remember to reproduce this bug, don't go to any applet page first.
Assignee | ||
Comment 3•23 years ago
|
||
Assignee | ||
Comment 4•23 years ago
|
||
Assignee | ||
Comment 5•23 years ago
|
||
Beard and Ed: Finally, we found out the reason why this problem exists. Actually it is caused by lazy loading fix. After lazy loading fix, the responsiblity of JVM start up was delayed until a Java application/applet are run. So, when the JVM started, a system class loader will be created to load the system class such as java.lang.System. There is a system properties called "usr.dir" which is used with "java.class.path" by the system classloader to find the class to load. When the user uses File->Open to open an applet page, the user.dir is set to the directory where is the class file is. This is why we always see that the applet was loaded by the system classloader which may cause liveconnect call from Java to JS crash. My fix is to save the current directory to a place and then reset the current directory to the java plugin lib directory. After the JVM is started up, we may just restore the directory to the original one. Unix plugin implementation does not have this problem. So this is why I use #ifdef. Also a static variable was used to indicate that the JVM was started up or not. This is out of the performance reason. The fix is fairly simple and please give it a review and sr. Thanks in advance!
Status: NEW → ASSIGNED
Assignee | ||
Comment 6•23 years ago
|
||
Assignee | ||
Comment 7•23 years ago
|
||
Sorry for attching a wrong diff. Please use the last patch call " Correct one" foe reviewing. Thanks!
Assignee | ||
Comment 8•23 years ago
|
||
With this patch, it is still not enough to make the testcase work. It just avoid crash when you bring up the testcase after the browser start up.
With applied patch, no crash on win32, but no alert either. r=edburns.
Comment 10•23 years ago
|
||
Shouldn't this be fixed in the plugin itself?
Assignee | ||
Comment 11•23 years ago
|
||
Sean: Actually this is not wrong with Java Plugin. It is caused by the JVM start up behavior change in browser side. Please refer my 5/16(13:12) comment for the reason.
Comment 12•23 years ago
|
||
I guess I don't follow the explanation of the cause. But from looking at the patch, it appears that you're just reading the Java plugin's Windows registry information to temporarily set the current working directory to "JavaHome" while a plugin instance is created. Why should that code be in the browser? Why doesn't the plugin read it's own registry information itself to see where the system classloader can find the class to load?
Assignee | ||
Comment 13•23 years ago
|
||
sean: Sorry if I made you confused. Actually it is possible to make change in the plugin code. However, the change will be much extensive than here. Java plugin can not control the system class loader to do things. It is part of jdk code which we do not have much control. Basically the lazy loading fix does something sort of breaking the assumption which Java plugin used to design.
Assignee | ||
Updated•23 years ago
|
Summary: Browser crashes when the liveconnect call invoked → Browser crashes when the liveconnect call invoked using File->open
Comment 14•23 years ago
|
||
Adding status to indicate that it's currently being worked on by Xiaobin.
Whiteboard: oji_working
Assignee | ||
Updated•23 years ago
|
Priority: -- → P1
Assignee | ||
Comment 15•23 years ago
|
||
Assignee | ||
Comment 16•23 years ago
|
||
Patrick: Please sr the patch. Tested in Windows and Linux, works fine!
Comment 17•23 years ago
|
||
Comments on the patch, you shouldn't put your "static classLoaded" variable at top-level in the file, but only inside the function, right next to the declaration of origDir. Consider renaming the variable to something like firstPluginInstance. Also, you appear to have a memory leak where you call "binDirectory->GetPath(&path);" because path is dynamically allocated. You need to free the memory. Might as well use an nsXPIDLCString for this, and the getter_Copies() pattern. Fix these problems, and then you'll my r=beard.
Assignee | ||
Comment 18•23 years ago
|
||
Assignee | ||
Comment 19•23 years ago
|
||
Patrick: It is better to use char* instead of nsXPIDLString because the parameter to pass into the SetCurrentDirectory is char*, I am trying to avoid using cast here. Thanks for your suggestion anyway.
Comment 20•23 years ago
|
||
xiaobin, beard recommended nsXPIDLCString, and you don't need to cast when passing one (call it s) wrapped by getter_Copies(s) -- don't cast away compiler errors in cases like this -- there's a helper class to use. See xpcom/ds/nsXPIDLString.h's comments. /be
Assignee | ||
Comment 21•23 years ago
|
||
Brendan: I think the reason I don't want to use nsXPIDLString is the parameter GetPath accepts is a char** instead of char*. I don't know if there is any way to use nsXPIDLString in that case.
getter_Copies (the usual case) and getter_Shares (the unusual case) create temporary objects with implicit conversions to char** and correctly manipulate the internals of the nsXPIDL[C]String. See http://lxr.mozilla.org/seamonkey/source/string/obsolete/nsXPIDLString.h#33
Assignee | ||
Comment 23•23 years ago
|
||
Comment 24•23 years ago
|
||
r=beard
Comment 25•23 years ago
|
||
So if I understand correctly, you need to change current process directory only for the first time, and only on Windows, because after the first time, the JVM will be already started, running in its own process, with the right current working directory? If so, sr=brendan@mozilla.org. /be
Assignee | ||
Comment 26•23 years ago
|
||
Yes, exactly like what you said here. This fix should be considered as part of Lazy loading fix(Beard).
Comment 27•23 years ago
|
||
a= asa@mozilla.org for checkin to the trunk. (on behalf of drivers)
Assignee | ||
Comment 28•23 years ago
|
||
Fix checked in!
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 29•23 years ago
|
||
This still works on win32 with Wednesday's trunk.
Comment 31•23 years ago
|
||
Verified ok without crash on windows 98 (commercial trunk: 2001-08-29-11-trunk) and Java(TM) Plug-in: Version 1.3.0_01.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•