Commit 9593ee49 authored by Tanya Gupta's avatar Tanya Gupta Committed by Commit Bot

[SharingHub] Changed the way firstPartyOptions are created.

They will not be created on demand. This simplifies the logic to decide
which options are relevant given the current state of the Chrome app.

Change-Id: I9b3ebf3030cfaa48088d7c1d0082d659b47f28cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2485838
Commit-Queue: Tanya Gupta <tgupta@chromium.org>
Reviewed-by: default avatarKyle Milka <kmilka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821914}
parent fae7adc6
...@@ -47,10 +47,6 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio ...@@ -47,10 +47,6 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio
private final BottomSheetController mBottomSheetController; private final BottomSheetController mBottomSheetController;
private final Supplier<Tab> mTabProvider; private final Supplier<Tab> mTabProvider;
private final ShareSheetPropertyModelBuilder mPropertyModelBuilder; private final ShareSheetPropertyModelBuilder mPropertyModelBuilder;
private final Callback<Tab> mPrintTabCallback;
private final SettingsLauncher mSettingsLauncher;
private final boolean mIsSyncEnabled;
private long mShareStartTime;
private boolean mExcludeFirstParty; private boolean mExcludeFirstParty;
private boolean mIsMultiWindow; private boolean mIsMultiWindow;
private Set<Integer> mContentTypes; private Set<Integer> mContentTypes;
...@@ -68,7 +64,7 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio ...@@ -68,7 +64,7 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio
* *
* @param controller The {@link BottomSheetController} for the current activity. * @param controller The {@link BottomSheetController} for the current activity.
* @param lifecycleDispatcher Dispatcher for activity lifecycle events, e.g. configuration * @param lifecycleDispatcher Dispatcher for activity lifecycle events, e.g. configuration
* changes. * changes.
* @param tabProvider Supplier for the current activity tab. * @param tabProvider Supplier for the current activity tab.
* @param modelBuilder The {@link ShareSheetPropertyModelBuilder} for the share sheet. * @param modelBuilder The {@link ShareSheetPropertyModelBuilder} for the share sheet.
*/ */
...@@ -82,9 +78,6 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio ...@@ -82,9 +78,6 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio
mLifecycleDispatcher.register(this); mLifecycleDispatcher.register(this);
mTabProvider = tabProvider; mTabProvider = tabProvider;
mPropertyModelBuilder = modelBuilder; mPropertyModelBuilder = modelBuilder;
mPrintTabCallback = printTab;
mSettingsLauncher = settingsLauncher;
mIsSyncEnabled = isSyncEnabled;
mBottomSheetObserver = new EmptyBottomSheetObserver() { mBottomSheetObserver = new EmptyBottomSheetObserver() {
@Override @Override
public void onSheetContentChanged(BottomSheetContent bottomSheet) { public void onSheetContentChanged(BottomSheetContent bottomSheet) {
...@@ -100,6 +93,11 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio ...@@ -100,6 +93,11 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio
}; };
mBottomSheetController.addObserver(mBottomSheetObserver); mBottomSheetController.addObserver(mBottomSheetObserver);
mIconBridge = iconBridge; mIconBridge = iconBridge;
mChromeProvidedSharingOptionsProvider =
new ChromeProvidedSharingOptionsProvider(tabProvider, controller, mBottomSheet);
mChromeProvidedSharingOptionsProvider.setFeatureSpecificParams(
printTab, settingsLauncher, isSyncEnabled);
} }
protected void destroy() { protected void destroy() {
...@@ -117,6 +115,7 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio ...@@ -117,6 +115,7 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio
mLifecycleDispatcher.unregister(this); mLifecycleDispatcher.unregister(this);
mLifecycleDispatcher = null; mLifecycleDispatcher = null;
} }
mChromeProvidedSharingOptionsProvider = null;
} }
// TODO(crbug/1022172): Should be package-protected once modularization is complete. // TODO(crbug/1022172): Should be package-protected once modularization is complete.
...@@ -135,12 +134,11 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio ...@@ -135,12 +134,11 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio
mBottomSheet = new ShareSheetBottomSheetContent(mActivity, mIconBridge, this, params); mBottomSheet = new ShareSheetBottomSheetContent(mActivity, mIconBridge, this, params);
mShareStartTime = shareStartTime;
mContentTypes = ShareSheetPropertyModelBuilder.getContentTypes(params, chromeShareExtras); mContentTypes = ShareSheetPropertyModelBuilder.getContentTypes(params, chromeShareExtras);
List<PropertyModel> firstPartyApps = List<PropertyModel> firstPartyApps = createFirstPartyPropertyModels(
createFirstPartyPropertyModels(mActivity, params, chromeShareExtras, mContentTypes); mActivity, params, chromeShareExtras, mContentTypes, shareStartTime);
List<PropertyModel> thirdPartyApps = createThirdPartyPropertyModels( List<PropertyModel> thirdPartyApps = createThirdPartyPropertyModels(
mActivity, params, mContentTypes, chromeShareExtras.saveLastUsed()); mActivity, params, mContentTypes, chromeShareExtras.saveLastUsed(), shareStartTime);
mBottomSheet.createRecyclerViews( mBottomSheet.createRecyclerViews(
firstPartyApps, thirdPartyApps, mContentTypes, params.getFileContentType()); firstPartyApps, thirdPartyApps, mContentTypes, params.getFileContentType());
...@@ -162,25 +160,24 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio ...@@ -162,25 +160,24 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio
} }
List<PropertyModel> createFirstPartyPropertyModels(Activity activity, ShareParams shareParams, List<PropertyModel> createFirstPartyPropertyModels(Activity activity, ShareParams shareParams,
ChromeShareExtras chromeShareExtras, Set<Integer> contentTypes) { ChromeShareExtras chromeShareExtras, Set<Integer> contentTypes, long shareStartTime) {
if (mExcludeFirstParty) { if (mExcludeFirstParty) {
return new ArrayList<>(); return new ArrayList<>();
} }
mChromeProvidedSharingOptionsProvider = new ChromeProvidedSharingOptionsProvider(activity,
mTabProvider, mBottomSheetController, mBottomSheet, shareParams, chromeShareExtras,
mPrintTabCallback, mSettingsLauncher, mIsSyncEnabled, mShareStartTime, this);
mIsMultiWindow = ApiCompatibilityUtils.isInMultiWindowMode(activity); mIsMultiWindow = ApiCompatibilityUtils.isInMultiWindowMode(activity);
return mChromeProvidedSharingOptionsProvider.getPropertyModels( mChromeProvidedSharingOptionsProvider.setShareRelatedParams(
contentTypes, mIsMultiWindow); shareParams, chromeShareExtras, shareStartTime, this, contentTypes);
mChromeProvidedSharingOptionsProvider.setIsMultiWindow(mIsMultiWindow);
return mChromeProvidedSharingOptionsProvider.calculatePropertyModels();
} }
@VisibleForTesting @VisibleForTesting
List<PropertyModel> createThirdPartyPropertyModels(Activity activity, ShareParams params, List<PropertyModel> createThirdPartyPropertyModels(Activity activity, ShareParams params,
Set<Integer> contentTypes, boolean saveLastUsed) { Set<Integer> contentTypes, boolean saveLastUsed, long shareStartTime) {
if (params == null) return null; if (params == null) return null;
List<PropertyModel> models = mPropertyModelBuilder.selectThirdPartyApps(mBottomSheet, List<PropertyModel> models = mPropertyModelBuilder.selectThirdPartyApps(mBottomSheet,
contentTypes, params, saveLastUsed, params.getWindow(), mShareStartTime); contentTypes, params, saveLastUsed, params.getWindow(), shareStartTime);
// More... // More...
PropertyModel morePropertyModel = ShareSheetPropertyModelBuilder.createPropertyModel( PropertyModel morePropertyModel = ShareSheetPropertyModelBuilder.createPropertyModel(
AppCompatResources.getDrawable(activity, R.drawable.sharing_more), AppCompatResources.getDrawable(activity, R.drawable.sharing_more),
...@@ -218,16 +215,22 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio ...@@ -218,16 +215,22 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio
return; return;
} }
boolean isMultiWindow = ApiCompatibilityUtils.isInMultiWindowMode(mActivity); boolean isMultiWindow = ApiCompatibilityUtils.isInMultiWindowMode(mActivity);
// mContentTypes is null if Chrome features should not be shown. if (mIsMultiWindow == isMultiWindow) {
if (mIsMultiWindow == isMultiWindow || mContentTypes == null) {
return; return;
} }
mIsMultiWindow = isMultiWindow; mIsMultiWindow = isMultiWindow;
mBottomSheet.createFirstPartyRecyclerViews( mChromeProvidedSharingOptionsProvider.setIsMultiWindow(mIsMultiWindow);
mChromeProvidedSharingOptionsProvider.getPropertyModels( List<PropertyModel> firstPartyOptions =
mContentTypes, mIsMultiWindow)); mChromeProvidedSharingOptionsProvider.calculatePropertyModels();
mBottomSheetController.requestShowContent(mBottomSheet, /*animate=*/false);
// firstPartyOptions is empty if Chrome features should not be shown.
if (firstPartyOptions == null || firstPartyOptions.isEmpty()) {
return;
}
mBottomSheet.createFirstPartyRecyclerViews(firstPartyOptions);
mBottomSheetController.requestShowContent(mBottomSheet, /* animate= */ false);
} }
// View.OnLayoutChangeListener // View.OnLayoutChangeListener
...@@ -242,5 +245,4 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio ...@@ -242,5 +245,4 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio
mBottomSheet.getThirdPartyView().invalidate(); mBottomSheet.getThirdPartyView().invalidate();
mBottomSheet.getThirdPartyView().requestLayout(); mBottomSheet.getThirdPartyView().requestLayout();
} }
} }
...@@ -22,6 +22,7 @@ import org.junit.Test; ...@@ -22,6 +22,7 @@ import org.junit.Test;
import org.junit.rules.TestRule; import org.junit.rules.TestRule;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.mockito.Mock; import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations; import org.mockito.MockitoAnnotations;
import org.chromium.base.test.util.CommandLineFlags; import org.chromium.base.test.util.CommandLineFlags;
...@@ -33,6 +34,8 @@ import org.chromium.chrome.test.ChromeJUnit4ClassRunner; ...@@ -33,6 +34,8 @@ import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.util.browser.Features; import org.chromium.chrome.test.util.browser.Features;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController; import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
import org.chromium.components.browser_ui.share.ShareParams; import org.chromium.components.browser_ui.share.ShareParams;
import org.chromium.ui.base.ImmutableWeakReference;
import org.chromium.ui.base.WindowAndroid;
import org.chromium.ui.modelutil.PropertyModel; import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.test.util.DummyUiActivity; import org.chromium.ui.test.util.DummyUiActivity;
...@@ -68,6 +71,9 @@ public final class ShareSheetCoordinatorTest { ...@@ -68,6 +71,9 @@ public final class ShareSheetCoordinatorTest {
@Mock @Mock
private ShareParams mParams; private ShareParams mParams;
@Mock
private WindowAndroid mWindow;
private Activity mActivity; private Activity mActivity;
private ShareSheetCoordinator mShareSheetCoordinator; private ShareSheetCoordinator mShareSheetCoordinator;
...@@ -95,6 +101,9 @@ public final class ShareSheetCoordinatorTest { ...@@ -95,6 +101,9 @@ public final class ShareSheetCoordinatorTest {
mShareSheetCoordinator = new ShareSheetCoordinator(mController, mLifecycleDispatcher, null, mShareSheetCoordinator = new ShareSheetCoordinator(mController, mLifecycleDispatcher, null,
mPropertyModelBuilder, null, null, null, false); mPropertyModelBuilder, null, null, null, false);
Mockito.when(mParams.getWindow()).thenReturn(mWindow);
Mockito.when(mWindow.getActivity()).thenReturn(new ImmutableWeakReference<>(mActivity));
} }
@Test @Test
...@@ -104,7 +113,7 @@ public final class ShareSheetCoordinatorTest { ...@@ -104,7 +113,7 @@ public final class ShareSheetCoordinatorTest {
List<PropertyModel> propertyModels = mShareSheetCoordinator.createFirstPartyPropertyModels( List<PropertyModel> propertyModels = mShareSheetCoordinator.createFirstPartyPropertyModels(
mActivity, mParams, /*chromeShareExtras=*/null, mActivity, mParams, /*chromeShareExtras=*/null,
ShareSheetPropertyModelBuilder.ALL_CONTENT_TYPES); ShareSheetPropertyModelBuilder.ALL_CONTENT_TYPES, /*shareStartTime=*/0);
assertEquals("Property model list should be empty.", 0, propertyModels.size()); assertEquals("Property model list should be empty.", 0, propertyModels.size());
} }
...@@ -113,7 +122,7 @@ public final class ShareSheetCoordinatorTest { ...@@ -113,7 +122,7 @@ public final class ShareSheetCoordinatorTest {
public void testCreateThirdPartyPropertyModels() { public void testCreateThirdPartyPropertyModels() {
List<PropertyModel> propertyModels = mShareSheetCoordinator.createThirdPartyPropertyModels( List<PropertyModel> propertyModels = mShareSheetCoordinator.createThirdPartyPropertyModels(
mActivity, mParams, ShareSheetPropertyModelBuilder.ALL_CONTENT_TYPES, mActivity, mParams, ShareSheetPropertyModelBuilder.ALL_CONTENT_TYPES,
/*saveLastUsed=*/false); /*saveLastUsed=*/false, /*shareStartTime=*/0);
assertEquals("Incorrect number of property models.", 3, propertyModels.size()); assertEquals("Incorrect number of property models.", 3, propertyModels.size());
assertEquals("First property model isn't testModel1.", "testModel1", assertEquals("First property model isn't testModel1.", "testModel1",
......
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