Browser crashes when the liveconnect call invoked using File->open

VERIFIED FIXED

Status

P1
major
VERIFIED FIXED
18 years ago
8 years ago

People

(Reporter: xiaobin.lu, Assigned: xiaobin.lu)

Tracking

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: oji_working)

Attachments

(6 attachments)

(Assignee)

Description

18 years ago
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 1

18 years ago
Reassign to myself!
Assignee: edburns → xiaobin.lu

Comment 2

18 years ago
merking NEW
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 3

18 years ago
Created attachment 33192 [details]
Testcase in Zip format

Updated

18 years ago
Blocks: 77600
(Assignee)

Comment 4

18 years ago
Created attachment 34802 [details] [diff] [review]
Patch (Iteration 1)
(Assignee)

Comment 5

18 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

18 years ago
Created attachment 34811 [details] [diff] [review]
Correct one
(Assignee)

Comment 7

18 years ago
Sorry for attching a wrong diff. Please use the last patch call " Correct one" 
foe reviewing.
Thanks!
(Assignee)

Comment 8

18 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.

Updated

18 years ago
Blocks: 78966

Comment 9

18 years ago
With applied patch, no crash on win32, but no alert either.

r=edburns.

Comment 10

18 years ago
Shouldn't this be fixed in the plugin itself?
(Assignee)

Comment 11

18 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

18 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

18 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

18 years ago
Summary: Browser crashes when the liveconnect call invoked → Browser crashes when the liveconnect call invoked using File->open

Comment 14

18 years ago
Adding status to indicate that it's currently being worked on by Xiaobin.
Whiteboard: oji_working
(Assignee)

Updated

18 years ago
Priority: -- → P1
(Assignee)

Comment 15

18 years ago
Created attachment 36929 [details] [diff] [review]
Using DirectoryService instead of using registry service
(Assignee)

Comment 16

18 years ago
Patrick:
   Please sr the patch. Tested in Windows and Linux, works fine!

Comment 17

18 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

18 years ago
Created attachment 37031 [details] [diff] [review]
Put static variable inside function
(Assignee)

Comment 19

18 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.

Updated

18 years ago
Blocks: 83989
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

18 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

18 years ago
Created attachment 37372 [details] [diff] [review]
use nsXPIDLCString

Comment 24

18 years ago
r=beard
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

18 years ago
Yes, exactly like what you said here. This fix should be considered as part of 
Lazy loading fix(Beard).

Comment 27

18 years ago
a= asa@mozilla.org for checkin to the trunk.
(on behalf of drivers)
(Assignee)

Comment 28

18 years ago
Fix checked in!
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 29

18 years ago
This still works on win32 with Wednesday's trunk.

Comment 30

17 years ago
qa->pmac
QA Contact: shrir → pmac

Comment 31

17 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

Updated

8 years ago
Component: Java: OJI → Java: OJI
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.