Commit be709442 authored by Rayan Kanso's avatar Rayan Kanso Committed by Commit Bot

Revert "[SharingHub] Changed the way firstPartyOptions are created."

This reverts commit 9593ee49.

Reason for revert: Breaking tests on Android.
Looks like Mockito is failing to mock 'WindowAndroid'
Sample build: https://ci.chromium.org/p/chromium/builders/ci/Lollipop%20Phone%20Tester/27576?

Bug: 1143640

Original change's description:
> [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: Kyle Milka <kmilka@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#821914}

TBR=kmilka@chromium.org,tgupta@chromium.org,sophey@chromium.org

Change-Id: I904fb819d2fd37899faefb6c71140cd9c54ad419
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2506576Reviewed-by: default avatarRayan Kanso <rayankans@chromium.org>
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822108}
parent 50cb1a54
...@@ -47,6 +47,10 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio ...@@ -47,6 +47,10 @@ 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;
...@@ -64,7 +68,7 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio ...@@ -64,7 +68,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.
*/ */
...@@ -78,6 +82,9 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio ...@@ -78,6 +82,9 @@ 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) {
...@@ -93,11 +100,6 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio ...@@ -93,11 +100,6 @@ 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() {
...@@ -115,7 +117,6 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio ...@@ -115,7 +117,6 @@ 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.
...@@ -134,11 +135,12 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio ...@@ -134,11 +135,12 @@ 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 = createFirstPartyPropertyModels( List<PropertyModel> firstPartyApps =
mActivity, params, chromeShareExtras, mContentTypes, shareStartTime); createFirstPartyPropertyModels(mActivity, params, chromeShareExtras, mContentTypes);
List<PropertyModel> thirdPartyApps = createThirdPartyPropertyModels( List<PropertyModel> thirdPartyApps = createThirdPartyPropertyModels(
mActivity, params, mContentTypes, chromeShareExtras.saveLastUsed(), shareStartTime); mActivity, params, mContentTypes, chromeShareExtras.saveLastUsed());
mBottomSheet.createRecyclerViews( mBottomSheet.createRecyclerViews(
firstPartyApps, thirdPartyApps, mContentTypes, params.getFileContentType()); firstPartyApps, thirdPartyApps, mContentTypes, params.getFileContentType());
...@@ -160,24 +162,25 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio ...@@ -160,24 +162,25 @@ 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, long shareStartTime) { ChromeShareExtras chromeShareExtras, Set<Integer> contentTypes) {
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);
mChromeProvidedSharingOptionsProvider.setShareRelatedParams( return mChromeProvidedSharingOptionsProvider.getPropertyModels(
shareParams, chromeShareExtras, shareStartTime, this, contentTypes); contentTypes, mIsMultiWindow);
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, long shareStartTime) { Set<Integer> contentTypes, boolean saveLastUsed) {
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(), shareStartTime); contentTypes, params, saveLastUsed, params.getWindow(), mShareStartTime);
// More... // More...
PropertyModel morePropertyModel = ShareSheetPropertyModelBuilder.createPropertyModel( PropertyModel morePropertyModel = ShareSheetPropertyModelBuilder.createPropertyModel(
AppCompatResources.getDrawable(activity, R.drawable.sharing_more), AppCompatResources.getDrawable(activity, R.drawable.sharing_more),
...@@ -215,22 +218,16 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio ...@@ -215,22 +218,16 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio
return; return;
} }
boolean isMultiWindow = ApiCompatibilityUtils.isInMultiWindowMode(mActivity); boolean isMultiWindow = ApiCompatibilityUtils.isInMultiWindowMode(mActivity);
if (mIsMultiWindow == isMultiWindow) { // mContentTypes is null if Chrome features should not be shown.
if (mIsMultiWindow == isMultiWindow || mContentTypes == null) {
return; return;
} }
mIsMultiWindow = isMultiWindow; mIsMultiWindow = isMultiWindow;
mChromeProvidedSharingOptionsProvider.setIsMultiWindow(mIsMultiWindow); mBottomSheet.createFirstPartyRecyclerViews(
List<PropertyModel> firstPartyOptions = mChromeProvidedSharingOptionsProvider.getPropertyModels(
mChromeProvidedSharingOptionsProvider.calculatePropertyModels(); mContentTypes, mIsMultiWindow));
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
...@@ -245,4 +242,5 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio ...@@ -245,4 +242,5 @@ public class ShareSheetCoordinator implements ActivityStateObserver, ChromeOptio
mBottomSheet.getThirdPartyView().invalidate(); mBottomSheet.getThirdPartyView().invalidate();
mBottomSheet.getThirdPartyView().requestLayout(); mBottomSheet.getThirdPartyView().requestLayout();
} }
} }
...@@ -22,7 +22,6 @@ import org.junit.Test; ...@@ -22,7 +22,6 @@ 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;
...@@ -34,8 +33,6 @@ import org.chromium.chrome.test.ChromeJUnit4ClassRunner; ...@@ -34,8 +33,6 @@ 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;
...@@ -71,9 +68,6 @@ public final class ShareSheetCoordinatorTest { ...@@ -71,9 +68,6 @@ 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;
...@@ -101,9 +95,6 @@ public final class ShareSheetCoordinatorTest { ...@@ -101,9 +95,6 @@ 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
...@@ -113,7 +104,7 @@ public final class ShareSheetCoordinatorTest { ...@@ -113,7 +104,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, /*shareStartTime=*/0); ShareSheetPropertyModelBuilder.ALL_CONTENT_TYPES);
assertEquals("Property model list should be empty.", 0, propertyModels.size()); assertEquals("Property model list should be empty.", 0, propertyModels.size());
} }
...@@ -122,7 +113,7 @@ public final class ShareSheetCoordinatorTest { ...@@ -122,7 +113,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, /*shareStartTime=*/0); /*saveLastUsed=*/false);
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