Commit 9dd16811 authored by Tanya Gupta's avatar Tanya Gupta Committed by Commit Bot

[QrCode] Restructured code so that tabs are created in the dialog.

This prevents the app from crashing when the user navigates to the
setting page.

Change-Id: I4c48d8996694a681d27bf423e5212eaf82de29ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2046711
Commit-Queue: Tanya Gupta <tgupta@chromium.org>
Reviewed-by: default avatarGayane Petrosyan <gayane@chromium.org>
Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743178}
parent 4f601867
......@@ -437,9 +437,9 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
mManualFillingComponent.getKeyboardExtensionViewResizer());
// Should be called after TabModels are initialized.
ShareDelegate shareDelegate = new ShareDelegateImpl(
mRootUiCoordinator.getBottomSheetController(), getActivityTabProvider(),
new ShareDelegateImpl.ShareSheetDelegate(), getCurrentTabCreator());
ShareDelegate shareDelegate =
new ShareDelegateImpl(mRootUiCoordinator.getBottomSheetController(),
getActivityTabProvider(), new ShareDelegateImpl.ShareSheetDelegate());
mShareDelegateSupplier.set(shareDelegate);
// If onStart was called before postLayoutInflation (because inflation was done in a
......
......@@ -24,7 +24,7 @@ import org.chromium.chrome.browser.share.qrcode.QrCodeShareActivity;
import org.chromium.chrome.browser.tab.SadTab;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabImpl;
import org.chromium.chrome.browser.tabmodel.TabCreatorManager;
import org.chromium.chrome.browser.tabmodel.TabModel;
import org.chromium.chrome.browser.util.ChromeFileProvider;
import org.chromium.chrome.browser.util.UrlConstants;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController;
......@@ -45,7 +45,6 @@ public class ShareDelegateImpl implements ShareDelegate {
private final BottomSheetController mBottomSheetController;
private final ShareSheetDelegate mDelegate;
private final ActivityTabProvider mActivityTabProvider;
private final TabCreatorManager.TabCreator mTabCreator;
private long mShareStartTime;
private static boolean sScreenshotCaptureSkippedForTesting;
......@@ -58,11 +57,10 @@ public class ShareDelegateImpl implements ShareDelegate {
* @param tabCreator The TabCreator for the current selected {@link TabModel}.
*/
public ShareDelegateImpl(BottomSheetController controller, ActivityTabProvider tabProvider,
ShareSheetDelegate delegate, TabCreatorManager.TabCreator tabCreator) {
ShareSheetDelegate delegate) {
mBottomSheetController = controller;
mDelegate = delegate;
mActivityTabProvider = tabProvider;
mTabCreator = tabCreator;
}
// ShareDelegate implementation.
......@@ -71,8 +69,7 @@ public class ShareDelegateImpl implements ShareDelegate {
if (mShareStartTime == 0L) {
mShareStartTime = System.currentTimeMillis();
}
mDelegate.share(
params, mBottomSheetController, mActivityTabProvider, mTabCreator, mShareStartTime);
mDelegate.share(params, mBottomSheetController, mActivityTabProvider, mShareStartTime);
mShareStartTime = 0;
}
......@@ -261,13 +258,12 @@ public class ShareDelegateImpl implements ShareDelegate {
* Trigger the share action for the specified params.
*/
void share(ShareParams params, BottomSheetController controller,
ActivityTabProvider tabProvider, TabCreatorManager.TabCreator tabCreator,
long shareStartTime) {
ActivityTabProvider tabProvider, long shareStartTime) {
if (params.shareDirectly()) {
ShareHelper.shareDirectly(params);
} else if (ChromeFeatureList.isEnabled(ChromeFeatureList.CHROME_SHARING_HUB)) {
ShareSheetCoordinator coordinator =
new ShareSheetCoordinator(controller, tabProvider, tabCreator,
new ShareSheetCoordinator(controller, tabProvider,
new ShareSheetPropertyModelBuilder(controller,
ContextUtils.getApplicationContext().getPackageManager()));
// TODO(crbug/1009124): open custom share sheet.
......
......@@ -20,10 +20,7 @@ import org.chromium.chrome.browser.send_tab_to_self.SendTabToSelfShareActivity;
import org.chromium.chrome.browser.share.qrcode.QrCodeCoordinator;
import org.chromium.chrome.browser.share.screenshot.ScreenshotCoordinator;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabLaunchType;
import org.chromium.chrome.browser.tabmodel.TabCreatorManager;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController;
import org.chromium.content_public.browser.LoadUrlParams;
import org.chromium.content_public.browser.NavigationEntry;
import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.widget.Toast;
......@@ -36,20 +33,17 @@ import java.util.ArrayList;
public class ShareSheetCoordinator {
private final BottomSheetController mBottomSheetController;
private final ActivityTabProvider mActivityTabProvider;
private final TabCreatorManager.TabCreator mTabCreator;
private final ShareSheetPropertyModelBuilder mPropertyModelBuilder;
/**
* Constructs a new ShareSheetCoordinator.
* @param controller The BottomSheetController for the current activity.
* @param provider The ActivityTabProvider for the current visible tab.
* @param tabCreator The TabCreator for the current selected {@link TabModel}.
*/
public ShareSheetCoordinator(BottomSheetController controller, ActivityTabProvider provider,
TabCreatorManager.TabCreator tabCreator, ShareSheetPropertyModelBuilder modelBuilder) {
ShareSheetPropertyModelBuilder modelBuilder) {
mBottomSheetController = controller;
mActivityTabProvider = provider;
mTabCreator = tabCreator;
mPropertyModelBuilder = modelBuilder;
}
......@@ -86,8 +80,7 @@ public class ShareSheetCoordinator {
(currentActivity)
-> {
mBottomSheetController.hideContent(bottomSheet, true);
QrCodeCoordinator qrCodeCoordinator =
new QrCodeCoordinator(activity, this::createNewTab);
QrCodeCoordinator qrCodeCoordinator = new QrCodeCoordinator(activity);
qrCodeCoordinator.show();
},
/*isFirstParty=*/true);
......@@ -171,9 +164,4 @@ public class ShareSheetCoordinator {
return models;
}
private void createNewTab(String url) {
mTabCreator.createNewTab(
new LoadUrlParams(url), TabLaunchType.FROM_LINK, mActivityTabProvider.get());
}
}
......@@ -20,7 +20,6 @@ import org.chromium.chrome.browser.ActivityTabProvider;
import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.ChromeSwitches;
import org.chromium.chrome.browser.share.ShareDelegateImpl.ShareSheetDelegate;
import org.chromium.chrome.browser.tabmodel.TabCreatorManager;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController;
import org.chromium.chrome.test.ChromeActivityTestRule;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
......@@ -115,16 +114,14 @@ public class ShareDelegateImplIntegrationTest {
ShareSheetDelegate delegate = new ShareSheetDelegate() {
@Override
void share(ShareParams params, BottomSheetController controller,
ActivityTabProvider tabProvider, TabCreatorManager.TabCreator tabCreator,
long shareStartTime) {
ActivityTabProvider tabProvider, long shareStartTime) {
paramsRef.set(params);
helper.notifyCalled();
}
};
new ShareDelegateImpl(mActivityTestRule.getActivity().getBottomSheetController(),
mActivityTestRule.getActivity().getActivityTabProvider(), delegate,
mActivityTestRule.getActivity().getCurrentTabCreator())
mActivityTestRule.getActivity().getActivityTabProvider(), delegate)
.share(mActivityTestRule.getActivity().getActivityTab(), false);
});
helper.waitForCallback(0);
......
......@@ -78,7 +78,7 @@ public final class ShareSheetCoordinatorTest {
@Features.DisableFeatures({ChromeFeatureList.CHROME_SHARE_SCREENSHOT})
public void testCreateTopRowPropertyModelsScreenshotsDisabled() {
ShareSheetCoordinator coordinator =
new ShareSheetCoordinator(null, null, null, mPropertyModelBuilder);
new ShareSheetCoordinator(null, null, mPropertyModelBuilder);
Activity activity = mActivityTestRule.getActivity();
ShareSheetBottomSheetContent bottomSheet = new ShareSheetBottomSheetContent(activity);
......@@ -108,7 +108,7 @@ public final class ShareSheetCoordinatorTest {
@Features.EnableFeatures({ChromeFeatureList.CHROME_SHARE_SCREENSHOT})
public void testCreateTopRowPropertyModelsScreenshotsEnabled() {
ShareSheetCoordinator coordinator =
new ShareSheetCoordinator(null, null, null, mPropertyModelBuilder);
new ShareSheetCoordinator(null, null, mPropertyModelBuilder);
Activity activity = mActivityTestRule.getActivity();
ShareSheetBottomSheetContent bottomSheet = new ShareSheetBottomSheetContent(activity);
......@@ -141,7 +141,7 @@ public final class ShareSheetCoordinatorTest {
@MediumTest
public void testCreateBottomRowPropertyModels() {
ShareSheetCoordinator coordinator =
new ShareSheetCoordinator(null, null, null, mPropertyModelBuilder);
new ShareSheetCoordinator(null, null, mPropertyModelBuilder);
Activity activity = mActivityTestRule.getActivity();
ShareSheetBottomSheetContent bottomSheet = new ShareSheetBottomSheetContent(activity);
......
......@@ -2,11 +2,10 @@ include_rules = [
# TODO(crbug/1022172): Remove this dependency when ShareActivity is moved to
# chrome/browser/share.
"+chrome/android/java/src/org/chromium/chrome/browser/share",
"+chrome/android/java/src/org/chromium/chrome/browser/ActivityTabProvider.java",
"+chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java",
"+chrome/android/java/src/org/chromium/chrome/browser/IntentHandler.java",
"+chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java",
"+chrome/android/java/src/org/chromium/chrome/browser/document/ChromeLauncherActivity.java",
"+chrome/android/java/src/org/chromium/chrome/browser/modules/ModuleInstallUi.java",
"+chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java",
"+chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabCreatorManager.java",
"+content/public/android/java/src/org/chromium/content_public/browser/LoadUrlParams.java",
]
......@@ -7,12 +7,6 @@ package org.chromium.chrome.browser.share.qrcode;
import android.app.Activity;
import android.app.FragmentManager;
import org.chromium.chrome.browser.share.qrcode.scan_tab.QrCodeScanCoordinator;
import org.chromium.chrome.browser.share.qrcode.scan_tab.QrCodeScanMediator;
import org.chromium.chrome.browser.share.qrcode.share_tab.QrCodeShareCoordinator;
import java.util.ArrayList;
/**
* Creates and represents the QrCode main UI.
*/
......@@ -20,16 +14,8 @@ public class QrCodeCoordinator {
private final QrCodeDialog mDialog;
private final FragmentManager mFragmentManager;
public QrCodeCoordinator(Activity activity, QrCodeScanMediator.TabCreator tabCreator) {
QrCodeShareCoordinator shareCoordinator = new QrCodeShareCoordinator(activity);
QrCodeScanCoordinator scanCoordinator =
new QrCodeScanCoordinator(activity, this::dismiss, tabCreator);
ArrayList<QrCodeDialogTab> tabs = new ArrayList<QrCodeDialogTab>();
tabs.add(shareCoordinator);
tabs.add(scanCoordinator);
mDialog = new QrCodeDialog(tabs);
public QrCodeCoordinator(Activity activity) {
mDialog = new QrCodeDialog();
mFragmentManager = activity.getFragmentManager();
}
......
......@@ -4,9 +4,9 @@
package org.chromium.chrome.browser.share.qrcode;
import android.annotation.SuppressLint;
import android.app.Dialog;
import android.app.DialogFragment;
import android.content.Context;
import android.os.Bundle;
import android.support.design.widget.TabLayout;
import android.support.v4.view.ViewPager;
......@@ -14,6 +14,8 @@ import android.support.v7.app.AlertDialog;
import android.view.View;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.share.qrcode.scan_tab.QrCodeScanCoordinator;
import org.chromium.chrome.browser.share.qrcode.share_tab.QrCodeShareCoordinator;
import org.chromium.ui.widget.ChromeImageButton;
import java.util.ArrayList;
......@@ -23,25 +25,25 @@ import java.util.ArrayList;
*/
public class QrCodeDialog extends DialogFragment {
private ArrayList<QrCodeDialogTab> mTabs;
private Context mContext;
/**
* The QrCodeDialog constructor.
* TODO(tgupta): This causes an NPE when the user moves away from Chrome
* and comes back. Fix this issue.
*/
public QrCodeDialog() {}
public QrCodeDialog() {
mTabs = new ArrayList<QrCodeDialogTab>();
}
/**
* The QrCodeDialog constructor.
* @param tabs The array of tabs for the tab layout.
*/
/**
* TODO(gayane): Resolve lint warning. Per warning, tabs should be passed through Bundle, but I
* don't want to make all the classes parcelable.
*/
@SuppressLint("ValidFragment")
public QrCodeDialog(ArrayList<QrCodeDialogTab> tabs) {
mTabs = tabs;
@Override
public void onAttach(Context context) {
super.onAttach(context);
mContext = context;
QrCodeShareCoordinator shareCoordinator = new QrCodeShareCoordinator(context);
QrCodeScanCoordinator scanCoordinator = new QrCodeScanCoordinator(context, this::dismiss);
mTabs.add(shareCoordinator);
mTabs.add(scanCoordinator);
}
@Override
......@@ -67,6 +69,16 @@ public class QrCodeDialog extends DialogFragment {
}
}
@Override
public void onDestroy() {
super.onDestroy();
mContext = null;
for (QrCodeDialogTab tab : mTabs) {
tab.onDestroy();
}
mTabs.clear();
}
private View getDialogView() {
View dialogView = (View) getActivity().getLayoutInflater().inflate(
org.chromium.chrome.browser.share.qrcode.R.layout.qrcode_dialog, null);
......
......@@ -15,4 +15,6 @@ public interface QrCodeDialogTab {
public void onResume();
public void onPause();
public void onDestroy();
}
\ No newline at end of file
......@@ -4,37 +4,22 @@
package org.chromium.chrome.browser.share.qrcode;
import org.chromium.chrome.browser.ActivityTabProvider;
import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.share.ShareActivity;
import org.chromium.chrome.browser.tab.TabLaunchType;
import org.chromium.chrome.browser.tabmodel.TabCreatorManager;
import org.chromium.content_public.browser.LoadUrlParams;
/**
* A simple activity that shows sharing QR code option in share menu.
*/
public class QrCodeShareActivity extends ShareActivity {
private ActivityTabProvider mActivityTabProvider;
private TabCreatorManager.TabCreator mTabCreator;
@Override
protected void handleShareAction(ChromeActivity triggeringActivity) {
mActivityTabProvider = triggeringActivity.getActivityTabProvider();
mTabCreator = triggeringActivity.getCurrentTabCreator();
QrCodeCoordinator qrCodeCoordinator =
new QrCodeCoordinator(triggeringActivity, this::createNewTab);
QrCodeCoordinator qrCodeCoordinator = new QrCodeCoordinator(triggeringActivity);
qrCodeCoordinator.show();
}
public static boolean featureIsAvailable() {
return ChromeFeatureList.isEnabled(ChromeFeatureList.SHARING_QR_CODE_ANDROID);
}
private void createNewTab(String url) {
mTabCreator.createNewTab(
new LoadUrlParams(url), TabLaunchType.FROM_LINK, mActivityTabProvider.get());
}
}
\ No newline at end of file
......@@ -23,10 +23,9 @@ public class QrCodeScanCoordinator implements QrCodeDialogTab {
*
* @param context The context to use for user permissions.
*/
public QrCodeScanCoordinator(Context context, QrCodeScanMediator.NavigationObserver observer,
QrCodeScanMediator.TabCreator tabCreator) {
public QrCodeScanCoordinator(Context context, QrCodeScanMediator.NavigationObserver observer) {
PropertyModel scanViewModel = new PropertyModel(QrCodeScanViewProperties.ALL_KEYS);
mMediator = new QrCodeScanMediator(context, scanViewModel, observer, tabCreator);
mMediator = new QrCodeScanMediator(context, scanViewModel, observer);
mScanView = new QrCodeScanView(
context, mMediator::onPreviewFrame, mMediator::promptForCameraPermission);
......@@ -48,4 +47,9 @@ public class QrCodeScanCoordinator implements QrCodeDialogTab {
public void onPause() {
mMediator.setIsOnForeground(false);
}
@Override
public void onDestroy() {
mScanView.stopCamera();
}
}
......@@ -7,12 +7,15 @@ package org.chromium.chrome.browser.share.qrcode.scan_tab;
import android.Manifest.permission;
import android.app.Activity;
import android.content.Context;
import android.content.Intent;
import android.content.pm.PackageManager;
import android.graphics.ImageFormat;
import android.hardware.Camera;
import android.net.Uri;
import android.os.Handler;
import android.os.Looper;
import android.os.Process;
import android.provider.Browser;
import android.util.SparseArray;
import android.webkit.URLUtil;
......@@ -20,6 +23,9 @@ import com.google.android.gms.vision.Frame;
import com.google.android.gms.vision.barcode.Barcode;
import com.google.android.gms.vision.barcode.BarcodeDetector;
import org.chromium.chrome.browser.IntentHandler;
import org.chromium.chrome.browser.ShortcutHelper;
import org.chromium.chrome.browser.document.ChromeLauncherActivity;
import org.chromium.ui.base.ActivityAndroidPermissionDelegate;
import org.chromium.ui.base.AndroidPermissionDelegate;
import org.chromium.ui.base.PermissionCallback;
......@@ -43,7 +49,6 @@ public class QrCodeScanMediator implements Camera.PreviewCallback {
private final PropertyModel mPropertyModel;
private final BarcodeDetector mDetector;
private final NavigationObserver mNavigationObserver;
private final TabCreator mTabCreator;
private final Handler mMainThreadHandler;
private final AndroidPermissionDelegate mPermissionDelegate;
......@@ -54,8 +59,7 @@ public class QrCodeScanMediator implements Camera.PreviewCallback {
* @param propertyModel The property modelto use to communicate with views.
* @param observer The observer for navigation event.
*/
QrCodeScanMediator(Context context, PropertyModel propertyModel, NavigationObserver observer,
TabCreator tabCreator) {
QrCodeScanMediator(Context context, PropertyModel propertyModel, NavigationObserver observer) {
mContext = context;
mPropertyModel = propertyModel;
mPermissionDelegate = new ActivityAndroidPermissionDelegate(
......@@ -63,7 +67,6 @@ public class QrCodeScanMediator implements Camera.PreviewCallback {
updatePermissionSettings();
mDetector = new BarcodeDetector.Builder(context).build();
mNavigationObserver = observer;
mTabCreator = tabCreator;
mMainThreadHandler = new Handler(Looper.getMainLooper());
}
......@@ -160,8 +163,21 @@ public class QrCodeScanMediator implements Camera.PreviewCallback {
/** Tab creation should happen on the main thread. */
mMainThreadHandler.post(() -> {
mTabCreator.createNewTab(firstCode.rawValue);
openUrl(firstCode.rawValue);
mNavigationObserver.onNavigation();
});
}
private void openUrl(String url) {
Intent intent =
new Intent()
.setAction(Intent.ACTION_VIEW)
.setData(Uri.parse(url))
.setClass(mContext, ChromeLauncherActivity.class)
.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK | Intent.FLAG_ACTIVITY_NEW_DOCUMENT)
.putExtra(Browser.EXTRA_APPLICATION_ID, mContext.getPackageName())
.putExtra(ShortcutHelper.REUSE_URL_MATCHING_TAB_ELSE_NEW_TAB, true);
IntentHandler.addTrustedIntentExtras(intent);
mContext.startActivity(intent);
}
}
......@@ -203,10 +203,7 @@ class QrCodeScanView {
/** Creates and sets the camera preview. */
private void setCameraPreview() {
mView.removeAllViews();
if (mCameraPreview != null) {
mCameraPreview.stopCamera();
mCameraPreview = null;
}
stopCamera();
if (mHasCameraPermission) {
mCameraPreview =
......@@ -250,6 +247,15 @@ class QrCodeScanView {
mView.addView(mCameraErrorView);
}
/**
* Returns an Intent to show the App Info page for the current app.
*/
private Intent getAppInfoIntent(String packageName) {
Intent intent = new Intent(Settings.ACTION_APPLICATION_DETAILS_SETTINGS);
intent.setData(new Uri.Builder().scheme("package").opaquePart(packageName).build());
return intent;
}
/**
* Displays the open settings dialog.
*/
......@@ -257,12 +263,14 @@ class QrCodeScanView {
mView.removeAllViews();
mView.addView(mOpenSettingsView);
}
/**
* Returns an Intent to show the App Info page for the current app.
* Stop the camera.
*/
private Intent getAppInfoIntent(String packageName) {
Intent intent = new Intent(Settings.ACTION_APPLICATION_DETAILS_SETTINGS);
intent.setData(new Uri.Builder().scheme("package").opaquePart(packageName).build());
return intent;
public void stopCamera() {
if (mCameraPreview != null) {
mCameraPreview.stopCamera();
mCameraPreview = null;
}
}
}
......@@ -30,4 +30,7 @@ public class QrCodeShareCoordinator implements QrCodeDialogTab {
@Override
public void onPause() {}
@Override
public void onDestroy() {}
}
\ No newline at end of file
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