Sharing temporary pages by content uri.

To share a temporary page, we use a content: URI instead of a file URI
so that we don't need to publish the page to the download directory.

Bug: 758733
Change-Id: I873756591fc29537fa1290a0153980f1135db04b
Reviewed-on: https://chromium-review.googlesource.com/1056214
Commit-Queue: Peter Williamson <petewil@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Yafei Duan <romax@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558786}

chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridge.java

@@ -657,6 +657,15 @@ public class OfflinePageBridge {
         return nativeIsOfflinePage(mNativeOfflinePageBridge, webContents);
     }
 
+    /**
+     * Determines if the page is in one of the user requested download namespaces.
+     * @param nameSpace Namespace of the page in question.
+     * @return true if the page is in a user requested download namespace.
+     */
+    public boolean isUserRequestedDownloadNamespace(String nameSpace) {
+        return nativeIsUserRequestedDownloadNamespace(mNativeOfflinePageBridge, nameSpace);
+    }
+
     /**
      * Checks if the supplied file path is in a private dir internal to chrome.
      * @param file_path Path of the file to check.
@@ -873,6 +882,8 @@ public class OfflinePageBridge {
             long nativeOfflinePageBridge, WebContents webContents);
     private native boolean nativeIsInPrivateDirectory(
             long nativeOfflinePageBridge, String filePath);
+    private native boolean nativeIsUserRequestedDownloadNamespace(
+            long nativeOfflinePageBridge, String nameSpace);
     private native OfflinePageItem nativeGetOfflinePage(
             long nativeOfflinePageBridge, WebContents webContents);
     private native void nativeCheckForNewOfflineContent(

chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java

@@ -20,6 +20,7 @@ import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.base.metrics.RecordUserAction;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ChromeActivity;
+import org.chromium.chrome.browser.FileProviderHelper;
 import org.chromium.chrome.browser.UrlConstants;
 import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.browser.share.ShareParams;
@@ -389,8 +390,18 @@ public class OfflinePageUtils {
         String offlinePath = offlinePage.getFilePath();
 
         final String pageUrl = tab.getUrl();
+        // We share temporary pages by content URI to prevent unanticipated side effects in the
+        // public directory. Temporary pages are ones not in a user requested download namespace.
+        Uri uri;
+        if (!offlinePageBridge.isUserRequestedDownloadNamespace(
+                    offlinePage.getClientId().getNamespace())) {
+            File file = new File(offlinePage.getFilePath());
+            uri = (new FileProviderHelper()).getContentUriFromFile(file);
+        } else {
+            uri = Uri.parse(pageUrl);
+        }
 
-        if (!isOfflinePageShareable(offlinePageBridge, offlinePage, pageUrl)) return false;
+        if (!isOfflinePageShareable(offlinePageBridge, offlinePage, uri)) return false;
 
         // The file access permission is needed since we may need to publish the archive file
         // if it resides in internal directory.
@@ -409,7 +420,8 @@ public class OfflinePageUtils {
 
             final String pageTitle = tab.getTitle();
             final File offlinePageFile = new File(offlinePath);
-            sharePage(activity, pageUrl, pageTitle, offlinePath, offlinePageFile, shareCallback);
+            sharePage(activity, uri.toString(), pageTitle, offlinePath, offlinePageFile,
+                    shareCallback);
         });
 
         return true;
@@ -423,22 +435,14 @@ public class OfflinePageUtils {
      * @return true if this page can be shared.
      */
     public static boolean isOfflinePageShareable(
-            OfflinePageBridge offlinePageBridge, OfflinePageItem offlinePage, String pageUri) {
+            OfflinePageBridge offlinePageBridge, OfflinePageItem offlinePage, Uri uri) {
         // Return false if there is no offline page or sharing is not enabled.
         if (offlinePage == null || !OfflinePageBridge.isPageSharingEnabled()) return false;
 
-        // We share temporary pages by URL instead of sharing the page to prevent unanticipated side
-        // effects in the public directory.
-        // TODO(petewil): https://crbug.com/831780 - Desired long term behavior is to share the page
-        // with a content URI instead of as a URL, so all offline pages are shared as MHTML.
-        if (isTemporaryNamespace(offlinePage.getClientId().getNamespace())) return false;
-
         String offlinePath = offlinePage.getFilePath();
-        Uri uri = Uri.parse(pageUri);
 
         // If we have a content or file Uri, then we can share the page.
         if (isSchemeContentOrFile(uri)) {
-            assert offlinePath.isEmpty();
             return true;
         }
 
@@ -464,17 +468,6 @@ public class OfflinePageUtils {
         return isContentScheme || isFileScheme;
     }
 
-    /**
-     * Determines if the page is in one of the temporary namespaces.
-     * @param namespace Namespace of the page in question.
-     * @return true if the page is in a temporary namespace.
-     */
-    public static boolean isTemporaryNamespace(String namespace) {
-        return namespace.equals(OfflinePageBridge.BOOKMARK_NAMESPACE)
-                || namespace.equals(OfflinePageBridge.LAST_N_NAMESPACE)
-                || namespace.equals(OfflinePageBridge.CCT_NAMESPACE)
-                || namespace.equals(OfflinePageBridge.SUGGESTED_ARTICLES_NAMESPACE);
-    }
 
     /**
      * For internal pages, we must publish them, then share them.

chrome/android/javatests/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtilsTest.java

@@ -322,10 +322,17 @@ public class OfflinePageUtilsTest {
                 boolean shared =
                         OfflinePageUtils.maybeShareOfflinePage(mActivityTestRule.getActivity(),
                                 mActivityTestRule.getActivity().getActivityTab(), shareCallback);
-                // The attempt to share a temporary page should fall back to sharing the URL.
-                Assert.assertFalse(shared);
+                // The attempt to share a temporary page should share a content URL.
+                Assert.assertTrue(shared);
             }
         });
+        // Wait for share callback to get called.
+        Assert.assertTrue(semaphore.tryAcquire(TIMEOUT_MS, TimeUnit.MILLISECONDS));
+        // Assert that URI is what we expected.
+        String foundUri = shareCallback.getSharedUri();
+        Uri uri = Uri.parse(foundUri);
+        String uriPath = uri.getPath();
+        Assert.assertEquals(TEST_PAGE, uriPath);
     }
 
     // Checks on the UI thread if an offline path corresponds to a sharable file.
@@ -340,7 +347,7 @@ public class OfflinePageUtilsTest {
                         mActivityTestRule.getActivity().getActivityTab().getProfile());
 
                 boolean isSharable = OfflinePageUtils.isOfflinePageShareable(
-                        offlinePageBridge, privateOfflinePageItem, uriPath);
+                        offlinePageBridge, privateOfflinePageItem, Uri.parse(uriPath));
                 Assert.assertEquals(sharable, isSharable);
             }
         });
@@ -381,13 +388,13 @@ public class OfflinePageUtilsTest {
 
         // Check that pages with temporary namespaces are not sharable.
         checkIfOfflinePageIsSharable(
-                fullPrivatePath, SHARED_URI, OfflinePageBridge.BOOKMARK_NAMESPACE, false);
+                fullPrivatePath, SHARED_URI, OfflinePageBridge.BOOKMARK_NAMESPACE, true);
         checkIfOfflinePageIsSharable(
-                fullPrivatePath, SHARED_URI, OfflinePageBridge.LAST_N_NAMESPACE, false);
+                fullPrivatePath, SHARED_URI, OfflinePageBridge.LAST_N_NAMESPACE, true);
         checkIfOfflinePageIsSharable(
-                fullPrivatePath, SHARED_URI, OfflinePageBridge.CCT_NAMESPACE, false);
+                fullPrivatePath, SHARED_URI, OfflinePageBridge.CCT_NAMESPACE, true);
         checkIfOfflinePageIsSharable(
-                fullPrivatePath, SHARED_URI, OfflinePageBridge.SUGGESTED_ARTICLES_NAMESPACE, false);
+                fullPrivatePath, SHARED_URI, OfflinePageBridge.SUGGESTED_ARTICLES_NAMESPACE, true);
     }
 
     @Test

chrome/browser/offline_pages/android/offline_page_bridge.cc

@@ -871,6 +871,15 @@ jboolean OfflinePageBridge::IsInPrivateDirectory(
   return offline_page_model_->IsArchiveInInternalDir(file_path);
 }
 
+jboolean OfflinePageBridge::IsUserRequestedDownloadNamespace(
+    JNIEnv* env,
+    const base::android::JavaParamRef<jobject>& obj,
+    const base::android::JavaParamRef<jstring>& j_name_space) {
+  std::string name_space(ConvertJavaStringToUTF8(env, j_name_space));
+  return (offline_page_model_->GetPolicyController()->IsUserRequestedDownload(
+      name_space));
+}
+
 ScopedJavaLocalRef<jobject> OfflinePageBridge::GetOfflinePage(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,

chrome/browser/offline_pages/android/offline_page_bridge.h

@@ -190,6 +190,11 @@ class OfflinePageBridge : public OfflinePageModel::Observer,
       const base::android::JavaParamRef<jobject>& obj,
       const base::android::JavaParamRef<jstring>& j_file_path);
 
+  jboolean IsUserRequestedDownloadNamespace(
+      JNIEnv* env,
+      const base::android::JavaParamRef<jobject>& obj,
+      const base::android::JavaParamRef<jstring>& j_name_space);
+
   base::android::ScopedJavaLocalRef<jobject> GetOfflinePage(
       JNIEnv* env,
       const base::android::JavaParamRef<jobject>& obj,