Adding Mouse Support for new GTK Autofill

chrome/browser/autofill/autofill_external_delegate_unittest.cc

Index: chrome/browser/autofill/autofill_external_delegate_unittest.cc
diff --git a/chrome/browser/autofill/autofill_external_delegate_unittest.cc b/chrome/browser/autofill/autofill_external_delegate_unittest.cc
index f9e6f6c0a65f19df99879fde185e8d60d7bbee1e..4b40df7ec2ad077abee961eb307a064f9a2c290f 100644
--- a/chrome/browser/autofill/autofill_external_delegate_unittest.cc
+++ b/chrome/browser/autofill/autofill_external_delegate_unittest.cc
@@ -2,17 +2,17 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "chrome/browser/autofill/autofill_external_delegate_unittest.h"

[Ilya Sherman, 2012/02/04 04:10:52]

Hmm, I don't see this file in the CL -- did you mean to add it or perhaps
#include a different header here?  If this is meant to be used by multiple
tests, it would probably be better to name it slightly differently -- perhaps
"autofill_external_delegate_test_helpers.h" or something along those lines.

[csharp, 2012/02/07 22:30:58]

Ya, I missed including this file. I changed it up a bit to just be a basic
AutofilExternalDelegate with all the pure virtual functions filled in to
removing the repeated code redeclaring those as empty functions.

+
 #include <vector>
 
 #include "base/compiler_specific.h"
 #include "base/memory/ref_counted.h"
 #include "base/string16.h"
-#include "chrome/browser/autofill/autofill_external_delegate.h"
 #include "chrome/browser/autofill/autofill_manager.h"
 #include "chrome/browser/ui/tab_contents/test_tab_contents_wrapper.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/test/test_browser_thread.h"
-#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/gfx/rect.h"
 #include "webkit/forms/form_data.h"
@@ -23,36 +23,18 @@ using testing::_;
 using webkit::forms::FormData;
 using webkit::forms::FormField;
 
-namespace {
-
-class MockAutofillExternalDelegate : public AutofillExternalDelegate {
+class AutofillManagerMock : public AutofillManager {

[Ilya Sherman, 2012/02/04 04:10:52]

nit: Any reason not to move this into an anonymous namespace?

[Ilya Sherman, 2012/02/04 04:10:52]

nit: "AutofillManagerMock" -> "MockAutofillManager"

[csharp, 2012/02/07 22:30:58]

Done.

[csharp, 2012/02/07 22:30:58]

Nope, moved.

  public:
-  explicit MockAutofillExternalDelegate(TabContentsWrapper* wrapper,
-                                        AutofillManager* autofill_manager)
-      : AutofillExternalDelegate(wrapper, autofill_manager) {}
-  virtual ~MockAutofillExternalDelegate() {}
-
-  virtual void HideAutofillPopup() OVERRIDE {}
-
-  MOCK_METHOD5(ApplyAutofillSuggestions, void(
-      const std::vector<string16>& autofill_values,
-      const std::vector<string16>& autofill_labels,
-      const std::vector<string16>& autofill_icons,
-      const std::vector<int>& autofill_unique_ids,
-      int separator_index));
-
-  MOCK_METHOD4(OnQueryPlatformSpecific,
-               void(int query_id,
-                    const webkit::forms::FormData& form,
-                    const webkit::forms::FormField& field,
-                    const gfx::Rect& bounds));
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(MockAutofillExternalDelegate);
+  explicit AutofillManagerMock(TabContentsWrapper* tab_contents)
+      : AutofillManager(tab_contents) {}
+  ~AutofillManagerMock() {}
+
+  virtual void OnFillAutofillFormData(int query_id,
+                                      const webkit::forms::FormData& form,
+                                      const webkit::forms::FormField& field,
+                                      int unique_id) {}
 };
 
-}  // namespace
-
 class AutofillExternalDelegateTest : public TabContentsWrapperTestHarness {
  public:
   AutofillExternalDelegateTest()
@@ -61,11 +43,11 @@ class AutofillExternalDelegateTest : public TabContentsWrapperTestHarness {
 
   virtual void SetUp() OVERRIDE {
     TabContentsWrapperTestHarness::SetUp();
-    autofill_manager_ = new AutofillManager(contents_wrapper());
+    autofill_manager_ = new AutofillManagerMock(contents_wrapper());
   }
 
  protected:
-  scoped_refptr<AutofillManager> autofill_manager_;
+  scoped_refptr<AutofillManagerMock> autofill_manager_;
 
  private:
   content::TestBrowserThread ui_thread_;
@@ -103,4 +85,12 @@ TEST_F(AutofillExternalDelegateTest, TestExternalDelegateVirtualCalls) {
                                           autofill_item,
                                           autofill_item,
                                           autofill_ids);
+
+
+  EXPECT_CALL(external_delegate, HideAutofillPopupInternal());

[Ilya Sherman, 2012/02/04 04:10:52]

nit: It's generally better to test the public interface rather than the private
interface -- any reason not to EXPECT_CALL(external_delegate, HideAutofillPopup)
instead?

[csharp, 2012/02/07 22:30:58]

Nope, fixed.

+
+  // This should trigger a call to hide the popup since
+  // we've selected an option.
+  external_delegate.didAcceptAutofillSuggestions(autofill_item[0],
+                                                 autofill_ids[0], 0);
 }