[Offline pages] Removing permission prompt when accessing offline page

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

Index: chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java b/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java
index 523e226f5f9a217ee39d53c42306ce8f65aa1610..9eb276209a292f1ae6514edd2dd88ff0c42e6ce7 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java
@@ -9,6 +9,8 @@ import android.net.ConnectivityManager;
 import android.net.NetworkInfo;
 import android.os.Environment;
 
+import org.chromium.base.ApplicationStatus;
+import org.chromium.base.PathUtils;
 import org.chromium.base.metrics.RecordUserAction;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ChromeActivity;
@@ -114,6 +116,19 @@ public class OfflinePageUtils {
     }
 
     /**
+     * Check whether the URL qualifies as offline URL for the purpose of storage permission check.
+     * @param url A URL to check.
+     * @return <code>true</code> if the URL qualifies as an offline page URL, <code>false</code>
+     *     otherwise.
+     */
+    public static boolean isOfflineUrlForStoragePermissionCheck(String url) {
+        Context context = ApplicationStatus.getApplicationContext();
+        return url != null && context != null
+                && url.startsWith("file://" + PathUtils.getDataDirectory(context))

[newt (away), 2016/01/04 19:45:05]

This seems prone to silent breakage if the location of offline URLs changes. Can
you instead just get the offline page directory from the canonical (C++?) method
that provides said directory?

Since this code only runs for file:// URLs, I think a call into native is fine
and probably better than duplicating knowledge about the location on disk of
offline pages.

(Alternatively, can you simply update
ExternalNavigationDelegateImpl.shouldRequestFileAccess() to check whether the
URL points inside PathUtils.getDataDirectory()? Does Chrome need file
permissions for the renderer to access, say,
file:///data/data/com.google.android.apps.chrome/app_chrome/Default/Preferences
? I would expect not.)

[jianli, 2016/01/04 21:42:00]

The name isOfflineUrlForStoragePermissionCheck sounds too restrictive. I don't
think this needs to be limited to storage permission check usage. I think we
want a version used for both C++ and Java that will tell if a given URL is very
likely an offline URL.

I agree with newt. It seems to be better to add more checks in C++'s
MightBeOfflineURL and expose it to Java.

[fgorski, 2016/01/04 23:11:35]

This sits inside of the profile folder and I am not seeing references to
profile/browser context here, so I'd prefer to avoid it.
I thought of that earlier, but the difficulty there is that we only pass a tab
to the method, and not the url in question. Do you suggest getting it form the
tab somehow or changing the interface to the Delegate?
Let's explore that option deeper (I think we have a basic agreement as to how to
do it, based on our conversation) once I hear from Newton about the other
option.

+                && url.contains("/Offline%20Pages/archives/") && url.endsWith(".mhtml");
+    }
+
+    /**
      * Shows the snackbar for the current tab to provide offline specific information.
      * @param activity The activity owning the tab.
      * @param tabId The ID of current tab.