[Android] Ensure filling component isn't used before initialization

This CL ensures that any access to the ManualFillingCoordinator checks
whether the latter exists in the first place. Because its initialization
happens after the native initialization, this is a possible source of
crashes.

Also, this CL handles that cases in which the bottom control space
changes even if there is no accessory to fill it.

Bug: 876215, 877687
Change-Id: I713ab1343742c858d6ab0277b94cf3190e88e5e2
Reviewed-on: https://chromium-review.googlesource.com/1189805
Commit-Queue: Friedrich Horschig [CEST] <fhorschig@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586732}

chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java

@@ -261,7 +261,6 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
     private BottomSheet mBottomSheet;
     private ContextualSuggestionsCoordinator mContextualSuggestionsCoordinator;
     private ScrimView mScrimView;
-    private ManualFillingCoordinator mManualFillingController;
     private float mStatusBarScrimFraction;
     private int mBaseStatusBarColor;
     private int mScrimColor;
@@ -278,6 +277,8 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
     private Runnable mRecordMultiWindowModeScreenWidthRunnable;
 
     private final DiscardableReferencePool mReferencePool = new DiscardableReferencePool();
+    private final ManualFillingCoordinator mManualFillingController =
+            new ManualFillingCoordinator();
 
     private AssistStatusHandler mAssistStatusHandler;
 
@@ -688,13 +689,6 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
         return mFindToolbarManager;
     }
 
-    /**
-     * @return The {@link KeyboardAccessoryCoordinator} that belongs to this activity.
-     */
-    public KeyboardAccessoryCoordinator getKeyboardAccessory() {
-        return mManualFillingController.getKeyboardAccessory();
-    }
-
     /**
      * @return The {@link KeyboardAccessoryCoordinator} that belongs to this activity.
      */
@@ -939,7 +933,7 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
 
         OfflineIndicatorController.onUpdate();
 
-        if (getManualFillingController() != null) getManualFillingController().onResume();
+        getManualFillingController().onResume();
     }
 
     @Override
@@ -957,7 +951,7 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
         RecordUserAction.record("MobileGoToBackground");
         Tab tab = getActivityTab();
         if (tab != null) getTabContentManager().cacheTabThumbnail(tab);
-        if (getManualFillingController() != null) getManualFillingController().onPause();
+        getManualFillingController().onPause();
 
         VrModuleProvider.getDelegate().maybeUnregisterVrEntryHook();
         markSessionEnd();
@@ -1269,10 +1263,7 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
             mTabContentManager = null;
         }
 
-        if (mManualFillingController != null) {
         mManualFillingController.destroy();
-            mManualFillingController = null;
-        }
 
         if (mActivityTabStartupMetricsTracker != null) {
             mActivityTabStartupMetricsTracker.destroy();
@@ -1383,7 +1374,7 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
             setStatusBarColor(null, mBaseStatusBarColor);
         }, coordinator);
 
-        mManualFillingController = new ManualFillingCoordinator(getWindowAndroid(),
+        mManualFillingController.initialize(getWindowAndroid(),
                 findViewById(R.id.keyboard_accessory_stub),
                 findViewById(R.id.keyboard_accessory_sheet_stub));
 

chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java

@@ -1759,8 +1759,7 @@ public class ChromeTabbedActivity
         // BottomSheet can be opened before native is initialized.
         if (!mUIInitialized) return getBottomSheet() != null && getBottomSheet().handleBackPress();
 
-        if (getManualFillingController() != null && getManualFillingController().handleBackPress())
-            return true;
+        if (getManualFillingController().handleBackPress()) return true;
 
         final Tab currentTab = getActivityTab();
 

chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillKeyboardAccessoryBridge.java

@@ -57,7 +57,7 @@ public class AutofillKeyboardAccessoryBridge
     @Override
     public void suggestionSelected(int listIndex) {
         KeyboardAccessoryMetricsRecorder.recordActionSelected(AccessoryAction.AUTOFILL_SUGGESTION);
-        if (mManualFillingCoordinator != null) mManualFillingCoordinator.dismiss();
+        mManualFillingCoordinator.dismiss();
         if (mNativeAutofillKeyboardAccessory == 0) return;
         nativeSuggestionSelected(mNativeAutofillKeyboardAccessory, listIndex);
     }
@@ -96,9 +96,11 @@ public class AutofillKeyboardAccessoryBridge
         assert mContext != null;
         if (mContext instanceof ChromeActivity) {
             mManualFillingCoordinator = ((ChromeActivity) mContext).getManualFillingController();
+            if (mManualFillingCoordinator.getKeyboardAccessory() != null) {
                 mManualFillingCoordinator.getKeyboardAccessory().registerActionListProvider(
                         mChipProvider);
             }
+        }
 
         mNativeAutofillKeyboardAccessory = nativeAutofillKeyboardAccessory;
     }
@@ -118,7 +120,6 @@ public class AutofillKeyboardAccessoryBridge
     private void dismiss() {
         mChipProvider.notifyObservers(new KeyboardAccessoryData.Action[0]);
         mContext = null;
-        mManualFillingCoordinator = null;
     }
 
     /**

chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryCoordinator.java

@@ -191,6 +191,15 @@ public class KeyboardAccessoryCoordinator {
         return mMediator.isShown();
     }
 
+    /**
+     * This method returns whether the accessory has any contents that justify showing it. A single
+     * tab, action or suggestion chip would already allow that.
+     * @return True if there is any content to be shown. False otherwise.
+     */
+    public boolean hasContents() {
+        return mMediator.hasContents();
+    }
+
     /**
      * Returns whether the active tab is non-null. The returned property reflects the latest change
      * while the view might still be in progress of being updated accordingly.

chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryMediator.java

@@ -171,9 +171,13 @@ class KeyboardAccessoryMediator
         }
     }
 
+    boolean hasContents() {
+        return mModel.getActionList().size() > 0 || mModel.getTabList().size() > 0;
+    }
+
     private boolean shouldShowAccessory() {
         if (!mShowIfNotEmpty && mModel.activeTab() == null) return false;
-        return mModel.getActionList().size() > 0 || mModel.getTabList().size() > 0;
+        return hasContents();
     }
 
     private void updateVisibility() {

chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingCoordinator.java

@@ -4,6 +4,7 @@
 
 package org.chromium.chrome.browser.autofill.keyboard_accessory;
 
+import android.support.annotation.Nullable;
 import android.view.ViewStub;
 
 import org.chromium.base.VisibleForTesting;
@@ -23,12 +24,13 @@ public class ManualFillingCoordinator {
     private final ManualFillingMediator mMediator = new ManualFillingMediator();
 
     /**
-     * Creates a the manual filling controller.
+     * Initializes the manual filling component. Calls to this class are NoOps until
+     * {@link #initialize(WindowAndroid, ViewStub, ViewStub)} is called.
      * @param windowAndroid The window needed to listen to the keyboard and to connect to activity.
      * @param keyboardAccessoryStub The view stub for keyboard accessory bar.
      * @param accessorySheetStub The view stub for the keyboard accessory bottom sheet.
      */
-    public ManualFillingCoordinator(WindowAndroid windowAndroid, ViewStub keyboardAccessoryStub,
+    public void initialize(WindowAndroid windowAndroid, ViewStub keyboardAccessoryStub,
             ViewStub accessorySheetStub) {
         KeyboardAccessoryCoordinator keyboardAccessory =
                 new KeyboardAccessoryCoordinator(keyboardAccessoryStub, mMediator);
@@ -59,6 +61,10 @@ public class ManualFillingCoordinator {
         mMediator.dismiss();
     }
 
+    /**
+     * Notifies the component that a popup window exists so it can be dismissed if necessary.
+     * @param popup A {@link DropdownPopupWindow} that might be dismissed later.
+     */
     public void notifyPopupAvailable(DropdownPopupWindow popup) {
         mMediator.notifyPopupOpened(popup);
     }
@@ -101,7 +107,7 @@ public class ManualFillingCoordinator {
      * the keyboard accessory (e.g. by providing suggestions or actions).
      * @return The coordinator of the Keyboard accessory component.
      */
-    public KeyboardAccessoryCoordinator getKeyboardAccessory() {
+    public @Nullable KeyboardAccessoryCoordinator getKeyboardAccessory() {
         return mMediator.getKeyboardAccessory();
     }
 

chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingMediator.java

@@ -172,10 +172,15 @@ class ManualFillingMediator
         }
     }
 
+    boolean isInitialized() {
+        return mWindowAndroid != null;
+    }
+
     // TODO(fhorschig): Look for stronger signals than |keyboardVisibilityChanged|.
     // This variable remembers the last state of |keyboardVisibilityChanged| which might not be
     // sufficient for edge cases like hardware keyboards, floating keyboards, etc.
     private void onKeyboardVisibilityChanged(boolean isShowing) {
+        if (!mKeyboardAccessory.hasContents()) return; // Exit early to not affect the layout.
         if (isShowing) {
             mKeyboardAccessory.requestShowing();
             mActivity.getFullscreenManager().setBottomControlsHeight(calculateAccessoryBarHeight());
@@ -184,22 +189,21 @@ class ManualFillingMediator
             mAccessorySheet.hide();
         } else {
             mKeyboardAccessory.close();
+            onBottomControlSpaceChanged();
             if (mKeyboardAccessory.hasActiveTab()) {
-                mKeyboardAccessory.setBottomOffset(mAccessorySheet.getHeight());
                 mAccessorySheet.show();
-                onBottomControlSpaceChanged();
-            } else {
-                mActivity.getFullscreenManager().setBottomControlsHeight(mPreviousControlHeight);
             }
         }
     }
 
     void registerPasswordProvider(Provider<KeyboardAccessoryData.Item> itemProvider) {
+        if (!isInitialized()) return;
         assert getPasswordAccessorySheet() != null : "No password sheet available!";
         getPasswordAccessorySheet().registerItemProvider(itemProvider);
     }
 
     void registerActionProvider(KeyboardAccessoryData.PropertyProvider<Action> actionProvider) {
+        if (!isInitialized()) return;
         ActionProviderCacheAdapter adapter =
                 new ActionProviderCacheAdapter(mActiveBrowserTab, actionProvider, new Action[0]);
         mModel.get(mActiveBrowserTab).mActionsProvider = adapter;
@@ -207,6 +211,7 @@ class ManualFillingMediator
     }
 
     void destroy() {
+        if (!isInitialized()) return;
         if (mWindowAndroid != null) {
             mWindowAndroid.removeKeyboardVisibilityListener(mVisibilityListener);
         }
@@ -214,7 +219,7 @@ class ManualFillingMediator
     }
 
     boolean handleBackPress() {
-        if (mAccessorySheet.isShown()) {
+        if (isInitialized() && mAccessorySheet.isShown()) {
             mKeyboardAccessory.dismiss();
             return true;
         }
@@ -222,6 +227,7 @@ class ManualFillingMediator
     }
 
     void dismiss() {
+        if (!isInitialized()) return;
         mKeyboardAccessory.dismiss();
         if (getContentView() != null) UiUtils.hideKeyboard(getContentView());
     }
@@ -231,10 +237,12 @@ class ManualFillingMediator
     }
 
     public void pause() {
+        if (!isInitialized()) return;
         mKeyboardAccessory.dismiss();
     }
 
     void resume() {
+        if (!isInitialized()) return;
         restoreCachedState(mActiveBrowserTab);
     }
 
@@ -266,7 +274,7 @@ class ManualFillingMediator
      * Opens the keyboard which implicitly dismisses the sheet. Without open sheet, this is a NoOp.
      */
     void swapSheetWithKeyboard() {
-        if (mAccessorySheet.isShown()) onOpenKeyboard();
+        if (isInitialized() && mAccessorySheet.isShown()) onOpenKeyboard();
     }
 
     @Override
@@ -278,12 +286,15 @@ class ManualFillingMediator
 
     @Override
     public void onBottomControlSpaceChanged() {
-        @Px
         int newControlsHeight = calculateAccessoryBarHeight();
-        if (mAccessorySheet.isShown()) {
+        int newControlsOffset = 0;
+        if (mKeyboardAccessory.hasActiveTab()) {
             newControlsHeight += mAccessorySheet.getHeight();
+            newControlsOffset += mAccessorySheet.getHeight();
         }
-        mActivity.getFullscreenManager().setBottomControlsHeight(newControlsHeight);
+        mKeyboardAccessory.setBottomOffset(newControlsOffset);
+        mActivity.getFullscreenManager().setBottomControlsHeight(
+                mKeyboardAccessory.isShown() ? newControlsHeight : mPreviousControlHeight);
     }
 
     /**
@@ -342,6 +353,7 @@ class ManualFillingMediator
 
     @VisibleForTesting
     void addTab(KeyboardAccessoryData.Tab tab) {
+        if (!isInitialized()) return;
         // TODO(fhorschig): This should add the tab only to the state. Sheet and accessory should be
         // using a |set| method or even observe the state.
         mKeyboardAccessory.addTab(tab);
@@ -351,6 +363,7 @@ class ManualFillingMediator
     @VisibleForTesting
     @Nullable
     PasswordAccessorySheetCoordinator getPasswordAccessorySheet() {
+        if (!isInitialized()) return null;
         if (!ChromeFeatureList.isEnabled(ChromeFeatureList.EXPERIMENTAL_UI)
                 && !ChromeFeatureList.isEnabled(ChromeFeatureList.PASSWORDS_KEYBOARD_ACCESSORY)) {
             return null;
@@ -365,6 +378,7 @@ class ManualFillingMediator
     }
 
     // TODO(fhorschig): Should be @VisibleForTesting.
+    @Nullable
     KeyboardAccessoryCoordinator getKeyboardAccessory() {
         return mKeyboardAccessory;
     }

chrome/android/junit/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingControllerTest.java

@@ -86,7 +86,7 @@ public class ManualFillingControllerTest {
     @Rule
     public Features.JUnitProcessor mFeaturesProcessor = new Features.JUnitProcessor();
 
-    private ManualFillingCoordinator mController;
+    private ManualFillingCoordinator mController = new ManualFillingCoordinator();
 
     @Before
     public void setUp() {
@@ -100,7 +100,7 @@ public class ManualFillingControllerTest {
         when(mMockActivity.getResources()).thenReturn(mMockResources);
         when(mMockResources.getDimensionPixelSize(anyInt())).thenReturn(48);
         PasswordAccessorySheetCoordinator.IconProvider.getInstance().setIconForTesting(mMockIcon);
-        mController = new ManualFillingCoordinator(mMockWindow, mMockViewStub, mMockViewStub);
+        mController.initialize(mMockWindow, mMockViewStub, mMockViewStub);
     }
 
     @Test