[Android] Pass fully initialized view to accessory adapter or exit

As described in the linked bug, the adapter assumed WindowAndroid to
never be null which causes a crash.
With this CL, the view will be initialized early on and if WindowAndroid
is nullptr, the creation of the view (and the adapter) will be aborted.

To do that, the unused constructor parameters for label width and length
were removed.

Bug: 1048849, 1049090
Change-Id: I60151a560907a984ba4e45ee949624400b8f86da
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2037672
Commit-Queue: Friedrich [CET] <fhorschig@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738564}

chrome/android/features/keyboard_accessory/internal/java/src/org/chromium/chrome/browser/keyboard_accessory/AutofillKeyboardAccessoryViewBridge.java

@@ -79,15 +79,9 @@ public class AutofillKeyboardAccessoryViewBridge
      * This function should be called at most one time.
      * @param nativeAutofillKeyboardAccessory Handle to the native counterpart.
      * @param windowAndroid The window on which to show the suggestions.
-     * @param animationDurationMillis If 0, do not animate. Otherwise, animation duration in each
-     *                                direction. We reverse animation to scroll the first suggestion
-     *                                (which is a hint to call attention to the accessory) out of
-     *                                the viewport at the end of the reversed animation.
-     * @param shouldLimitLabelWidth If true, limit suggestion label width to 1/2 device's width.
      */
     @CalledByNative
-    private void init(long nativeAutofillKeyboardAccessory, WindowAndroid windowAndroid,
-            int animationDurationMillis, boolean shouldLimitLabelWidth) {
+    private void init(long nativeAutofillKeyboardAccessory, WindowAndroid windowAndroid) {
         mContext = windowAndroid.getActivity().get();
         assert mContext != null;
         if (mContext instanceof ChromeActivity) {

chrome/browser/autofill/autofill_keyboard_accessory_adapter.cc

@@ -35,12 +35,8 @@ base::string16 CreateLabel(const Suggestion& suggestion) {
 }  // namespace
 
 AutofillKeyboardAccessoryAdapter::AutofillKeyboardAccessoryAdapter(
-    base::WeakPtr<AutofillPopupController> controller,
-    unsigned int animation_duration_millis,
-    bool should_limit_label_width)
-    : controller_(controller),
-      animation_duration_millis_(animation_duration_millis),
-      should_limit_label_width_(should_limit_label_width) {}
+    base::WeakPtr<AutofillPopupController> controller)
+    : controller_(controller) {}
 
 AutofillKeyboardAccessoryAdapter::~AutofillKeyboardAccessoryAdapter() = default;
 
@@ -48,7 +44,6 @@ AutofillKeyboardAccessoryAdapter::~AutofillKeyboardAccessoryAdapter() = default;
 
 void AutofillKeyboardAccessoryAdapter::Show() {
   DCHECK(view_) << "Show called before a View was set!";
-  view_->Initialize(animation_duration_millis_, should_limit_label_width_);
   OnSuggestionsChanged();
 }
 

chrome/browser/autofill/autofill_keyboard_accessory_adapter.h

@@ -26,9 +26,7 @@ class AutofillKeyboardAccessoryAdapter : public AutofillPopupView,
                                          public AutofillPopupController {
  public:
   AutofillKeyboardAccessoryAdapter(
-      base::WeakPtr<AutofillPopupController> controller,
-      unsigned int animation_duration_millis,
-      bool should_limit_label_width);
+      base::WeakPtr<AutofillPopupController> controller);
   ~AutofillKeyboardAccessoryAdapter() override;
 
   // Interface describing the minimal capabilities for the native view.
@@ -36,9 +34,9 @@ class AutofillKeyboardAccessoryAdapter : public AutofillPopupView,
    public:
     virtual ~AccessoryView() = default;
 
-    // Initializes the Java-side of this bridge.
-    virtual void Initialize(unsigned int animation_duration_millis,
-                            bool should_limit_label_width) = 0;
+    // Initializes the Java-side of this bridge. Returns true after a successful
+    // creation and false otherwise.
+    virtual bool Initialize() = 0;
 
     // Requests to dismiss this view.
     virtual void Hide() = 0;
@@ -106,12 +104,6 @@ class AutofillKeyboardAccessoryAdapter : public AutofillPopupView,
   // The labels to be used for the input chips.
   std::vector<base::string16> labels_;
 
-  // If 0, don't animate suggestion view.
-  const unsigned int animation_duration_millis_;
-
-  // If true, limits label width to 1/2 device's width.
-  const bool should_limit_label_width_;
-
   // Position that the front element has in the suggestion list returned by
   // controller_. It is used to determine the offset suggestions.
   base::Optional<int> front_element_;

chrome/browser/autofill/autofill_keyboard_accessory_adapter_unittest.cc

@@ -37,7 +37,7 @@ class MockAccessoryView
     : public AutofillKeyboardAccessoryAdapter::AccessoryView {
  public:
   MockAccessoryView() {}
-  MOCK_METHOD2(Initialize, void(unsigned int, bool));
+  MOCK_METHOD0(Initialize, bool());
   MOCK_METHOD0(Hide, void());
   MOCK_METHOD0(Show, void());
   MOCK_METHOD3(ConfirmDeletion,
@@ -116,7 +116,7 @@ class AutofillKeyboardAccessoryAdapterTest : public testing::Test {
 
     autofill_accessory_adapter_ =
         std::make_unique<AutofillKeyboardAccessoryAdapter>(
-            popup_controller_->GetWeakPtr(), 0, false);
+            popup_controller_->GetWeakPtr());
     autofill_accessory_adapter_->SetAccessoryView(std::move(view));
   }
 
@@ -151,11 +151,7 @@ class AutofillKeyboardAccessoryAdapterTest : public testing::Test {
 };
 
 TEST_F(AutofillKeyboardAccessoryAdapterTest, ShowingInitializesAndUpdatesView) {
-  {
-    ::testing::Sequence s;
-    EXPECT_CALL(*view(), Initialize(_, _));
-    EXPECT_CALL(*view(), Show());
-  }
+  EXPECT_CALL(*view(), Show());
   adapter_as_view()->Show();
 }
 

chrome/browser/ui/android/autofill/autofill_keyboard_accessory_view.cc

@@ -38,16 +38,17 @@ AutofillKeyboardAccessoryView::~AutofillKeyboardAccessoryView() {
       base::android::AttachCurrentThread(), java_object_);
 }
 
-void AutofillKeyboardAccessoryView::Initialize(
-    unsigned int animation_duration_millis,
-    bool should_limit_label_width) {
+bool AutofillKeyboardAccessoryView::Initialize() {
   ui::ViewAndroid* view_android = controller_->container_view();
-  DCHECK(view_android);
+  if (!view_android)
+    return false;
+  ui::WindowAndroid* window_android = view_android->GetWindowAndroid();
+  if (!window_android)
+    return false;  // The window might not be attached (yet or anymore).
   Java_AutofillKeyboardAccessoryViewBridge_init(
       base::android::AttachCurrentThread(), java_object_,
-      reinterpret_cast<intptr_t>(this),
-      view_android->GetWindowAndroid()->GetJavaObject(),
-      animation_duration_millis, should_limit_label_width);
+      reinterpret_cast<intptr_t>(this), window_android->GetJavaObject());
+  return true;
 }
 
 void AutofillKeyboardAccessoryView::Hide() {

chrome/browser/ui/android/autofill/autofill_keyboard_accessory_view.h

@@ -30,8 +30,7 @@ class AutofillKeyboardAccessoryView
   ~AutofillKeyboardAccessoryView() override;
 
   // Implementation of AutofillKeyboardAccessoryAdapter::AccessoryView.
-  void Initialize(unsigned int animation_duration_millis,
-                  bool limit_label_width) override;
+  bool Initialize() override;
   void Hide() override;
   void Show() override;
   void ConfirmDeletion(const base::string16& confirmation_title,

chrome/browser/ui/android/autofill/autofill_popup_view_android.cc

@@ -159,7 +159,7 @@ void AutofillPopupViewAndroid::PopupDismissed(
   delete this;
 }
 
-void AutofillPopupViewAndroid::Init() {
+bool AutofillPopupViewAndroid::Init() {
   JNIEnv* env = base::android::AttachCurrentThread();
   ui::ViewAndroid* view_android = controller_->container_view();
 
@@ -167,11 +167,15 @@ void AutofillPopupViewAndroid::Init() {
   popup_view_ = view_android->AcquireAnchorView();
   const ScopedJavaLocalRef<jobject> view = popup_view_.view();
   if (view.is_null())
-    return;
+    return false;
+  ui::WindowAndroid* window_android = view_android->GetWindowAndroid();
+  if (!window_android)
+    return false;  // The window might not be attached (yet or anymore).
 
   java_object_.Reset(Java_AutofillPopupBridge_create(
       env, view, reinterpret_cast<intptr_t>(this),
-      view_android->GetWindowAndroid()->GetJavaObject()));
+      window_android->GetJavaObject()));
+  return true;
 }
 
 bool AutofillPopupViewAndroid::WasSuppressed() {
@@ -184,18 +188,21 @@ bool AutofillPopupViewAndroid::WasSuppressed() {
 AutofillPopupView* AutofillPopupView::Create(
     base::WeakPtr<AutofillPopupController> controller) {
   if (IsKeyboardAccessoryEnabled()) {
-    auto adapter = std::make_unique<AutofillKeyboardAccessoryAdapter>(
-        controller, GetKeyboardAccessoryAnimationDuration(),
-        ShouldLimitKeyboardAccessorySuggestionLabelWidth());
-    adapter->SetAccessoryView(
-        std::make_unique<AutofillKeyboardAccessoryView>(adapter.get()));
+    auto adapter =
+        std::make_unique<AutofillKeyboardAccessoryAdapter>(controller);
+    auto accessory_view =
+        std::make_unique<AutofillKeyboardAccessoryView>(adapter.get());
+    if (!accessory_view->Initialize())
+      return nullptr;  // Don't create an adapter without initialized view.
+    adapter->SetAccessoryView(std::move(accessory_view));
     return adapter.release();
   }
 
   auto popup_view =
       std::make_unique<AutofillPopupViewAndroid>(controller.get());
-  popup_view->Init();
-  return popup_view->WasSuppressed() ? nullptr : popup_view.release();
+  if (!popup_view->Init() || popup_view->WasSuppressed())
+    return nullptr;
+  return popup_view.release();
 }
 
 }  // namespace autofill

chrome/browser/ui/android/autofill/autofill_popup_view_android.h

@@ -53,7 +53,7 @@ class AutofillPopupViewAndroid : public AutofillPopupView {
  private:
   friend class AutofillPopupView;
   // Creates the AutofillPopupBridge Java object.
-  void Init();
+  bool Init();
   // Returns whether the dropdown was suppressed (mainly due to not enough
   // screen space available).
   bool WasSuppressed();