Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1243)

Unified Diff: chrome/android/java/src/org/chromium/chrome/browser/share/ShareHelper.java

Issue 2917703004: [Android] Wrap all share parameters into the ShareParams class (Closed)
Patch Set: Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/android/java/src/org/chromium/chrome/browser/share/ShareHelper.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/share/ShareHelper.java b/chrome/android/java/src/org/chromium/chrome/browser/share/ShareHelper.java
index 7566d2e5ba354c43d3b9b0d87dcb67daa09731fc..e892f4dedfbc7b287bce9e29f08b9d474768af2d 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/share/ShareHelper.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/share/ShareHelper.java
@@ -285,31 +285,15 @@ public class ShareHelper {
* Cleaning up doesn't happen automatically, and so an app should call clearSharedScreenshots()
* explicitly when needed.
*
- * @param shareDirectly Whether it should share directly with the activity that was most
- * recently used to share.
- * @param saveLastUsed Whether to save the chosen activity for future direct sharing.
- * @param activity Activity that is used to access package manager.
- * @param title Title of the page to be shared.
- * @param text Text to be shared. If both |text| and |url| are supplied, they are concatenated
- * with a space.
- * @param url URL of the page to be shared.
- * @param offlineUri URI to the offline MHTML file to be shared.
- * @param screenshotUri Uri of the screenshot of the page to be shared.
- * @param callback Optional callback to be called when user makes a choice. Will not be called
- * if receiving a response when the user makes a choice is not supported (on
- * older Android versions).
+ * @param params The container holding the share parameters.
*/
- public static void share(boolean shareDirectly, boolean saveLastUsed, Activity activity,
- String title, String text, String url, @Nullable Uri offlineUri, Uri screenshotUri,
- @Nullable TargetChosenCallback callback) {
- if (shareDirectly) {
- shareWithLastUsed(activity, title, text, url, offlineUri, screenshotUri);
+ public static void share(ShareParams params) {
+ if (params.isShareDirectly()) {
+ shareWithLastUsed(params);
} else if (TargetChosenReceiver.isSupported()) {
- makeIntentAndShare(saveLastUsed, activity, title, text, url, offlineUri, screenshotUri,
- null, callback);
+ makeIntentAndShare(params, null);
} else {
- showShareDialog(
- saveLastUsed, activity, title, text, url, offlineUri, screenshotUri, callback);
+ showShareDialog(params);
}
}
@@ -440,22 +424,16 @@ public class ShareHelper {
/**
* Creates and shows a share intent picker dialog.
*
- * @param saveLastUsed Whether to save the chosen activity for future direct sharing.
- * @param activity Activity that is used to access package manager.
- * @param title Title of the page to be shared.
- * @param text Text to be shared. If both |text| and |url| are supplied, they are concatenated
- * with a space.
- * @param url URL of the page to be shared.
- * @oaram offlineUri URI of the offline page to be shared.
- * @param screenshotUri Uri of the screenshot of the page to be shared.
- * @param callback Optional callback to be called when user makes a choice. Will not be called
- * if receiving a response when the user makes a choice is not supported (on
- * older Android versions).
+ * @param params The container holding the share parameters.
*/
- private static void showShareDialog(final boolean saveLastUsed, final Activity activity,
- final String title, final String text, final String url, final Uri offlineUri,
- final Uri screenshotUri, @Nullable final TargetChosenCallback callback) {
- Intent intent = getShareIntent(activity, title, text, url, null, null);
+ private static void showShareDialog(final ShareParams params) {
+ Activity activity = params.getActivity();
+ final TargetChosenCallback callback = params.getCallback();
Yusuf 2017/06/01 20:41:45 is it the callback that is creating the problem he
ltian 2017/06/01 23:27:11 No, I think the callback problem has been solved s
+ ShareParams simpleShareParams =
Ted C 2017/06/01 20:39:03 does this need to be any different from the defaul
ltian 2017/06/01 23:27:07 Yes, after digging on these I find both of them on
+ new ShareParams.Builder(params.getActivity(), params.getTitle(), params.getUrl())
+ .setText(params.getText())
+ .build();
+ Intent intent = getShareIntent(simpleShareParams);
PackageManager manager = activity.getPackageManager();
List<ResolveInfo> resolveInfoList = manager.queryIntentActivities(intent, 0);
assert resolveInfoList.size() > 0;
@@ -480,13 +458,13 @@ public class ShareHelper {
ActivityInfo ai = info.activityInfo;
ComponentName component =
new ComponentName(ai.applicationInfo.packageName, ai.name);
+
if (callback != null && !callbackCalled[0]) {
callback.onTargetChosen(component);
callbackCalled[0] = true;
}
- if (saveLastUsed) setLastShareComponentName(component);
- makeIntentAndShare(false, activity, title, text, url, offlineUri, screenshotUri,
- component, null);
+ if (params.isSaveLastUsed()) setLastShareComponentName(component);
+ makeIntentAndShare(params, component);
dialog.dismiss();
}
});
@@ -511,41 +489,29 @@ public class ShareHelper {
/**
* Starts a share intent with the activity that was most recently used to share.
* If there is no most recently used activity, it does nothing.
- * @param activity Activity that is used to start the share intent.
- * @param title Title of the page to be shared.
- * @param text Text to be shared. If both |text| and |url| are supplied, they are concatenated
- * with a space.
- * @param url URL of the page to be shared.
- * @oaram offlineUri URI of the offline page to be shared.
- * @param screenshotUri Uri of the screenshot of the page to be shared.
+ * @param params The container holding the share parameters.
*/
- private static void shareWithLastUsed(Activity activity, String title, String text, String url,
- Uri offlineUri, Uri screenshotUri) {
+ private static void shareWithLastUsed(ShareParams params) {
Ted C 2017/06/01 20:39:03 I think this is only used in share, let's just rem
ltian 2017/06/01 23:27:09 Done.
ComponentName component = getLastShareComponentName();
if (component == null) return;
- makeIntentAndShare(
- false, activity, title, text, url, offlineUri, screenshotUri, component, null);
+ makeIntentAndShare(params, component);
}
- private static void shareIntent(boolean saveLastUsed, Activity activity, Intent sharingIntent,
- @Nullable TargetChosenCallback callback) {
+ private static void shareIntent(ShareParams params, Intent sharingIntent) {
Ted C 2017/06/01 20:39:03 Is this only used from within makeIntentAndShare?
ltian 2017/06/01 23:27:08 Done.
if (sharingIntent.getComponent() != null) {
// If a component was specified, there should not also be a callback.
- assert callback == null;
- fireIntent(activity, sharingIntent);
+ assert params.getCallback() == null;
+ fireIntent(params.getActivity(), sharingIntent);
} else {
assert TargetChosenReceiver.isSupported();
- TargetChosenReceiver.sendChooserIntent(saveLastUsed, activity, sharingIntent, callback);
+ TargetChosenReceiver.sendChooserIntent(params.isSaveLastUsed(), params.getActivity(),
+ sharingIntent, params.getCallback());
Yusuf 2017/06/01 20:41:45 why not also pass params here?
ltian 2017/06/01 23:27:07 The only reason is because in ShareImage the onPos
}
}
- private static void makeIntentAndShare(final boolean saveLastUsed, final Activity activity,
- final String title, final String text, final String url, final Uri offlineUri,
- final Uri screenshotUri, final ComponentName component,
- @Nullable final TargetChosenCallback callback) {
- Intent intent = getDirectShareIntentForComponent(
- activity, title, text, url, offlineUri, screenshotUri, component);
- shareIntent(saveLastUsed, activity, intent, callback);
+ private static void makeIntentAndShare(ShareParams params, ComponentName component) {
Ted C 2017/06/01 20:39:03 let's mark ComponentName with @Nullable
Yusuf 2017/06/01 20:41:45 why is componentName not a part of the shareParam?
ltian 2017/06/01 23:27:07 Done.
ltian 2017/06/01 23:27:11 One reason is in showShareDialog's onItemClick, it
Yusuf 2017/06/02 20:07:46 Acknowledged.
+ Intent intent = getDirectShareIntentForComponent(params, component);
+ shareIntent(params, intent);
}
/**
@@ -555,7 +521,8 @@ public class ShareHelper {
* @param item The menu item that is used for direct share
*/
public static void configureDirectShareMenuItem(Activity activity, MenuItem item) {
- Intent shareIntent = getShareIntent(activity, "", "", "", null, null);
+ ShareParams params = new ShareParams.Builder(activity, "", "").setText("").build();
+ Intent shareIntent = getShareIntent(params);
Pair<Drawable, CharSequence> directShare = getShareableIconAndName(activity, shareIntent);
Drawable directShareIcon = directShare.first;
CharSequence directShareTitle = directShare.second;
@@ -649,8 +616,9 @@ public class ShareHelper {
}
@VisibleForTesting
- public static Intent getShareIntent(Activity activity, String title, String text, String url,
- Uri offlineUri, Uri screenshotUri) {
+ public static Intent getShareIntent(ShareParams params) {
Ted C 2017/06/01 20:39:03 let's rename this getShareLinkIntent to differenti
ltian 2017/06/01 23:27:10 The problem I meet to combine these two is the Int
+ String url = params.getUrl();
+ String text = params.getText();
if (!TextUtils.isEmpty(url)) {
url = DomDistillerUrlUtils.getOriginalUrlFromDistillerUrl(url);
if (!TextUtils.isEmpty(text)) {
@@ -663,10 +631,11 @@ public class ShareHelper {
Intent intent = new Intent(Intent.ACTION_SEND);
intent.addFlags(ApiCompatibilityUtils.getActivityNewDocumentFlag());
- intent.putExtra(Intent.EXTRA_SUBJECT, title);
+ intent.putExtra(Intent.EXTRA_SUBJECT, params.getTitle());
intent.putExtra(Intent.EXTRA_TEXT, text);
- intent.putExtra(EXTRA_TASK_ID, activity.getTaskId());
+ intent.putExtra(EXTRA_TASK_ID, params.getActivity().getTaskId());
+ Uri screenshotUri = params.getScreenshotUri();
if (screenshotUri != null) {
Ted C 2017/06/01 20:39:03 tangential cleanup, but it seems like we should be
ltian 2017/06/01 23:27:07 Done.
intent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION);
}
@@ -677,11 +646,11 @@ public class ShareHelper {
intent.setClipData(ClipData.newRawUri("", screenshotUri));
intent.putExtra(EXTRA_SHARE_SCREENSHOT_AS_STREAM, screenshotUri);
}
- if (offlineUri == null) {
+ if (params.getOfflineUri() == null) {
intent.setType("text/plain");
} else {
intent.setType("multipart/related");
- intent.putExtra(Intent.EXTRA_STREAM, offlineUri);
+ intent.putExtra(Intent.EXTRA_STREAM, params.getOfflineUri());
}
return intent;
}
@@ -700,9 +669,9 @@ public class ShareHelper {
return intent;
}
- private static Intent getDirectShareIntentForComponent(Activity activity, String title,
- String text, String url, Uri offlineUri, Uri screenshotUri, ComponentName component) {
- Intent intent = getShareIntent(activity, title, text, url, offlineUri, screenshotUri);
+ private static Intent getDirectShareIntentForComponent(
Ted C 2017/06/01 20:39:03 tangential cleanup, this method is used for all sh
ltian 2017/06/01 23:27:13 Done.
+ ShareParams params, ComponentName component) {
+ Intent intent = getShareIntent(params);
intent.addFlags(Intent.FLAG_ACTIVITY_FORWARD_RESULT
| Intent.FLAG_ACTIVITY_PREVIOUS_IS_TOP);
intent.setComponent(component);

Powered by Google App Engine
This is Rietveld 408576698