Request storage permission for saving offline pages by users

Bug: 758690
Change-Id: Iaffd4a3bd4327a601732b29b63837a2d63d4802c
Reviewed-on: https://chromium-review.googlesource.com/905394Reviewed-by: Dmitry Titov <dimich@chromium.org>
Commit-Queue: Jian Li <jianli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535847}

chrome/browser/offline_pages/android/downloads/offline_page_download_bridge.cc

@@ -21,6 +21,7 @@
 #include "chrome/browser/offline_pages/offline_page_utils.h"
 #include "chrome/browser/offline_pages/recent_tab_helper.h"
 #include "chrome/browser/offline_pages/request_coordinator_factory.h"
+#include "chrome/browser/profiles/incognito_helpers.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_android.h"
 #include "components/offline_items_collection/core/offline_content_aggregator.h"
@@ -224,20 +225,7 @@ content::ResourceRequestInfo::WebContentsGetter GetWebContentsGetter(
                     web_contents->GetMainFrame()->GetRoutingID());
 }
 
-void OnOfflinePageAcquireFileAccessPermissionDone(
-    const content::ResourceRequestInfo::WebContentsGetter& web_contents_getter,
-    bool granted) {
-  if (!granted)
-    return;
-
-  content::WebContents* web_contents = web_contents_getter.Run();
-  if (!web_contents)
-    return;
-
-  GURL url = web_contents->GetLastCommittedURL();
-  if (url.is_empty())
-    return;
-
+void DownloadAsFile(content::WebContents* web_contents, const GURL& url) {
   content::DownloadManager* dlm = content::BrowserContext::GetDownloadManager(
       web_contents->GetBrowserContext());
   std::unique_ptr<content::DownloadUrlParameters> dl_params(
@@ -259,6 +247,40 @@ void OnOfflinePageAcquireFileAccessPermissionDone(
   dlm->DownloadUrl(std::move(dl_params));
 }
 
+void OnOfflinePageAcquireFileAccessPermissionDone(
+    const content::ResourceRequestInfo::WebContentsGetter& web_contents_getter,
+    const ScopedJavaGlobalRef<jobject>& j_tab_ref,
+    const std::string& origin,
+    bool granted) {
+  if (!granted)
+    return;
+
+  content::WebContents* web_contents = web_contents_getter.Run();
+  if (!web_contents)
+    return;
+
+  GURL url = web_contents->GetLastCommittedURL();
+  if (url.is_empty())
+    return;
+
+  // If the page is not a HTML page, route to DownloadManager.
+  if (!offline_pages::OfflinePageUtils::CanDownloadAsOfflinePage(
+          url, web_contents->GetContentsMimeType())) {
+    DownloadAsFile(web_contents, url);
+    return;
+  }
+
+  // Otherwise, save the HTML page as archive.
+  GURL original_url =
+      offline_pages::OfflinePageUtils::GetOriginalURLFromWebContents(
+          web_contents);
+  OfflinePageUtils::CheckDuplicateDownloads(
+      chrome::GetBrowserContextRedirectedInIncognito(
+          web_contents->GetBrowserContext()),
+      url,
+      base::Bind(&DuplicateCheckDone, url, original_url, j_tab_ref, origin));
+}
+
 }  // namespace
 
 OfflinePageDownloadBridge::OfflinePageDownloadBridge(
@@ -286,32 +308,17 @@ void JNI_OfflinePageDownloadBridge_StartDownload(
   if (!web_contents)
     return;
 
-  GURL url = web_contents->GetLastCommittedURL();
-  if (url.is_empty())
-    return;
   std::string origin = ConvertJavaStringToUTF8(env, j_origin);
-
-  GURL original_url =
-      offline_pages::OfflinePageUtils::GetOriginalURLFromWebContents(
-          web_contents);
-
-  // If the page is not a HTML page, route to DownloadManager.
-  if (!offline_pages::OfflinePageUtils::CanDownloadAsOfflinePage(
-          url, web_contents->GetContentsMimeType())) {
-    content::ResourceRequestInfo::WebContentsGetter web_contents_getter =
-        GetWebContentsGetter(web_contents);
-    DownloadControllerBase::Get()->AcquireFileAccessPermission(
-        web_contents_getter,
-        base::Bind(&OnOfflinePageAcquireFileAccessPermissionDone,
-                   web_contents_getter));
-    return;
-  }
-
   ScopedJavaGlobalRef<jobject> j_tab_ref(env, j_tab);
 
-  OfflinePageUtils::CheckDuplicateDownloads(
-      tab->GetProfile()->GetOriginalProfile(), url,
-      base::Bind(&DuplicateCheckDone, url, original_url, j_tab_ref, origin));
+  // Ensure that the storage permission is granted since the target file
+  // is going to be placed in the public directory.
+  content::ResourceRequestInfo::WebContentsGetter web_contents_getter =
+      GetWebContentsGetter(web_contents);
+  DownloadControllerBase::Get()->AcquireFileAccessPermission(
+      web_contents_getter,
+      base::Bind(&OnOfflinePageAcquireFileAccessPermissionDone,
+                 web_contents_getter, j_tab_ref, origin));
 }
 
 static jlong JNI_OfflinePageDownloadBridge_Init(

chrome/browser/offline_pages/offline_page_utils.cc

@@ -31,9 +31,15 @@
 #include "components/offline_pages/core/request_header/offline_page_header.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/navigation_entry.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_contents.h"
 #include "net/base/mime_util.h"
 
+#if defined(OS_ANDROID)
+#include "chrome/browser/android/download/download_controller_base.h"
+#endif  // defined(OS_ANDROID)
+
 namespace offline_pages {
 namespace {
 
@@ -139,6 +145,48 @@ void DoCalculateSizeBetween(
   callback.Run(total_size);
 }
 
+content::WebContents* GetWebContentsByFrameID(int render_process_id,
+                                              int render_frame_id) {
+  content::RenderFrameHost* render_frame_host =
+      content::RenderFrameHost::FromID(render_process_id, render_frame_id);
+  if (!render_frame_host)
+    return NULL;
+  return content::WebContents::FromRenderFrameHost(render_frame_host);
+}
+
+content::ResourceRequestInfo::WebContentsGetter GetWebContentsGetter(
+    content::WebContents* web_contents) {
+  // PlzNavigate: The FrameTreeNode ID should be used to access the WebContents.
+  int frame_tree_node_id = web_contents->GetMainFrame()->GetFrameTreeNodeId();
+  if (frame_tree_node_id != -1) {
+    return base::Bind(content::WebContents::FromFrameTreeNodeId,
+                      frame_tree_node_id);
+  }
+
+  // In other cases, use the RenderProcessHost ID + RenderFrameHost ID to get
+  // the WebContents.
+  return base::Bind(&GetWebContentsByFrameID,
+                    web_contents->GetMainFrame()->GetProcess()->GetID(),
+                    web_contents->GetMainFrame()->GetRoutingID());
+}
+
+void AcquireFileAccessPermissionDoneForScheduleDownload(
+    content::WebContents* web_contents,
+    const std::string& name_space,
+    const GURL& url,
+    OfflinePageUtils::DownloadUIActionFlags ui_action,
+    const std::string& request_origin,
+    bool granted) {
+  if (!granted)
+    return;
+  OfflinePageTabHelper* tab_helper =
+      OfflinePageTabHelper::FromWebContents(web_contents);
+  if (!tab_helper)
+    return;
+  tab_helper->ScheduleDownloadHelper(web_contents, name_space, url, ui_action,
+                                     request_origin);
+}
+
 }  // namespace
 
 // static
@@ -289,12 +337,19 @@ void OfflinePageUtils::ScheduleDownload(content::WebContents* web_contents,
                                         const std::string& request_origin) {
   DCHECK(web_contents);
 
-  OfflinePageTabHelper* tab_helper =
-      OfflinePageTabHelper::FromWebContents(web_contents);
-  if (!tab_helper)
-    return;
-  tab_helper->ScheduleDownloadHelper(web_contents, name_space, url, ui_action,
-                                     request_origin);
+// Ensure that the storage permission is granted since the archive file is
+// going to be placed in the public directory.
+#if defined(OS_ANDROID)
+  content::ResourceRequestInfo::WebContentsGetter web_contents_getter =
+      GetWebContentsGetter(web_contents);
+  DownloadControllerBase::Get()->AcquireFileAccessPermission(
+      web_contents_getter,
+      base::Bind(&AcquireFileAccessPermissionDoneForScheduleDownload,
+                 web_contents, name_space, url, ui_action, request_origin));
+#else
+  AcquireFileAccessPermissionDoneForScheduleDownload(
+      web_contents, name_space, url, ui_action, origin, true /*granted*/);
+#endif  // defined(OS_ANDROID)
 }
 
 // static

chrome/browser/offline_pages/offline_page_utils_unittest.cc

@@ -19,6 +19,7 @@
 #include "base/test/simple_test_clock.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
+#include "build/build_config.h"
 #include "chrome/browser/offline_pages/offline_page_model_factory.h"
 #include "chrome/browser/offline_pages/offline_page_tab_helper.h"
 #include "chrome/browser/offline_pages/request_coordinator_factory.h"
@@ -42,6 +43,10 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
+#if defined(OS_ANDROID)
+#include "chrome/browser/android/download/mock_download_controller.h"
+#endif
+
 namespace offline_pages {
 namespace {
 
@@ -81,6 +86,7 @@ class OfflinePageUtilsTest
   ~OfflinePageUtilsTest() override;
 
   void SetUp() override;
+  void TearDown() override;
   void RunUntilIdle();
 
   void SavePage(const GURL& url,
@@ -123,6 +129,9 @@ class OfflinePageUtilsTest
   std::unique_ptr<content::WebContents> web_contents_;
   base::test::ScopedFeatureList scoped_feature_list_;
   int64_t last_cache_size_;
+#if defined(OS_ANDROID)
+  chrome::android::MockDownloadController download_controller_;
+#endif
 };
 
 OfflinePageUtilsTest::OfflinePageUtilsTest() = default;
@@ -149,6 +158,17 @@ void OfflinePageUtilsTest::SetUp() {
   // Make sure to create offline pages and requests.
   CreateOfflinePages();
   CreateRequests();
+
+// This is needed in order to skip the logic to request storage permission.
+#if defined(OS_ANDROID)
+  DownloadControllerBase::SetDownloadControllerBase(&download_controller_);
+#endif
+}
+
+void OfflinePageUtilsTest::TearDown() {
+#if defined(OS_ANDROID)
+  DownloadControllerBase::SetDownloadControllerBase(nullptr);
+#endif
 }
 
 void OfflinePageUtilsTest::RunUntilIdle() {
@@ -345,6 +365,17 @@ TEST_F(OfflinePageUtilsTest, ScheduleDownload) {
   EXPECT_EQ(1, FindRequestByNamespaceAndURL(kDownloadNamespace, kTestPage4Url));
 }
 
+#if defined(OS_ANDROID)
+TEST_F(OfflinePageUtilsTest, ScheduleDownloadWithFailedFileAcecssRequest) {
+  DownloadControllerBase::Get()->SetApproveFileAccessRequestForTesting(false);
+  OfflinePageUtils::ScheduleDownload(
+      web_contents(), kDownloadNamespace, kTestPage4Url,
+      OfflinePageUtils::DownloadUIActionFlags::NONE);
+  RunUntilIdle();
+  EXPECT_EQ(0, FindRequestByNamespaceAndURL(kDownloadNamespace, kTestPage4Url));
+}
+#endif
+
 TEST_F(OfflinePageUtilsTest, EqualsIgnoringFragment) {
   EXPECT_TRUE(OfflinePageUtils::EqualsIgnoringFragment(
       GURL("http://example.com/"), GURL("http://example.com/")));