Closed Bug 340960 Opened 18 years ago Closed 18 years ago

Provide profile support on 1.8.1 branch

Categories

(Core Graveyard :: Java to XPCOM Bridge, defect)

1.8 Branch
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jhpedemonte, Assigned: jhpedemonte)

Details

(Keywords: fixed1.8.1)

Attachments

(1 file, 4 obsolete files)

Unfortunately, bug 321359 will not make it to the 1.8 branch, which means we can't check in the patch from bug 323231.  So we'll have to roll our own profile locking code into JavaXPCOM for the 1.8 branch.  If possible, it would be best to keep the two methods exposed by the patch from bug 323231, in order to minimize the changes from XULRunner 1.8.1 (branch) to XULRunner 1.9 (trunk).
Attached patch profile support on 1.7 branch (obsolete) — Splinter Review
code to start with; separated from bug #332869
(In reply to comment #1)
Hello,

this a work in progress of the backported profile support to the 1.8.1 branch.

The c++ code is ready, but some javaxpcom java-code changes are
probably necessary, so your feedback is needed.

For starting the profiling, the correct call order should be:
  lockProfileDir()
  initEmbedding()
  notifyProfile()

The remaining open problem is, that the glue library is loaded only in the 
initEmbedding() function and therefore, the call to lockProfile fails with 
'unsatisfied link error'.

Therefore, the call to lockProfileDir() has to be commented out:
  //lockProfileDir()
  initEmbedding()
  notifyProfile()


In the current patch I put some workaround functions to get it running,
but this is too ugly for a final solution. 
Could you have a look and let me know, how the API should adapted to
correctly incorporate the lockProfileDir function?

Note, this problem happens also on the 1.9 branch, in the testcase
there, I was incorrectly using the call order:
initEmbedding()
lockProfileDir()
notifyProfile()





Attached patch profile support (v1) (obsolete) — Splinter Review
Attachment #227005 - Attachment is obsolete: true
Attached patch patch v1.1 (obsolete) — Splinter Review
Heh, I was working on the same thing.  And looks like we had similar ideas.  This is my patch, basically a simpler version than the one you put up.

There is nothing in the docs that says that XRE_LockProfileDirectory() has to be called before or after XRE_InitEmbedding().  For our needs, though, we can make it a hard rule that lockProfileDirectory() must be called after initEmbedding().

Unfortunately, even with this patch and handling "ProfD" in the AppDirProvider passed to initEmbedding(), I still can't get HTTPS sites to work.  I see the security files copied to the profile dir (secmod.db, key3.db & cert8.db), but page doesn't load.  Either I'm doing something wrong in this patch, or something else is going on.
Attachment #241197 - Attachment is obsolete: true
(In reply to comment #4)

> Heh, I was working on the same thing.  And looks like we had similar ideas. 
> This is my patch, basically a simpler version than the one you put up.
:)

> Unfortunately, even with this patch and handling "ProfD" in the AppDirProvider
> passed to initEmbedding(), I still can't get HTTPS sites to work.  I see the
> security files copied to the profile dir (secmod.db, key3.db & cert8.db), but
> page doesn't load.  Either I'm doing something wrong in this patch, or
> something else is going on.
just few minutes, and I'll isolate the necessary widget patch from the moz17 widget

the initial version of the necessary swt widget fixes for https is at:

https://bugs.eclipse.org/bugs/show_bug.cgi?id=148991
Comment on attachment 241204 [details] [diff] [review]
patch v1.1

I eventually found the code in AbstractMozillaBrowser.java that was blocking HTTPS requests (also pointed out by Michal).  After removing that code, everything worked as expected.

So this patch is ready to be reviewed.
Attachment #241204 - Flags: review?(benjamin)
(In reply to comment #7)

> I eventually found the code in AbstractMozillaBrowser.java that was blocking
Do also the pages with invalid certificates work for you? i.e. the SWT-embedded modal https warning windows? 

>There is nothing in the docs that says that XRE_LockProfileDirectory() has to
>be called before or after XRE_InitEmbedding().  For our needs, though, we can
>make it a hard rule that lockProfileDirectory() must be called after
>initEmbedding().
But this seems quite strange to me:
1) First initEmbedding() is called, which asks for the ProfD directory
in LocationProvider, and then starts to use this directory
2) Then lockProfileDirectory is called, but the directory is already used

Eventually, we could return for "ProfD" from Locatation provider null
until the lockProfileDirectory & notifyProfile are called, but
this would fallback to the second variant of profile initialization.
http://lxr.mozilla.org/seamonkey/source/toolkit/xre/nsXULAppAPI.h#312
All lockProfileDirectory does is add a 'lock' file to the dir so that another Mozilla process won't also use that profile.  There would just be that small window of time during initEmbedding where the profile isn't locked.  But I don't think we need to worry about that.  Benjamin, what do you think?
How do you know that the lock method will succeed? The whole point of locking is that if another process is using the profile, the first process will fail to obtain the lock.
Comment on attachment 241204 [details] [diff] [review]
patch v1.1

Heh, I always miss the obvious case!

Well, since lockProfileDirectory() can be called before initEmbedding(), I can't use native code.  So I'll probably have to rewrite the NS_LockProfileDirectory code in Java (like I did for GRE_GetGREPathWithProperites).
Attachment #241204 - Flags: review?(benjamin) → review-
Attached file ProfileLock.java (obsolete) —
ProfileLock.java tries to duplicate the functionality of nsProfileLock.cpp.  Create a ProfileLock in order to lock the given profile directory.  Throws an exception if profile is already locked (or error occurs).  Call release() to unlock profile dir.

Seems to work well on Linux, although I'm not done fully testing it yet.

Tried to copy the LockWithSymlink() function, but much of what is needed isn't supported by Java.  In fact, Java can't even create a symlink; I have to call 'ln' in order to do so.  I can't check if the symlink lock is 'stale', nor can I delete the symlink during an abnormal exit.

Benjamin, how necessary is LockWithSymlink()?  Seems like it's mostly for compatibility with older Mozilla products.  Could we just do with LockWithFcntl()?
Attachment #241204 - Attachment is obsolete: true
symlink locking is still unfortunately very common because of NFS/AFS implementations that don't support fcntl or do so very poorly.

I'm not sure I understand, though, why you need to do this in Java. Can't you

1) find and hook up to the correct xulrunner
2) lock the profile
3) start XPCOM?

This is how native apps are expected to work, and I'm not sure why Java apps couldn't do the same basic thing. Of course the "lock object" couldn't be an nsISupports because you don't have XPCOM started yet, but it could just be an Object.
> 1) find and hook up to the correct xulrunner

This currently happens when calling the initEmbedding() method.  It takes care of loading the XULRunner at the given path and starting XPCOM/embedding.

I could break off the XULRunner-hookup into a separate initialize() method, which would take the XR path.  Then I could call into native code which would just call NS_LockProfileDirectory().

It just seems redundant to call initialize(), followed soon after by initEmbedding().  But I guess I don't have much choice.
Well, that's how the native iface works.
Attached patch patch v1.2Splinter Review
This patch introduces a new method: Mozila.initialize().  It takes the path provided by getGREPathWithProperties() and loads the JavaXPCOM glue from the XULRunner installation.  This allows me to make Mozilla.lockProfileDirectory() to call into NS_LockProfileDirectory().

Using this code would look something like this:
  Mozilla mozilla = Mozilla.getInstance();
  GREVersionRange[] range = new GREVersionRange[1];
  range[0] = new GREVersionRange("1.8.0.*", false, "1.8.1.*", true);
  try {
     File grePath = Mozilla.getGREPathWithProperties(range, null);
     mozilla.initialize(grePath);
     profLock = mozilla.lockProfileDirectory(profileDir);
     LocationProvider locProvider = new LocationProvider(grePath, profileDir);
     mozilla.initEmbedding(grePath, grePath, locProvider);
     mozilla.notifyProfile();
  } catch (XPCOMInitializationException xie) {
     // handle exception
  } catch (XPCOMException xe) {
     // handle exception
  }
Attachment #241863 - Attachment is obsolete: true
Attachment #242087 - Flags: review?(benjamin)
Attachment #242087 - Flags: review?(benjamin) → review+
Comment on attachment 242087 [details] [diff] [review]
patch v1.2

Asking for 1.8.1 branch approval.  Patch is XULRunner only.  Provides necessary profile locking support on 1.8.1 branch.
Attachment #242087 - Flags: approval1.8.1?
Comment on attachment 242087 [details] [diff] [review]
patch v1.2

a=mconnor on behalf of drivers for 1.8 branch checkin, NPOTB for Firefox.
Attachment #242087 - Flags: approval1.8.1? → approval1.8.1+
Checked in to trunk and MOZILLA_1_8_BRANCH. ->FIXED
Status: NEW → RESOLVED
Closed: 18 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: