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); |