[Offline Page] Offline page sharing implementation

chrome/android/java/src/org/chromium/chrome/browser/share/ShareHelper.java

Index: chrome/android/java/src/org/chromium/chrome/browser/share/ShareHelper.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/share/ShareHelper.java b/chrome/android/java/src/org/chromium/chrome/browser/share/ShareHelper.java
index 10e37003b6d30427035f16a308ed4efb7e27fa4a..7f8e86fe08ee1e493a22eccfddc659b8b85449fe 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/share/ShareHelper.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/share/ShareHelper.java
@@ -217,15 +217,16 @@ public class ShareHelper {
      *                 older Android versions).
      */
     public static void share(boolean shareDirectly, boolean saveLastUsed, Activity activity,
-            String title, String text, String url, Bitmap screenshot,
+            String title, String text, String url, Uri offlineUri, Bitmap screenshot,
             @Nullable TargetChosenCallback callback) {
         if (shareDirectly) {
-            shareWithLastUsed(activity, title, text, url, screenshot);
+            shareWithLastUsed(activity, title, text, url, offlineUri, screenshot);
         } else if (TargetChosenReceiver.isSupported()) {
-            makeIntentAndShare(saveLastUsed, activity, title, text, url, screenshot, null,
-                               callback);
+            makeIntentAndShare(saveLastUsed, activity, title, text, url, offlineUri, screenshot,
+                    null, callback);
         } else {
-            showShareDialog(saveLastUsed, activity, title, text, url, screenshot, callback);
+            showShareDialog(
+                    saveLastUsed, activity, title, text, url, offlineUri, screenshot, callback);
         }
     }
 
@@ -303,9 +304,9 @@ public class ShareHelper {
      *                 older Android versions).
      */
     private static void showShareDialog(final boolean saveLastUsed, final Activity activity,
-            final String title, final String text, final String url, final Bitmap screenshot,
-            @Nullable final TargetChosenCallback callback) {
-        Intent intent = getShareIntent(activity, title, text, url, null);
+            final String title, final String text, final String url, final Uri offlineUri,
+            final Bitmap screenshot, @Nullable final TargetChosenCallback callback) {
+        Intent intent = getShareIntent(activity, title, text, url, null, null);
         PackageManager manager = activity.getPackageManager();
         List<ResolveInfo> resolveInfoList = manager.queryIntentActivities(intent, 0);
         assert resolveInfoList.size() > 0;
@@ -329,8 +330,8 @@ public class ShareHelper {
                         new ComponentName(ai.applicationInfo.packageName, ai.name);
                 callback.onTargetChosen(component);
                 if (saveLastUsed) setLastShareComponentName(component);
-                makeIntentAndShare(false, activity, title, text, url, screenshot, component,
-                                   null);
+                makeIntentAndShare(
+                        false, activity, title, text, url, offlineUri, screenshot, component, null);
                 dialog.dismiss();
             }
         });
@@ -346,11 +347,12 @@ public class ShareHelper {
      * @param url URL of the page to be shared.
      * @param screenshot Screenshot of the page to be shared.
      */
-    private static void shareWithLastUsed(
-            Activity activity, String title, String text, String url, Bitmap screenshot) {
+    private static void shareWithLastUsed(Activity activity, String title, String text, String url,
+            Uri offlineUri, Bitmap screenshot) {
         ComponentName component = getLastShareComponentName();
         if (component == null) return;
-        makeIntentAndShare(false, activity, title, text, url, screenshot, component, null);
+        makeIntentAndShare(
+                false, activity, title, text, url, offlineUri, screenshot, component, null);
     }
 
     private static void shareIntent(boolean saveLastUsed, Activity activity, Intent sharingIntent,
@@ -366,12 +368,12 @@ public class ShareHelper {
     }
 
     private static void makeIntentAndShare(final boolean saveLastUsed, final Activity activity,
-            final String title, final String text, final String url, final Bitmap screenshot,
-            final ComponentName component, @Nullable final TargetChosenCallback callback) {
+            final String title, final String text, final String url, final Uri offlineUri,
+            final Bitmap screenshot, final ComponentName component,
+            @Nullable final TargetChosenCallback callback) {
         if (screenshot == null) {
-            shareIntent(
-                    saveLastUsed, activity,
-                    getDirectShareIntentForComponent(activity, title, text, url, null, component),
+            shareIntent(saveLastUsed, activity, getDirectShareIntentForComponent(activity, title,

[gone, 2016/08/12 01:26:13]

Pull out the getDirectShareIntentForComponent call to get around this weird
indentation.

[Vivian, 2016/08/12 23:07:13]

Done.

+                                                        text, url, offlineUri, null, component),
                     callback);
         } else {
             new AsyncTask<Void, Void, File>() {
@@ -411,10 +413,9 @@ public class ShareHelper {
                             != ApplicationState.HAS_DESTROYED_ACTIVITIES) {
                         Uri screenshotUri = saveFile == null
                                 ? null : UiUtils.getUriForImageCaptureFile(activity, saveFile);
-                        shareIntent(
-                                saveLastUsed, activity,
+                        shareIntent(saveLastUsed, activity,
                                 getDirectShareIntentForComponent(activity, title, text, url,
-                                                                 screenshotUri, component),
+                                            offlineUri, screenshotUri, component),
                                 callback);
                     }
                 }
@@ -435,7 +436,7 @@ public class ShareHelper {
         final ComponentName component = getLastShareComponentName();
         boolean isComponentValid = false;
         if (component != null) {
-            Intent intent = getShareIntent(activity, "", "", "", null);
+            Intent intent = getShareIntent(activity, "", "", "", null, null);
             intent.setPackage(component.getPackageName());
             PackageManager manager = activity.getPackageManager();
             List<ResolveInfo> resolveInfoList = manager.queryIntentActivities(intent, 0);
@@ -494,8 +495,8 @@ public class ShareHelper {
     }
 
     @VisibleForTesting
-    protected static Intent getShareIntent(
-            Activity activity, String title, String text, String url, Uri screenshotUri) {
+    public static Intent getShareIntent(Activity activity, String title, String text, String url,
+            Uri offlineUri, Uri screenshotUri) {

[Theresa, 2016/08/12 17:36:44]

It seems a bit odd to pass in a url and an offlineUri. Can we use a boolean to
indicate whether the String url (possibly renamed to uri) is an offline page
URI?

[Vivian, 2016/08/12 23:07:12]

The url and offlineUri here would be two different thing. URL is an http link to
the original online URL of the page. OfflineUri is the file URI to the mhtml
file to be shared. Say you chose Gmail in share dialog, url would be shown in
the email context, while mhtml file would be added as attachment. 

[Theresa, 2016/08/13 15:58:48]

Ah, I see now - I missed that we were using both when sharing an offline page.
Thanks.

         if (!TextUtils.isEmpty(url)) {
             url = DomDistillerUrlUtils.getOriginalUrlFromDistillerUrl(url);
             if (!TextUtils.isEmpty(text)) {
@@ -507,20 +508,26 @@ public class ShareHelper {
         }
 
         Intent intent = new Intent(Intent.ACTION_SEND);
-        intent.addFlags(ApiCompatibilityUtils.getActivityNewDocumentFlag());
         intent.setType("text/plain");
+        intent.addFlags(ApiCompatibilityUtils.getActivityNewDocumentFlag());
         intent.putExtra(Intent.EXTRA_SUBJECT, title);
         intent.putExtra(Intent.EXTRA_TEXT, text);
         intent.putExtra(EXTRA_TASK_ID, activity.getTaskId());
 
-        if (screenshotUri != null) {
+        if (screenshotUri != null || offlineUri != null) {
             intent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION);
-            // To give read access to an Intent target, we need to put |screenshotUri| in clipData
-            // because adding Intent.FLAG_GRANT_READ_URI_PERMISSION doesn't work for
+        }
+        if (screenshotUri != null) {
+            // To give read access to an Intent target, we need to put |screenshotUri| in
+            // clipData because adding Intent.FLAG_GRANT_READ_URI_PERMISSION doesn't work for

[gone, 2016/08/12 01:26:13]

Revert the comment move

[Vivian, 2016/08/12 23:07:13]

Done.

             // EXTRA_SHARE_SCREENSHOT_AS_STREAM.
             intent.setClipData(ClipData.newRawUri("", screenshotUri));
             intent.putExtra(EXTRA_SHARE_SCREENSHOT_AS_STREAM, screenshotUri);
         }

[gone, 2016/08/12 01:26:13]

Maybe:

if (offlineUri == null) {
    intent.setType("text/plain");
} else {
    intent.setType("multipart/related");
    intent.putExtra(Intent.EXTRA_STREAM, offlineUri);
}

[Vivian, 2016/08/12 23:07:12]

Done.

+        if (offlineUri != null) {
+            intent.setType("multipart/related");

[Theresa, 2016/08/12 17:36:44]

What sort of apps can handle this type mime type besides Chrome (also, do we
show up on the intent picker for these share intents?)? Maybe upload to drive or
email the file?

Do we think this is what the user wants to do vs sharing the original page URL?

[Vivian, 2016/08/12 23:07:13]

List of apps that can handle this mime type: Gmail, Drive, SHAREit, Xender,
Beam.
I have a patch enabling Chrome handling incoming multipart/related intent. So
the user who receives the file would be able to open it via Chrome. (See
https://codereview.chromium.org/2098263002/)

The feature is behind flag "OfflinePagesSharing". If user does not have the flag
set to ON, original page URL is shared instead.

+            intent.putExtra(Intent.EXTRA_STREAM, offlineUri);

[Theresa, 2016/08/12 17:36:44]

Since this is just a single extra, I think we should be able to share multiple
offline pages from the downloads UI unless there's something else specifically
prohibiting that?

To share multiple, we just have to do something like this instead of putting a
single extra:  shareIntent.putParcelableArrayListExtra(Intent.EXTRA_STREAM,
ArrayList<Uri> uris);

[Vivian, 2016/08/12 23:07:13]

I talked with Dmitry. I think we want to leave it this way in this patch. And
maybe add this capacity later.

[Theresa, 2016/08/13 15:58:48]

It doesn't need to get integrated here. I'm asking because we're implement
sharing multiple downloads from the download manager ui.

+        }
         return intent;
     }
 
@@ -533,10 +540,9 @@ public class ShareHelper {
         return intent;
     }
 
-    private static Intent getDirectShareIntentForComponent(
-            Activity activity, String title, String text, String url,
-            Uri screenshotUri, ComponentName component) {
-        Intent intent = getShareIntent(activity, title, text, url, screenshotUri);
+    private static Intent getDirectShareIntentForComponent(Activity activity, String title,
+            String text, String url, Uri offlineUri, Uri screenshotUri, ComponentName component) {
+        Intent intent = getShareIntent(activity, title, text, url, offlineUri, screenshotUri);
         intent.addFlags(Intent.FLAG_ACTIVITY_FORWARD_RESULT
                 | Intent.FLAG_ACTIVITY_PREVIOUS_IS_TOP);
         intent.setComponent(component);