323231 - Expose new XRE Profile APIs to Java embedders

Status: RESOLVED FIXED

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

Description

[jhp (no longer active)]

In bug 321359, bsmedberg created a couple of methods for using and locking profiles, for use by embedders.  Need to expose these methods for use by Java embedders.

Comment 1

[jhp (no longer active)]

*** Bug 337046 has been marked as a duplicate of this bug. ***

Comment 2

[Michal Ceresna]

Attachment: profile support

patch for profile support reworked to the api as you asked in the bug #323231

Comment 3

[Michal Ceresna]

Attachment: profile support (v2)

improved exception handling

Comment 4

[jhp (no longer active)]

Comment on attachment 221426 [details] [diff] [review]
profile support (v2)

+  /**
+   * Lock a profile directory using platform-specific semantics.
+   *
+   * @param aDirectory  The profile directory to lock.
+   * @param aLockObject An opaque lock object. The directory will remain locked
+   *                    as long as the XPCOM reference is held.
+   */
+  void lockProfileDirectory(File aDirectory, nsISupports aLockObject)
+    throws XPCOMException;

It looks to me that aLockObject is actually a return value.  As long as the caller keeps the XPCOM ref to the lock object alive, then the profile is locked.  When the XPCOM ref is NS_RELEASEd, the profile is unlocked.

In our case, we would want to wrap the returned XPCOM ref in a Java object, and return that Java object.  That way, when the Java app drops the ref to that object and it is garbage collected, it will release the XPCOM ref and unlock the profile.

So this method declaration should look like:

 nsISupports lockProfileDirectory(File aDirectory)
    throws XPCOMException;

Otherwise, the patch looks good.

Comment 5

[Michal Ceresna]

Attachment: profile support (v3)

you are right, it is an out param;
updated

Comment 6

[jhp (no longer active)]

Comment on attachment 221558 [details] [diff] [review]
profile support (v3)

>+  /**
>+   * Lock a profile directory using platform-specific semantics.
>+   *
>+   * @param aDirectory  The profile directory to lock.
>+   * @param aLockObject An opaque lock object. The directory will remain locked
>+   *                    as long as the XPCOM reference is held.
>+   */

|aLockObject| should be @return, rather than @param.

>+nsresult
>+LockProfileDirectory_Impl(JNIEnv* env, jobject aDirectory, 
>+                          nsISupports* *aLockObject)
>+{
>+
>+  nsresult rv;
>+
>+  nsCOMPtr<nsILocalFile> profDir;
>+  if (aDirectory) {
>+    rv = File_to_nsILocalFile(env, aDirectory, getter_AddRefs(profDir));
>+    NS_ENSURE_SUCCESS(rv, rv);
>+  }
>+
>+  return XRE_LockProfileDirectory(profDir, aLockObject); //lock AddRef'd inside
>+}
>+
>+extern "C" NS_EXPORT jobject
>+GRE_NATIVE(lockProfileDirectory) (JNIEnv* env, jobject, jobject aDirectory)
>+{
>+
>+  nsCOMPtr<nsISupports> lockObj;
>+  nsresult rv = LockProfileDirectory_Impl(env, aDirectory, 
>+                                          getter_AddRefs(lockObj));
>+
>+  if (NS_SUCCEEDED(rv)) {
>+    jobject javaProxy;
>+    rv = GetNewOrUsedJavaObject(env, lockObj, NS_GET_IID(nsISupports),
>+                                &javaProxy);
>+    if (NS_SUCCEEDED(rv))
>+      return javaProxy;
>+  }
>+
>+  ThrowException(env, rv, "Failure in lockProfileDirectory");
>+  return nsnull;
>+}

Since the code here is small, and there aren't many return points, you should just combine this into one method.  Or at least move the |GetNewOrUsedJavaObject| call into the *_Impl method.

With those changes, r=jhpedemonte

Comment 7

[Michal Ceresna]

Attachment: profile support (v4)

the code was cleaner when keeping the _Impl method. Otherwise I got 4 nested if{}

Comment 8

[Michal Ceresna]

Attachment: profile support (v5)

fixed aDirectory null test

Comment 9

[jhp (no longer active)]

Comment on attachment 221831 [details] [diff] [review]
profile support (v5)

+  nsCOMPtr<nsILocalFile> profDir;
+  if (!aDirectory) return NS_ERROR_NULL_POINTER;
+  
+  rv = File_to_nsILocalFile(env, aDirectory, getter_AddRefs(profDir));

The NULL_POINTER check should be above the comptr.  But I can fix that when I check this in.  Looks good.

Michal, have you had a chance to test this patch with your version of the SWT Browser?

Comment 10

[Michal Ceresna]

Attachment: testcase

here is the testcase I was using :) 

If you run with xulrunner-1.9 and no profile is set, then during loading the www.news.com the java process crashes.

Comment 11

[Michal Ceresna]

Attachment: testcase result

and the result

Comment 12

[jhp (no longer active)]

Comment on attachment 221831 [details] [diff] [review]
profile support (v5)

bsmedberg, I wanted to get your opinion on this patch, from the design point of view.  It just adds the two profile methods to the main Mozilla.java class.

Comment 13

[jhp (no longer active)]

Checked in to trunk. ->FIXED

Opened bug 340960 for creating a solution for the 1.8 branch.