Commit 0332c0d4 authored by Pete Williamson's avatar Pete Williamson Committed by Commit Bot

Pass through content URI when we have one.

Sometimes offline pages are displayed from a content URI instead of
opening a file from our offline page store.  The user might then press
the "share" menu.  When they do, we should use the content URI to
share the page by instead of using a file path (since we have no file path).

This change will pass back the content URI instead of just disabling sharing.
Unit tests are included.

Bug: 817608
Change-Id: I1d788a83de3aafe978b37374ec22d77ad328b59d
Reviewed-on: https://chromium-review.googlesource.com/962990
Commit-Queue: Peter Williamson <petewil@chromium.org>
Reviewed-by: default avatarJian Li <jianli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545115}
parent 8b1e1f26
...@@ -9,6 +9,7 @@ import android.content.Context; ...@@ -9,6 +9,7 @@ import android.content.Context;
import android.net.Uri; import android.net.Uri;
import android.os.AsyncTask; import android.os.AsyncTask;
import android.os.Environment; import android.os.Environment;
import android.text.TextUtils;
import org.chromium.base.ActivityState; import org.chromium.base.ActivityState;
import org.chromium.base.ApplicationStatus; import org.chromium.base.ApplicationStatus;
...@@ -369,19 +370,27 @@ public class OfflinePageUtils { ...@@ -369,19 +370,27 @@ public class OfflinePageUtils {
OfflinePageItem offlinePage = offlinePageBridge.getOfflinePage(tab.getWebContents()); OfflinePageItem offlinePage = offlinePageBridge.getOfflinePage(tab.getWebContents());
String offlinePath = offlinePage.getFilePath(); String offlinePath = offlinePage.getFilePath();
if (!isOfflinePageShareable(offlinePageBridge, offlinePage)) return false; final String pageUrl = tab.getUrl();
if (!isOfflinePageShareable(offlinePageBridge, offlinePage, pageUrl)) return false;
final String tabTitle = tab.getTitle(); final String tabTitle = tab.getTitle();
final String tabUrl = tab.getUrl();
final File offlinePageFile = new File(offlinePath); final File offlinePageFile = new File(offlinePath);
AsyncTask<Void, Void, Uri> task = new AsyncTask<Void, Void, Uri>() { AsyncTask<Void, Void, Uri> task = new AsyncTask<Void, Void, Uri>() {
@Override @Override
protected Uri doInBackground(Void... v) { protected Uri doInBackground(Void... v) {
// If we have a content or file URI, we will not have a filename, just return the
// URI.
if (offlinePath.isEmpty()) {
Uri uri = Uri.parse(pageUrl);
assert(isSchemeContentOrFile(uri));
return uri;
}
return ChromeFileProvider.generateUri(activity, offlinePageFile); return ChromeFileProvider.generateUri(activity, offlinePageFile);
} }
@Override @Override
protected void onPostExecute(Uri uri) { protected void onPostExecute(Uri uri) {
ShareParams shareParams = new ShareParams.Builder(activity, tabTitle, tabUrl) ShareParams shareParams = new ShareParams.Builder(activity, tabTitle, pageUrl)
.setShareDirectly(false) .setShareDirectly(false)
.setOfflineUri(uri) .setOfflineUri(uri)
.build(); .build();
...@@ -396,17 +405,29 @@ public class OfflinePageUtils { ...@@ -396,17 +405,29 @@ public class OfflinePageUtils {
/** /**
* Check to see if the offline page is sharable. * Check to see if the offline page is sharable.
* @param offlinePage Page to check for sharability. * @param offlinePage Page to check for sharability.
* @param pageUri Uri of the page to check
* @return true if this page can be shared. * @return true if this page can be shared.
*/ */
public static boolean isOfflinePageShareable( public static boolean isOfflinePageShareable(
OfflinePageBridge offlinePageBridge, OfflinePageItem offlinePage) { OfflinePageBridge offlinePageBridge, OfflinePageItem offlinePage, String pageUri) {
// Return false if there is no offline page or sharing is not enabled. // Return false if there is no offline page or sharing is not enabled.
if (offlinePage == null || !OfflinePageBridge.isPageSharingEnabled()) return false; if (offlinePage == null || !OfflinePageBridge.isPageSharingEnabled()) return false;
// We cannot share a file without a file path, so return false.
// TODO(petewil) Allow sharing if there is a content or file URI, even if there is no path.
// https://crbug.com/817608
String offlinePath = offlinePage.getFilePath(); String offlinePath = offlinePage.getFilePath();
Uri uri = Uri.parse(pageUri);
// If we have a content or file Uri, then we can share the page.
if (isSchemeContentOrFile(uri)) {
assert offlinePath.isEmpty();
return true;
}
// If the scheme is not one we recognize, return false.
if (!TextUtils.equals(uri.getScheme(), UrlConstants.HTTP_SCHEME)
&& !TextUtils.equals(uri.getScheme(), UrlConstants.HTTPS_SCHEME))
return false;
// If we have a http or https page with no file path, we cannot share it.
if (offlinePath.isEmpty()) { if (offlinePath.isEmpty()) {
Log.w(TAG, "Tried to share a page with no path."); Log.w(TAG, "Tried to share a page with no path.");
return false; return false;
...@@ -418,6 +439,14 @@ public class OfflinePageUtils { ...@@ -418,6 +439,14 @@ public class OfflinePageUtils {
return true; return true;
} }
// Returns true if the scheme of the URI is either content or file.
private static boolean isSchemeContentOrFile(Uri uri) {
boolean isContentScheme = TextUtils.equals(uri.getScheme(), UrlConstants.CONTENT_SCHEME);
boolean isFileScheme = TextUtils.equals(uri.getScheme(), UrlConstants.FILE_SCHEME);
return isContentScheme || isFileScheme;
}
/** /**
* Retrieves the extra request header to reload the offline page. * Retrieves the extra request header to reload the offline page.
* @param tab The current tab. * @param tab The current tab.
......
...@@ -59,6 +59,10 @@ public class OfflinePageUtilsTest { ...@@ -59,6 +59,10 @@ public class OfflinePageUtilsTest {
private static final ClientId ASYNC_ID = private static final ClientId ASYNC_ID =
new ClientId(OfflinePageBridge.ASYNC_NAMESPACE, "5678"); new ClientId(OfflinePageBridge.ASYNC_NAMESPACE, "5678");
private static final String SHARED_URI = "http://127.0.0.1/chrome/test/data/android/about.html"; private static final String SHARED_URI = "http://127.0.0.1/chrome/test/data/android/about.html";
private static final String CONTENT_URI = "content://chromium/some-content-id";
private static final String FILE_URI = "file://some-dir/some-file.mhtml";
private static final String INVALID_URI = "This is not a uri.";
private static final String EMPTY_URI = "";
private static final String EMPTY_PATH = ""; private static final String EMPTY_PATH = "";
private static final String CACHE_SUBDIR = "/Offline Pages/archives"; private static final String CACHE_SUBDIR = "/Offline Pages/archives";
private static final String NEW_FILE = "/newfile.mhtml"; private static final String NEW_FILE = "/newfile.mhtml";
...@@ -323,18 +327,19 @@ public class OfflinePageUtilsTest { ...@@ -323,18 +327,19 @@ public class OfflinePageUtilsTest {
} }
// Checks on the UI thread if an offline path corresponds to a sharable file. // Checks on the UI thread if an offline path corresponds to a sharable file.
private void checkIfOfflinePageIsSharable(final String filePath, boolean sharable) { private void checkIfOfflinePageIsSharable(
final String filePath, final String uriPath, boolean sharable) {
ThreadUtils.runOnUiThreadBlocking(new Runnable() { ThreadUtils.runOnUiThreadBlocking(new Runnable() {
@Override @Override
public void run() { public void run() {
OfflinePageItem privateOfflinePageItem = new OfflinePageItem(SHARED_URI, OFFLINE_ID, OfflinePageItem privateOfflinePageItem =
OfflinePageBridge.ASYNC_NAMESPACE, PAGE_ID, TITLE, filePath, FILE_SIZE, 0, new OfflinePageItem(uriPath, OFFLINE_ID, OfflinePageBridge.ASYNC_NAMESPACE,
0, 0, REQUEST_ORIGIN); PAGE_ID, TITLE, filePath, FILE_SIZE, 0, 0, 0, REQUEST_ORIGIN);
OfflinePageBridge offlinePageBridge = OfflinePageBridge.getForProfile( OfflinePageBridge offlinePageBridge = OfflinePageBridge.getForProfile(
mActivityTestRule.getActivity().getActivityTab().getProfile()); mActivityTestRule.getActivity().getActivityTab().getProfile());
boolean isSharable = OfflinePageUtils.isOfflinePageShareable( boolean isSharable = OfflinePageUtils.isOfflinePageShareable(
offlinePageBridge, privateOfflinePageItem); offlinePageBridge, privateOfflinePageItem, uriPath);
Assert.assertEquals(sharable, isSharable); Assert.assertEquals(sharable, isSharable);
} }
}); });
...@@ -350,14 +355,26 @@ public class OfflinePageUtilsTest { ...@@ -350,14 +355,26 @@ public class OfflinePageUtilsTest {
// Check that an offline page item in the private directory is not sharable. // Check that an offline page item in the private directory is not sharable.
final String fullPrivatePath = privatePath + CACHE_SUBDIR + NEW_FILE; final String fullPrivatePath = privatePath + CACHE_SUBDIR + NEW_FILE;
checkIfOfflinePageIsSharable(fullPrivatePath, false); checkIfOfflinePageIsSharable(fullPrivatePath, SHARED_URI, false);
// Check that an offline page item with no file path is not sharable. // Check that an offline page item with no file path is not sharable.
checkIfOfflinePageIsSharable("", false); checkIfOfflinePageIsSharable(EMPTY_PATH, SHARED_URI, false);
// Check that a public offline page item with a file path is sharable. // Check that a public offline page item with a file path is sharable.
final String fullPublicPath = publicPath + NEW_FILE; final String fullPublicPath = publicPath + NEW_FILE;
checkIfOfflinePageIsSharable(fullPublicPath, true); checkIfOfflinePageIsSharable(fullPublicPath, SHARED_URI, true);
// Check that a page with a content URI and no file path is sharable.
checkIfOfflinePageIsSharable(EMPTY_PATH, CONTENT_URI, true);
// Check that a page with a file URI and no file path is sharable.
checkIfOfflinePageIsSharable(EMPTY_PATH, FILE_URI, true);
// Check that a malformed URI is not sharable.
checkIfOfflinePageIsSharable(EMPTY_PATH, INVALID_URI, false);
// Check that an empty URL is not sharable.
checkIfOfflinePageIsSharable(fullPublicPath, EMPTY_URI, false);
} }
private void loadPageAndSave(ClientId clientId) throws Exception { private void loadPageAndSave(ClientId clientId) throws Exception {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment