Allow sharing of an offline page from Downloads Home

Allows a share-able offline page to be shared from long press or per
item 3-dot menu from the Downloads Home, but still gated by the offline
page sharing feature being enabled. Tests are also updated to check
sharing is working in this condition.

Bug: 758690
Change-Id: I6f65384b8f190cb037355edafdb9cbbdabd0769c
Reviewed-on: https://chromium-review.googlesource.com/922869
Commit-Queue: Dmitry Titov <dimich@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Dmitry Titov <dimich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537181}

chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java

@@ -326,7 +326,7 @@ public class DownloadUtils {
         for (int i = 0; i < items.size(); i++) {
             DownloadHistoryItemWrapper wrappedItem  = items.get(i);
 
-            if (wrappedItem.isOfflinePage()) {
+            if (wrappedItem.isOfflinePage() && !OfflinePageBridge.isPageSharingEnabled()) {
                 if (offlinePagesString.length() != 0) {
                     offlinePagesString.append("\n");
                 }
@@ -390,7 +390,7 @@ public class DownloadUtils {
         }
 
         if (itemUris.size() == 1 && offlinePagesString.length() == 0) {
-            // Sharing a DownloadItem.
+            // Sharing a downloaded item or an offline page.
             shareIntent.putExtra(Intent.EXTRA_STREAM, getUriForItem(items.get(0).getFile()));
         } else {
             shareIntent.putParcelableArrayListExtra(Intent.EXTRA_STREAM, itemUris);

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

@@ -58,7 +58,6 @@ public class OfflinePageBridge {
 
     /** Whether an offline sub-feature is enabled or not. */
     private static Boolean sOfflineBookmarksEnabled;
-    private static Boolean sIsPageSharingEnabled;
 
     /**
      * Callback used when saving an offline page.
@@ -132,11 +131,7 @@ public class OfflinePageBridge {
      */
     @VisibleForTesting
     public static boolean isPageSharingEnabled() {
-        ThreadUtils.assertOnUiThread();
-        if (sIsPageSharingEnabled == null) {
-            sIsPageSharingEnabled = nativeIsPageSharingEnabled();
-        }
-        return sIsPageSharingEnabled;
+        return nativeIsPageSharingEnabled();
     }
 
     /**

chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadActivityTest.java

@@ -23,6 +23,7 @@ import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TestRule;
 import org.junit.runner.RunWith;
 
 import org.chromium.base.ContextUtils;
@@ -45,6 +46,8 @@ import org.chromium.chrome.browser.util.IntentUtils;
 import org.chromium.chrome.browser.widget.ListMenuButton.Item;
 import org.chromium.chrome.browser.widget.selection.SelectionDelegate.SelectionObserver;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
+import org.chromium.chrome.test.util.browser.Features;
+import org.chromium.chrome.test.util.browser.Features.EnableFeatures;
 import org.chromium.components.offline_items_collection.OfflineItem;
 import org.chromium.content.browser.test.util.Criteria;
 import org.chromium.content.browser.test.util.CriteriaHelper;
@@ -62,6 +65,9 @@ public class DownloadActivityTest {
     public ActivityTestRule<DownloadActivity> mActivityTestRule =
             new ActivityTestRule<>(DownloadActivity.class);
 
+    @Rule
+    public TestRule mProcessor = new Features.InstrumentationProcessor();
+
     private static class TestObserver extends RecyclerView.AdapterDataObserver
             implements SelectionObserver<DownloadHistoryItemWrapper>,
                     DownloadManagerUi.DownloadUiObserver, SpaceDisplay.Observer {
@@ -577,6 +583,34 @@ public class DownloadActivityTest {
                 IntentUtils.safeGetStringExtra(shareIntent, Intent.EXTRA_TEXT));
     }
 
+    // TODO(carlosk): OfflineItems used here come from StubbedProvider so this might not be the best
+    // place to test peer-2-peer sharing.
+    @Test
+    @MediumTest
+    @EnableFeatures("OfflinePagesSharing")
+    public void testShareOfflinePageWithP2PSharingEnabled() throws Exception {
+        // Scroll to ensure the item at position 2 is visible.
+        ThreadUtils.runOnUiThread(() -> mRecyclerView.scrollToPosition(3));
+        InstrumentationRegistry.getInstrumentation().waitForIdleSync();
+
+        // Select the offline page located at position #3.
+        toggleItemSelection(3);
+        List<DownloadHistoryItemWrapper> selected_items =
+                mUi.getBackendProvider().getSelectionDelegate().getSelectedItems();
+        Assert.assertEquals("There should be only one item selected", 1, selected_items.size());
+        Intent shareIntent = DownloadUtils.createShareIntent(selected_items);
+
+        Assert.assertEquals("Incorrect intent action", Intent.ACTION_SEND, shareIntent.getAction());
+        Assert.assertEquals("Incorrect intent mime type", "*/*", shareIntent.getType());
+        Assert.assertNotNull("Intent expected to have parcelable ArrayList",
+                shareIntent.getParcelableExtra(Intent.EXTRA_STREAM));
+        Assert.assertEquals("Intent expected to have parcelable Uri",
+                "file:///data/fake_path/Downloads/4",
+                shareIntent.getParcelableExtra(Intent.EXTRA_STREAM).toString());
+        Assert.assertNull("Intent expected to not have any text for offline page",
+                IntentUtils.safeGetStringExtra(shareIntent, Intent.EXTRA_TEXT));
+    }
+
     @Test
     @MediumTest
     public void testToggleSelection() throws Exception {

components/offline_pages/core/downloads/BUILD.gn

@@ -21,6 +21,7 @@ static_library("offline_pages_ui_adapter") {
     "//base",
     "//components/offline_items_collection/core",
     "//components/offline_pages/core",
+    "//components/offline_pages/core:switches",
     "//components/offline_pages/core/background:background_offliner",
     "//url",
   ]
@@ -40,6 +41,7 @@ source_set("unit_tests") {
     "//base/test:test_support",
     "//components/offline_items_collection/core",
     "//components/offline_pages/core",
+    "//components/offline_pages/core:switches",
     "//components/offline_pages/core:test_support",
     "//components/offline_pages/core/background:background_offliner",
     "//components/offline_pages/core/background:test_support",

components/offline_pages/core/downloads/offline_item_conversions.cc

@@ -7,6 +7,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "components/offline_items_collection/core/pending_state.h"
 #include "components/offline_pages/core/background/save_page_request.h"
+#include "components/offline_pages/core/offline_page_feature.h"
 #include "components/offline_pages/core/offline_page_item.h"
 
 using OfflineItemFilter = offline_items_collection::OfflineItemFilter;
@@ -15,6 +16,15 @@ using OfflineItemProgressUnit =
     offline_items_collection::OfflineItemProgressUnit;
 using PendingState = offline_items_collection::PendingState;
 
+namespace {
+
+const std::string GetMimeType() {
+  return offline_pages::IsOfflinePagesSharingEnabled() ? "multipart/related"
+                                                       : "text/html";
+}
+
+}  // namespace
+
 namespace offline_pages {
 
 OfflineItem OfflineItemConversions::CreateOfflineItem(
@@ -29,7 +39,7 @@ OfflineItem OfflineItemConversions::CreateOfflineItem(
   item.creation_time = page.creation_time;
   item.last_accessed_time = page.last_access_time;
   item.file_path = page.file_path;
-  item.mime_type = "text/html";
+  item.mime_type = GetMimeType();
   item.page_url = page.url;
   item.original_url = page.original_url;
   item.progress.value = 100;
@@ -48,7 +58,7 @@ OfflineItem OfflineItemConversions::CreateOfflineItem(
   item.creation_time = request.creation_time();
   item.total_size_bytes = -1L;
   item.received_bytes = 0;
-  item.mime_type = "text/html";
+  item.mime_type = GetMimeType();
   item.page_url = request.url();
   item.original_url = request.original_url();
   switch (request.request_state()) {

components/offline_pages/core/downloads/offline_item_conversions.h

@@ -7,8 +7,6 @@
 
 #include "components/offline_items_collection/core/offline_item.h"
 
-const char kOfflinePageNamespace[] = "LEGACY_OFFLINE_PAGE";
-
 using ContentId = offline_items_collection::ContentId;
 using OfflineItem = offline_items_collection::OfflineItem;
 
@@ -17,6 +15,8 @@ namespace offline_pages {
 struct OfflinePageItem;
 class SavePageRequest;
 
+const char kOfflinePageNamespace[] = "LEGACY_OFFLINE_PAGE";
+
 // Allows to convert between internal offline pages types and their offline item
 // collection representation (for displaying in UI).
 class OfflineItemConversions {

components/offline_pages/core/downloads/offline_item_conversions_unittest.cc

@@ -5,7 +5,9 @@
 #include "components/offline_pages/core/downloads/offline_item_conversions.h"
 
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/scoped_feature_list.h"
 #include "components/offline_pages/core/background/save_page_request.h"
+#include "components/offline_pages/core/offline_page_feature.h"
 #include "components/offline_pages/core/offline_page_item.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -53,6 +55,21 @@ TEST(OfflineItemConversionsTest, OfflinePageItemConversion) {
   EXPECT_EQ(100, offline_item.progress.max.value());
   EXPECT_EQ(OfflineItemProgressUnit::PERCENTAGE, offline_item.progress.unit);
   EXPECT_TRUE(offline_item.is_suggested);
+
+  // Enabled P2P sharing and flag the item as suggested when creating the
+  // OfflineItem. Then check that only the mime type is and is_suggested
+  // information changed.
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(kOfflinePagesSharingFeature);
+  OfflineItem offline_item_p2p =
+      OfflineItemConversions::CreateOfflineItem(offline_page_item, false);
+  EXPECT_EQ("multipart/related", offline_item_p2p.mime_type);
+  EXPECT_FALSE(offline_item_p2p.is_suggested);
+
+  // Change offline_item_p2p to match offline_item and check that it does.
+  offline_item_p2p.mime_type = "text/html";
+  offline_item_p2p.is_suggested = true;
+  EXPECT_EQ(offline_item, offline_item_p2p);
 }
 
 TEST(OfflineItemConversionsTest, SavePageRequestConversion) {
@@ -83,6 +100,18 @@ TEST(OfflineItemConversionsTest, SavePageRequestConversion) {
   EXPECT_FALSE(offline_item.progress.max.has_value());
   EXPECT_EQ(OfflineItemProgressUnit::PERCENTAGE, offline_item.progress.unit);
   EXPECT_FALSE(offline_item.is_suggested);
+
+  // Enabled P2P sharing of offline pages and check that only the mime type is
+  // different.
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(kOfflinePagesSharingFeature);
+  OfflineItem offline_item_p2p =
+      OfflineItemConversions::CreateOfflineItem(save_page_request);
+  EXPECT_EQ("multipart/related", offline_item_p2p.mime_type);
+
+  // Change offline_item_p2p to match offline_item and check that it does.
+  offline_item_p2p.mime_type = "text/html";
+  EXPECT_EQ(offline_item, offline_item_p2p);
 }
 
 }  // namespace offline_pages