Commit 2e063dfe authored by Clark DuVall's avatar Clark DuVall Committed by Commit Bot

[WebLayer] Fix crash when opening download on Android Q

This also fixes another related bug where the filename was incorrect in
the notification being displayed on Q. The issue is that Q already has
the location in the form of a content URI, so we can just use that
directly.

Bug: 1111448
Change-Id: I20485efab2a93abd439bd972d00bde908183aa2f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2330943Reviewed-by: default avatarJohn Abd-El-Malek <jam@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#793611}
parent f368f2ba
...@@ -27,6 +27,7 @@ import org.chromium.weblayer.DownloadCallback; ...@@ -27,6 +27,7 @@ import org.chromium.weblayer.DownloadCallback;
import org.chromium.weblayer.DownloadError; import org.chromium.weblayer.DownloadError;
import org.chromium.weblayer.DownloadState; import org.chromium.weblayer.DownloadState;
import org.chromium.weblayer.Profile; import org.chromium.weblayer.Profile;
import org.chromium.weblayer.WebLayer;
import org.chromium.weblayer.shell.InstrumentationActivity; import org.chromium.weblayer.shell.InstrumentationActivity;
import java.io.File; import java.io.File;
...@@ -42,6 +43,8 @@ public class DownloadCallbackTest { ...@@ -42,6 +43,8 @@ public class DownloadCallbackTest {
public InstrumentationActivityTestRule mActivityTestRule = public InstrumentationActivityTestRule mActivityTestRule =
new InstrumentationActivityTestRule(); new InstrumentationActivityTestRule();
private static boolean sIsFileNameSupported;
private InstrumentationActivity mActivity; private InstrumentationActivity mActivity;
private Callback mCallback; private Callback mCallback;
...@@ -51,6 +54,7 @@ public class DownloadCallbackTest { ...@@ -51,6 +54,7 @@ public class DownloadCallbackTest {
public String mContentDisposition; public String mContentDisposition;
public String mMimetype; public String mMimetype;
public String mLocation; public String mLocation;
public String mFileName;
public @DownloadState int mState; public @DownloadState int mState;
public @DownloadError int mError; public @DownloadError int mError;
public long mContentLength; public long mContentLength;
...@@ -86,6 +90,9 @@ public class DownloadCallbackTest { ...@@ -86,6 +90,9 @@ public class DownloadCallbackTest {
public void onDownloadCompleted(Download download) { public void onDownloadCompleted(Download download) {
mSeenCompleted = true; mSeenCompleted = true;
mLocation = download.getLocation().toString(); mLocation = download.getLocation().toString();
if (sIsFileNameSupported) {
mFileName = download.getFileNameToReportToUser().toString();
}
mState = download.getState(); mState = download.getState();
mError = download.getError(); mError = download.getError();
mMimetype = download.getMimeType(); mMimetype = download.getMimeType();
...@@ -131,6 +138,9 @@ public class DownloadCallbackTest { ...@@ -131,6 +138,9 @@ public class DownloadCallbackTest {
Profile profile = mActivity.getBrowser().getProfile(); Profile profile = mActivity.getBrowser().getProfile();
profile.setDownloadCallback(mCallback); profile.setDownloadCallback(mCallback);
profile.setDownloadDirectory(new File(tempDownloadDirectory)); profile.setDownloadDirectory(new File(tempDownloadDirectory));
sIsFileNameSupported =
WebLayer.getSupportedMajorVersion(mActivity.getApplicationContext()) >= 86;
}); });
} }
...@@ -196,6 +206,9 @@ public class DownloadCallbackTest { ...@@ -196,6 +206,9 @@ public class DownloadCallbackTest {
Assert.assertTrue(mCallback.mLocation.contains( Assert.assertTrue(mCallback.mLocation.contains(
"org.chromium.weblayer.shell/cache/weblayer/Downloads/")); "org.chromium.weblayer.shell/cache/weblayer/Downloads/"));
if (sIsFileNameSupported) {
Assert.assertTrue(mCallback.mFileName.contains("test"));
}
Assert.assertEquals(DownloadState.COMPLETE, mCallback.mState); Assert.assertEquals(DownloadState.COMPLETE, mCallback.mState);
Assert.assertEquals(DownloadError.NO_ERROR, mCallback.mError); Assert.assertEquals(DownloadError.NO_ERROR, mCallback.mError);
Assert.assertEquals("text/html", mCallback.mMimetype); Assert.assertEquals("text/html", mCallback.mMimetype);
......
...@@ -54,6 +54,13 @@ base::android::ScopedJavaLocalRef<jstring> DownloadImpl::GetLocation( ...@@ -54,6 +54,13 @@ base::android::ScopedJavaLocalRef<jstring> DownloadImpl::GetLocation(
base::android::ConvertUTF8ToJavaString(env, GetLocation().value())); base::android::ConvertUTF8ToJavaString(env, GetLocation().value()));
} }
base::android::ScopedJavaLocalRef<jstring>
DownloadImpl::GetFileNameToReportToUser(JNIEnv* env) {
return base::android::ScopedJavaLocalRef<jstring>(
base::android::ConvertUTF8ToJavaString(
env, GetFileNameToReportToUser().value()));
}
base::android::ScopedJavaLocalRef<jstring> DownloadImpl::GetMimeTypeImpl( base::android::ScopedJavaLocalRef<jstring> DownloadImpl::GetMimeTypeImpl(
JNIEnv* env) { JNIEnv* env) {
return base::android::ScopedJavaLocalRef<jstring>( return base::android::ScopedJavaLocalRef<jstring>(
...@@ -117,6 +124,10 @@ base::FilePath DownloadImpl::GetLocation() { ...@@ -117,6 +124,10 @@ base::FilePath DownloadImpl::GetLocation() {
return item_->GetTargetFilePath(); return item_->GetTargetFilePath();
} }
base::FilePath DownloadImpl::GetFileNameToReportToUser() {
return item_->GetFileNameToReportUser();
}
std::string DownloadImpl::GetMimeType() { std::string DownloadImpl::GetMimeType() {
return item_->GetMimeType(); return item_->GetMimeType();
} }
......
...@@ -41,6 +41,8 @@ class DownloadImpl : public Download, public base::SupportsUserData::Data { ...@@ -41,6 +41,8 @@ class DownloadImpl : public Download, public base::SupportsUserData::Data {
void Resume(JNIEnv* env) { Resume(); } void Resume(JNIEnv* env) { Resume(); }
void Cancel(JNIEnv* env) { Cancel(); } void Cancel(JNIEnv* env) { Cancel(); }
base::android::ScopedJavaLocalRef<jstring> GetLocation(JNIEnv* env); base::android::ScopedJavaLocalRef<jstring> GetLocation(JNIEnv* env);
base::android::ScopedJavaLocalRef<jstring> GetFileNameToReportToUser(
JNIEnv* env);
// Add Impl suffix to avoid compiler clash with the C++ interface method. // Add Impl suffix to avoid compiler clash with the C++ interface method.
base::android::ScopedJavaLocalRef<jstring> GetMimeTypeImpl(JNIEnv* env); base::android::ScopedJavaLocalRef<jstring> GetMimeTypeImpl(JNIEnv* env);
int GetError(JNIEnv* env) { return static_cast<int>(GetError()); } int GetError(JNIEnv* env) { return static_cast<int>(GetError()); }
...@@ -58,6 +60,7 @@ class DownloadImpl : public Download, public base::SupportsUserData::Data { ...@@ -58,6 +60,7 @@ class DownloadImpl : public Download, public base::SupportsUserData::Data {
void Resume() override; void Resume() override;
void Cancel() override; void Cancel() override;
base::FilePath GetLocation() override; base::FilePath GetLocation() override;
base::FilePath GetFileNameToReportToUser() override;
std::string GetMimeType() override; std::string GetMimeType() override;
DownloadError GetError() override; DownloadError GetError() override;
......
...@@ -8,6 +8,7 @@ import android.content.ActivityNotFoundException; ...@@ -8,6 +8,7 @@ import android.content.ActivityNotFoundException;
import android.content.Context; import android.content.Context;
import android.content.Intent; import android.content.Intent;
import android.content.res.Resources; import android.content.res.Resources;
import android.net.Uri;
import android.os.RemoteException; import android.os.RemoteException;
import android.text.TextUtils; import android.text.TextUtils;
...@@ -91,10 +92,9 @@ public final class DownloadImpl extends IDownload.Stub { ...@@ -91,10 +92,9 @@ public final class DownloadImpl extends IDownload.Stub {
Intent openIntent = new Intent(Intent.ACTION_VIEW); Intent openIntent = new Intent(Intent.ACTION_VIEW);
if (TextUtils.isEmpty(mimeType)) { if (TextUtils.isEmpty(mimeType)) {
openIntent.setData(ContentUriUtils.getContentUriFromFile(new File(location))); openIntent.setData(getDownloadUri(location));
} else { } else {
openIntent.setDataAndType( openIntent.setDataAndType(getDownloadUri(location), mimeType);
ContentUriUtils.getContentUriFromFile(new File(location)), mimeType);
} }
openIntent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION); openIntent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION);
openIntent.addFlags(Intent.FLAG_GRANT_WRITE_URI_PERMISSION); openIntent.addFlags(Intent.FLAG_GRANT_WRITE_URI_PERMISSION);
...@@ -255,6 +255,13 @@ public final class DownloadImpl extends IDownload.Stub { ...@@ -255,6 +255,13 @@ public final class DownloadImpl extends IDownload.Stub {
return DownloadImplJni.get().getLocation(mNativeDownloadImpl); return DownloadImplJni.get().getLocation(mNativeDownloadImpl);
} }
@Override
public String getFileNameToReportToUser() {
StrictModeWorkaround.apply();
throwIfNativeDestroyed();
return DownloadImplJni.get().getFileNameToReportToUser(mNativeDownloadImpl);
}
@Override @Override
public String getMimeType() { public String getMimeType() {
StrictModeWorkaround.apply(); StrictModeWorkaround.apply();
...@@ -347,9 +354,9 @@ public final class DownloadImpl extends IDownload.Stub { ...@@ -347,9 +354,9 @@ public final class DownloadImpl extends IDownload.Stub {
.setPriorityBeforeO(NotificationCompat.PRIORITY_DEFAULT); .setPriorityBeforeO(NotificationCompat.PRIORITY_DEFAULT);
// The filename might not have been available initially. // The filename might not have been available initially.
String location = getLocation(); String name = getFileNameToReportToUser();
if (!TextUtils.isEmpty((location))) { if (!TextUtils.isEmpty(name)) {
builder.setContentTitle((new File(location)).getName()); builder.setContentTitle(name);
} }
if (state == DownloadState.CANCELLED) { if (state == DownloadState.CANCELLED) {
...@@ -453,6 +460,11 @@ public final class DownloadImpl extends IDownload.Stub { ...@@ -453,6 +460,11 @@ public final class DownloadImpl extends IDownload.Stub {
return new NotificationManagerProxyImpl(ContextUtils.getApplicationContext()); return new NotificationManagerProxyImpl(ContextUtils.getApplicationContext());
} }
private static Uri getDownloadUri(String location) {
if (ContentUriUtils.isContentUri(location)) return Uri.parse(location);
return ContentUriUtils.getContentUriFromFile(new File(location));
}
@CalledByNative @CalledByNative
private void onNativeDestroyed() { private void onNativeDestroyed() {
mNativeDownloadImpl = 0; mNativeDownloadImpl = 0;
...@@ -470,6 +482,7 @@ public final class DownloadImpl extends IDownload.Stub { ...@@ -470,6 +482,7 @@ public final class DownloadImpl extends IDownload.Stub {
void resume(long nativeDownloadImpl); void resume(long nativeDownloadImpl);
void cancel(long nativeDownloadImpl); void cancel(long nativeDownloadImpl);
String getLocation(long nativeDownloadImpl); String getLocation(long nativeDownloadImpl);
String getFileNameToReportToUser(long nativeDownloadImpl);
String getMimeTypeImpl(long nativeDownloadImpl); String getMimeTypeImpl(long nativeDownloadImpl);
int getError(long nativeDownloadImpl); int getError(long nativeDownloadImpl);
} }
......
...@@ -18,4 +18,5 @@ interface IDownload { ...@@ -18,4 +18,5 @@ interface IDownload {
int getError() = 7; int getError() = 7;
String getMimeType() = 8; String getMimeType() = 8;
void disableNotification() = 9; void disableNotification() = 9;
String getFileNameToReportToUser() = 10;
} }
...@@ -79,6 +79,10 @@ class Download { ...@@ -79,6 +79,10 @@ class Download {
// available until the download completes successfully. // available until the download completes successfully.
virtual base::FilePath GetLocation() = 0; virtual base::FilePath GetLocation() = 0;
// Returns the file name for the download that should be displayed to the
// user.
virtual base::FilePath GetFileNameToReportToUser() = 0;
// Returns the effective MIME type of downloaded content. // Returns the effective MIME type of downloaded content.
virtual std::string GetMimeType() = 0; virtual std::string GetMimeType() = 0;
......
...@@ -128,6 +128,24 @@ public class Download extends IClientDownload.Stub { ...@@ -128,6 +128,24 @@ public class Download extends IClientDownload.Stub {
} }
} }
/**
* Returns the file name for the download that should be displayed to the user.
*
* @since 86
*/
@NonNull
public File getFileNameToReportToUser() {
ThreadCheck.ensureOnUiThread();
if (WebLayer.getSupportedMajorVersionInternal() < 86) {
throw new UnsupportedOperationException();
}
try {
return new File(mDownloadImpl.getFileNameToReportToUser());
} catch (RemoteException e) {
throw new APICallException(e);
}
}
/** /**
* Returns the effective MIME type of downloaded content. * Returns the effective MIME type of downloaded content.
*/ */
......
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