[Offline Page] Offline page sharing implementation

chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java

Index: chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
index 22553bd67afaa904db916040230234e1cbbab4e8..b066fd5f55736ed23d9f2feedc98be647f1750e2 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
@@ -84,6 +84,7 @@ import org.chromium.chrome.browser.metrics.UmaUtils;
 import org.chromium.chrome.browser.multiwindow.MultiWindowUtils;
 import org.chromium.chrome.browser.nfc.BeamController;
 import org.chromium.chrome.browser.nfc.BeamProvider;
+import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
 import org.chromium.chrome.browser.offlinepages.OfflinePageUtils;
 import org.chromium.chrome.browser.omaha.UpdateMenuItemHelper;
 import org.chromium.chrome.browser.pageinfo.WebsiteSettingsPopup;
@@ -994,26 +995,41 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
 
         final Activity mainActivity = this;
         ContentBitmapCallback callback = new ContentBitmapCallback() {
-                    @Override
-                    public void onFinishGetBitmap(Bitmap bitmap, int response) {
-                        // Check whether this page is an offline page, and use its online URL if so.
-                        String url = currentTab.getOriginalUrl();
-                        RecordHistogram.recordBooleanHistogram(
-                                "OfflinePages.SharedPageWasOffline", url != null);
-
-                        // If there is no entry in the offline pages DB for this tab, use the tab's
-                        // URL directly.
-                        if (url == null) url = currentTab.getUrl();
-
-                        ShareHelper.share(shareDirectly, true, mainActivity, currentTab.getTitle(),
-                                          null, url, bitmap, null);
-                        if (shareDirectly) {
-                            RecordUserAction.record("MobileMenuDirectShare");
-                        } else {
-                            RecordUserAction.record("MobileMenuShare");
-                        }
+            @Override
+            public void onFinishGetBitmap(Bitmap bitmap, int response) {
+                // Check whether this page is an offline page, and use its online URL if so.
+                String url = currentTab.getOfflinePageOriginalUrl();

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

Revert this line and use currentTab.getOriginalUrl(), which was added explicitly
for this purpose.

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

Done.

+                RecordHistogram.recordBooleanHistogram(
+                        "OfflinePages.SharedPageWasOffline", url != null);
+                boolean canShareOfflinePage =
+                        (url != null && OfflinePageBridge.isPageSharingEnabled());
+
+                boolean saveLastUsed = true;
+                ShareHelper.TargetChosenCallback callback = null;

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

nit: Doesn't seem to be a point to pulling these variables out.  You don't
change anything.

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

Done.

+                String text = null;
+
+                if (!canShareOfflinePage) {
+                    // If there is no entry in the offline pages DB for this tab, use the
+                    // tab's URL directly.

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

Once you download an offline page, does going to the original URL always
navigate to the offline page?

If not, could we encounter a scenario like.
1. http://somesite.com download
2. Navigate to http://somesite.com's live webpage (not the offline version)
3. Click share - shares offline page instead of the live page the user is
looking at.

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

I have this feature in a separate patch.
(https://codereview.chromium.org/2202423004/)
This patch only enables share offline page if you're currently viewing one. The
other patch would enable share offline page from online tab.

Is my understanding of the question correct?

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

It sounds like yes, you can still view the live tab if it's been downloaded
offline. With the current patch if you're on the live tab it shares the URL
rather than the offline page. In my opinion, that's the correct behavior and we
shouldn't try to share the offline page if the user is on the live tab.

+                    if (url == null) url = currentTab.getUrl();
+
+                    ShareHelper.share(shareDirectly, saveLastUsed, mainActivity,
+                            currentTab.getTitle(), text, url, null, bitmap, callback);
+                    if (shareDirectly) {
+                        RecordUserAction.record("MobileMenuDirectShare");
+                    } else {
+                        RecordUserAction.record("MobileMenuShare");
                     }
-                };
+                } else {
+                    // This is for experienmental feature Offline Page Sharing.
+                    // If this page is an offline page, and the share the offline page

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

"... an offline page then share the page instead of the original url."?

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

I accidentally deleted a part of the comment it in the previous CL.
Changed according to Dan's suggestion.

+                    // instead of url.

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

1) Don't need to say it's experimental.  Just use this instead:
   // Share the offline page instead of the URL.

3) nit: Put the positive part of the conditional on top instead of the negative.

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

Done.

+                    Context mContext = getApplicationContext();

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

Variables starting with "m" indicate that they're member variables, which this
isn't.  You should just be able to use mainActivity for the context, anyway.

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

Done.

+                    OfflinePageUtils.shareOfflinePage(shareDirectly, saveLastUsed, mainActivity,
+                            text, url, bitmap, callback, mContext, currentTab);
+                }
+            }
+        };
         if (isIncognito || currentTab.getWebContents() == null) {
             callback.onFinishGetBitmap(null, ReadbackResponse.SURFACE_UNAVAILABLE);
         } else {