diff options
author | jeanfrancoisg@chromium.org <jeanfrancoisg@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-11-22 15:34:33 +0000 |
---|---|---|
committer | jeanfrancoisg@chromium.org <jeanfrancoisg@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-11-22 15:34:33 +0000 |
commit | d619ba647607147e95950ec6c045b93486bd6e92 (patch) | |
tree | d3f378825edf44be3b066bbfe913f13f20e31433 /components | |
parent | f04b0e97b29c20ff2119a5e842f766b0e604b44d (diff) | |
download | chromium_src-d619ba647607147e95950ec6c045b93486bd6e92.zip chromium_src-d619ba647607147e95950ec6c045b93486bd6e92.tar.gz chromium_src-d619ba647607147e95950ec6c045b93486bd6e92.tar.bz2 |
Abstracted AcceptPasswordAutofillSuggestion IPC out of core Autofill code.
BUG=302479
Review URL: https://codereview.chromium.org/79103002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@236768 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'components')
19 files changed, 103 insertions, 54 deletions
diff --git a/components/autofill/content/browser/autofill_driver_impl.cc b/components/autofill/content/browser/autofill_driver_impl.cc index 5e21fb1..ad2454e 100644 --- a/components/autofill/content/browser/autofill_driver_impl.cc +++ b/components/autofill/content/browser/autofill_driver_impl.cc @@ -61,7 +61,7 @@ AutofillDriverImpl::AutofillDriverImpl( : content::WebContentsObserver(web_contents), autofill_manager_(new AutofillManager( this, delegate, app_locale, enable_download_manager)), - autofill_external_delegate_(web_contents, autofill_manager_.get(), this) { + autofill_external_delegate_(autofill_manager_.get(), this) { autofill_manager_->SetExternalDelegate(&autofill_external_delegate_); } @@ -139,6 +139,16 @@ void AutofillDriverImpl::RendererShouldAcceptDataListSuggestion( value)); } +void AutofillDriverImpl::RendererShouldAcceptPasswordAutofillSuggestion( + const base::string16& username) { + if (!RendererIsAvailable()) + return; + content::RenderViewHost* host = web_contents()->GetRenderViewHost(); + host->Send( + new AutofillMsg_AcceptPasswordAutofillSuggestion(host->GetRoutingID(), + username)); +} + void AutofillDriverImpl::RendererShouldClearFilledForm() { if (!RendererIsAvailable()) return; diff --git a/components/autofill/content/browser/autofill_driver_impl.h b/components/autofill/content/browser/autofill_driver_impl.h index 27bdcdf..6f4393f 100644 --- a/components/autofill/content/browser/autofill_driver_impl.h +++ b/components/autofill/content/browser/autofill_driver_impl.h @@ -55,6 +55,8 @@ class AutofillDriverImpl : public AutofillDriver, const std::vector<FormStructure*>& forms) OVERRIDE; virtual void RendererShouldAcceptDataListSuggestion( const base::string16& value) OVERRIDE; + virtual void RendererShouldAcceptPasswordAutofillSuggestion( + const base::string16& username) OVERRIDE; virtual void RendererShouldClearFilledForm() OVERRIDE; virtual void RendererShouldClearPreviewedForm() OVERRIDE; virtual void RendererShouldSetNodeText(const base::string16& value) OVERRIDE; diff --git a/components/autofill/content/browser/autofill_driver_impl_unittest.cc b/components/autofill/content/browser/autofill_driver_impl_unittest.cc index 0785c40..a4651e2 100644 --- a/components/autofill/content/browser/autofill_driver_impl_unittest.cc +++ b/components/autofill/content/browser/autofill_driver_impl_unittest.cc @@ -142,6 +142,11 @@ class AutofillDriverImplTest : public ChromeRenderViewHostTestHarness { case AutofillMsg_AcceptDataListSuggestion::ID: AutofillMsg_AcceptDataListSuggestion::Read(message, &autofill_param); break; + case AutofillMsg_AcceptPasswordAutofillSuggestion::ID: + AutofillMsg_AcceptPasswordAutofillSuggestion::Read( + message, + &autofill_param); + break; default: NOTREACHED(); } @@ -256,6 +261,16 @@ TEST_F(AutofillDriverImplTest, AcceptDataListSuggestion) { EXPECT_EQ(input_value, output_value); } +TEST_F(AutofillDriverImplTest, AcceptPasswordAutofillSuggestion) { + base::string16 input_value(ASCIIToUTF16("barbaz")); + base::string16 output_value; + driver_->RendererShouldAcceptPasswordAutofillSuggestion(input_value); + EXPECT_TRUE(GetString16FromMessageWithID( + AutofillMsg_AcceptPasswordAutofillSuggestion::ID, + &output_value)); + EXPECT_EQ(input_value, output_value); +} + TEST_F(AutofillDriverImplTest, ClearFilledFormSentToRenderer) { driver_->RendererShouldClearFilledForm(); EXPECT_TRUE(HasMessageMatchingID(AutofillMsg_ClearForm::ID)); diff --git a/components/autofill/content/renderer/autofill_agent.cc b/components/autofill/content/renderer/autofill_agent.cc index c8a5e0e..25b02c3 100644 --- a/components/autofill/content/renderer/autofill_agent.cc +++ b/components/autofill/content/renderer/autofill_agent.cc @@ -461,13 +461,13 @@ void AutofillAgent::OnAcceptDataListSuggestion(const base::string16& value) { } void AutofillAgent::OnAcceptPasswordAutofillSuggestion( - const base::string16& value) { + const base::string16& username) { // We need to make sure this is handled here because the browser process // skipped it handling because it believed it would be handled here. If it // isn't handled here then the browser logic needs to be updated. bool handled = password_autofill_agent_->DidAcceptAutofillSuggestion( element_, - value); + username); DCHECK(handled); } diff --git a/components/autofill/content/renderer/autofill_agent.h b/components/autofill/content/renderer/autofill_agent.h index 1f25703..a16feeb 100644 --- a/components/autofill/content/renderer/autofill_agent.h +++ b/components/autofill/content/renderer/autofill_agent.h @@ -103,7 +103,7 @@ class AutofillAgent : public content::RenderViewObserver, void OnClearPreviewedForm(); void OnSetNodeText(const base::string16& value); void OnAcceptDataListSuggestion(const base::string16& value); - void OnAcceptPasswordAutofillSuggestion(const base::string16& value); + void OnAcceptPasswordAutofillSuggestion(const base::string16& username); // Called when interactive autocomplete finishes. void OnRequestAutocompleteResult( diff --git a/components/autofill/content/renderer/password_autofill_agent.cc b/components/autofill/content/renderer/password_autofill_agent.cc index b6f903e..034ec42 100644 --- a/components/autofill/content/renderer/password_autofill_agent.cc +++ b/components/autofill/content/renderer/password_autofill_agent.cc @@ -290,15 +290,15 @@ bool PasswordAutofillAgent::TextFieldHandlingKeyDown( bool PasswordAutofillAgent::DidAcceptAutofillSuggestion( const blink::WebNode& node, - const blink::WebString& value) { + const blink::WebString& username) { blink::WebInputElement input; PasswordInfo password; if (!FindLoginInfo(node, &input, &password)) return false; - // Set the incoming |value| in the text field and |FillUserNameAndPassword| + // Set the incoming |username| in the text field and |FillUserNameAndPassword| // will do the rest. - input.setValue(value, true); + input.setValue(username, true); return FillUserNameAndPassword(&input, &password.password_field, password.fill_data, true /* exact_username_match */, diff --git a/components/autofill/content/renderer/password_autofill_agent.h b/components/autofill/content/renderer/password_autofill_agent.h index c426f05..503621b 100644 --- a/components/autofill/content/renderer/password_autofill_agent.h +++ b/components/autofill/content/renderer/password_autofill_agent.h @@ -38,10 +38,10 @@ class PasswordAutofillAgent : public content::RenderViewObserver { bool TextFieldHandlingKeyDown(const blink::WebInputElement& element, const blink::WebKeyboardEvent& event); - // Fills the password associated with user name |value|. Returns true if the - // username and password fields were filled, false otherwise. + // Fills the password associated with user name |username|. Returns true if + // the username and password fields were filled, false otherwise. bool DidAcceptAutofillSuggestion(const blink::WebNode& node, - const blink::WebString& value); + const blink::WebString& username); // A no-op. Password forms are not previewed, so they do not need to be // cleared when the selection changes. However, this method returns // true when |node| is fillable by password Autofill. diff --git a/components/autofill/core/browser/autocomplete_history_manager_unittest.cc b/components/autofill/core/browser/autocomplete_history_manager_unittest.cc index 8fd5a3a..7fd26a7 100644 --- a/components/autofill/core/browser/autocomplete_history_manager_unittest.cc +++ b/components/autofill/core/browser/autocomplete_history_manager_unittest.cc @@ -29,7 +29,6 @@ #include "testing/gtest/include/gtest/gtest.h" #include "ui/gfx/rect.h" -using content::WebContents; using testing::_; namespace autofill { @@ -182,11 +181,9 @@ namespace { class MockAutofillExternalDelegate : public AutofillExternalDelegate { public: - MockAutofillExternalDelegate(content::WebContents* web_contents, - AutofillManager* autofill_manager, + MockAutofillExternalDelegate(AutofillManager* autofill_manager, AutofillDriver* autofill_driver) - : AutofillExternalDelegate(web_contents, autofill_manager, - autofill_driver) {} + : AutofillExternalDelegate(autofill_manager, autofill_driver) {} virtual ~MockAutofillExternalDelegate() {} MOCK_METHOD5(OnSuggestionsReturned, @@ -225,8 +222,7 @@ TEST_F(AutocompleteHistoryManagerTest, ExternalDelegate) { "en-US", AutofillManager::ENABLE_AUTOFILL_DOWNLOAD_MANAGER)); - MockAutofillExternalDelegate external_delegate(web_contents(), - autofill_manager.get(), + MockAutofillExternalDelegate external_delegate(autofill_manager.get(), autofill_driver_.get()); autocomplete_history_manager.SetExternalDelegate(&external_delegate); diff --git a/components/autofill/core/browser/autofill_driver.h b/components/autofill/core/browser/autofill_driver.h index 2d6cc64..b599c4d 100644 --- a/components/autofill/core/browser/autofill_driver.h +++ b/components/autofill/core/browser/autofill_driver.h @@ -78,6 +78,11 @@ class AutofillDriver { virtual void RendererShouldAcceptDataListSuggestion( const base::string16& value) = 0; + // Tells the renderer to accept the password autofill suggestion for + // |username|. + virtual void RendererShouldAcceptPasswordAutofillSuggestion( + const base::string16& username) = 0; + // Tells the renderer to clear the currently filled Autofill results. virtual void RendererShouldClearFilledForm() = 0; diff --git a/components/autofill/core/browser/autofill_external_delegate.cc b/components/autofill/core/browser/autofill_external_delegate.cc index 51954b9..7bbdeec 100644 --- a/components/autofill/core/browser/autofill_external_delegate.cc +++ b/components/autofill/core/browser/autofill_external_delegate.cc @@ -8,7 +8,6 @@ #include "components/autofill/core/browser/autocomplete_history_manager.h" #include "components/autofill/core/browser/autofill_driver.h" #include "components/autofill/core/browser/autofill_manager.h" -#include "content/public/browser/web_contents.h" #include "grit/component_strings.h" #include "third_party/WebKit/public/web/WebAutofillClient.h" #include "ui/base/l10n/l10n_util.h" @@ -18,12 +17,11 @@ using blink::WebAutofillClient; namespace autofill { AutofillExternalDelegate::AutofillExternalDelegate( - content::WebContents* web_contents, AutofillManager* autofill_manager, AutofillDriver* autofill_driver) : autofill_manager_(autofill_manager), autofill_driver_(autofill_driver), - password_autofill_manager_(web_contents), + password_autofill_manager_(autofill_driver), autofill_query_id_(0), display_warning_if_disabled_(false), has_autofill_suggestion_(false), diff --git a/components/autofill/core/browser/autofill_external_delegate.h b/components/autofill/core/browser/autofill_external_delegate.h index 95c483b..6f3f4d7 100644 --- a/components/autofill/core/browser/autofill_external_delegate.h +++ b/components/autofill/core/browser/autofill_external_delegate.h @@ -34,10 +34,9 @@ class AutofillManager; class AutofillExternalDelegate : public AutofillPopupDelegate { public: - // Creates an AutofillExternalDelegate for the specified contents, - // AutofillManager, and AutofillDriver. - AutofillExternalDelegate(content::WebContents* web_contents, - AutofillManager* autofill_manager, + // Creates an AutofillExternalDelegate for the specified AutofillManager and + // AutofillDriver. + AutofillExternalDelegate(AutofillManager* autofill_manager, AutofillDriver* autofill_driver); virtual ~AutofillExternalDelegate(); diff --git a/components/autofill/core/browser/autofill_external_delegate_unittest.cc b/components/autofill/core/browser/autofill_external_delegate_unittest.cc index 134915e..fe0f6f7 100644 --- a/components/autofill/core/browser/autofill_external_delegate_unittest.cc +++ b/components/autofill/core/browser/autofill_external_delegate_unittest.cc @@ -109,7 +109,6 @@ class AutofillExternalDelegateUnitTest &manager_delegate_)); external_delegate_.reset( new AutofillExternalDelegate( - web_contents(), autofill_manager_.get(), autofill_driver_.get())); } diff --git a/components/autofill/core/browser/autofill_manager_unittest.cc b/components/autofill/core/browser/autofill_manager_unittest.cc index 3b0f7c0..690c899 100644 --- a/components/autofill/core/browser/autofill_manager_unittest.cc +++ b/components/autofill/core/browser/autofill_manager_unittest.cc @@ -542,8 +542,7 @@ class TestAutofillExternalDelegate : public AutofillExternalDelegate { explicit TestAutofillExternalDelegate(content::WebContents* web_contents, AutofillManager* autofill_manager, AutofillDriver* autofill_driver) - : AutofillExternalDelegate(web_contents, autofill_manager, - autofill_driver), + : AutofillExternalDelegate(autofill_manager, autofill_driver), on_query_seen_(false), on_suggestions_returned_seen_(false) {} virtual ~TestAutofillExternalDelegate() {} diff --git a/components/autofill/core/browser/autofill_metrics_unittest.cc b/components/autofill/core/browser/autofill_metrics_unittest.cc index 5cbd6f6..8eb952b 100644 --- a/components/autofill/core/browser/autofill_metrics_unittest.cc +++ b/components/autofill/core/browser/autofill_metrics_unittest.cc @@ -295,7 +295,6 @@ void AutofillMetricsTest::SetUp() { autofill_driver_.get(), manager_delegate, personal_data_.get())); external_delegate_.reset(new AutofillExternalDelegate( - web_contents(), autofill_manager_.get(), autofill_driver_.get())); autofill_manager_->SetExternalDelegate(external_delegate_.get()); diff --git a/components/autofill/core/browser/password_autofill_manager.cc b/components/autofill/core/browser/password_autofill_manager.cc index def93f4..730c10e 100644 --- a/components/autofill/core/browser/password_autofill_manager.cc +++ b/components/autofill/core/browser/password_autofill_manager.cc @@ -2,10 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "base/logging.h" +#include "components/autofill/core/browser/autofill_driver.h" #include "components/autofill/core/browser/password_autofill_manager.h" -#include "components/autofill/core/common/autofill_messages.h" -#include "content/public/browser/render_view_host.h" -#include "content/public/browser/web_contents.h" #include "ui/events/keycodes/keyboard_codes.h" namespace autofill { @@ -14,7 +13,8 @@ namespace autofill { // PasswordAutofillManager, public: PasswordAutofillManager::PasswordAutofillManager( - content::WebContents* web_contents) : web_contents_(web_contents) { + AutofillDriver* autofill_driver) : autofill_driver_(autofill_driver) { + DCHECK(autofill_driver); } PasswordAutofillManager::~PasswordAutofillManager() { @@ -22,19 +22,13 @@ PasswordAutofillManager::~PasswordAutofillManager() { bool PasswordAutofillManager::DidAcceptAutofillSuggestion( const FormFieldData& field, - const base::string16& value) { + const base::string16& username) { PasswordFormFillData password; if (!FindLoginInfo(field, &password)) return false; - if (WillFillUserNameAndPassword(value, password)) { - if (web_contents_) { - content::RenderViewHost* render_view_host = - web_contents_->GetRenderViewHost(); - render_view_host->Send(new AutofillMsg_AcceptPasswordAutofillSuggestion( - render_view_host->GetRoutingID(), - value)); - } + if (WillFillUserNameAndPassword(username, password)) { + autofill_driver_->RendererShouldAcceptPasswordAutofillSuggestion(username); return true; } diff --git a/components/autofill/core/browser/password_autofill_manager.h b/components/autofill/core/browser/password_autofill_manager.h index 97af87f..ddde78e 100644 --- a/components/autofill/core/browser/password_autofill_manager.h +++ b/components/autofill/core/browser/password_autofill_manager.h @@ -16,22 +16,20 @@ #include "components/autofill/core/common/password_form_fill_data.h" -namespace content { -class WebContents; -} // namespace content - namespace autofill { +class AutofillDriver; + // This class is responsible for filling password forms. class PasswordAutofillManager { public: - explicit PasswordAutofillManager(content::WebContents* web_contents); + explicit PasswordAutofillManager(AutofillDriver* autofill_driver); virtual ~PasswordAutofillManager(); - // Fills the password associated with user name |value|. Returns true if the - // username and password fields were filled, false otherwise. + // Fills the password associated with user name |username|. Returns true if + // the username and password fields were filled, false otherwise. bool DidAcceptAutofillSuggestion(const FormFieldData& field, - const base::string16& value); + const base::string16& username); // Invoked when a password mapping is added. void AddPasswordFormMapping( @@ -62,10 +60,9 @@ class PasswordAutofillManager { // The logins we have filled so far with their associated info. LoginToPasswordInfoMap login_to_password_info_; - // We only need the RenderViewHost pointer in WebContents, but if we attempt - // to just store RenderViewHost on creation, it becomes invalid once we start - // using it. By having the WebContents we can always get a valid pointer. - content::WebContents* web_contents_; // Weak reference. + // Provides driver-level context to the shared code of the component. Must + // outlive |this|. + AutofillDriver* const autofill_driver_; // weak DISALLOW_COPY_AND_ASSIGN(PasswordAutofillManager); }; diff --git a/components/autofill/core/browser/password_autofill_manager_unittest.cc b/components/autofill/core/browser/password_autofill_manager_unittest.cc index 96ce202..a5bb791 100644 --- a/components/autofill/core/browser/password_autofill_manager_unittest.cc +++ b/components/autofill/core/browser/password_autofill_manager_unittest.cc @@ -3,8 +3,11 @@ // found in the LICENSE file. #include "base/compiler_specific.h" +#include "base/message_loop/message_loop.h" #include "base/strings/utf_string_conversions.h" #include "components/autofill/core/browser/password_autofill_manager.h" +#include "components/autofill/core/browser/test_autofill_driver.h" +#include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" // The name of the username/password element in the form. @@ -15,11 +18,23 @@ const char kPasswordName[] = "password"; const char kAliceUsername[] = "alice"; const char kAlicePassword[] = "password"; +namespace { + +class MockAutofillDriver : public autofill::TestAutofillDriver { + public: + MockAutofillDriver() : autofill::TestAutofillDriver(NULL) {} + MOCK_METHOD1(RendererShouldAcceptPasswordAutofillSuggestion, + void(const base::string16&)); +}; + +} // namespace + namespace autofill { class PasswordAutofillManagerTest : public testing::Test { protected: - PasswordAutofillManagerTest() : password_autofill_manager_(NULL) {} + PasswordAutofillManagerTest() : + password_autofill_manager_(&autofill_driver_) {} virtual void SetUp() OVERRIDE { // Add a preferred login and an additional login to the FillData. @@ -39,6 +54,10 @@ class PasswordAutofillManagerTest : public testing::Test { fill_data_); } + MockAutofillDriver* autofill_driver() { + return &autofill_driver_; + } + PasswordAutofillManager* password_autofill_manager() { return &password_autofill_manager_; } @@ -49,12 +68,23 @@ class PasswordAutofillManagerTest : public testing::Test { PasswordFormFillData fill_data_; FormFieldData username_field_; + // The TestAutofillDriver uses a SequencedWorkerPool which expects the + // existence of a MessageLoop. + base::MessageLoop message_loop_; + MockAutofillDriver autofill_driver_; PasswordAutofillManager password_autofill_manager_; }; TEST_F(PasswordAutofillManagerTest, DidAcceptAutofillSuggestion) { + EXPECT_CALL(*autofill_driver(), + RendererShouldAcceptPasswordAutofillSuggestion( + ASCIIToUTF16(kAliceUsername))); EXPECT_TRUE(password_autofill_manager()->DidAcceptAutofillSuggestion( username_field(), ASCIIToUTF16(kAliceUsername))); + + EXPECT_CALL(*autofill_driver(), + RendererShouldAcceptPasswordAutofillSuggestion( + ASCIIToUTF16(kInvalidUsername))).Times(0); EXPECT_FALSE(password_autofill_manager()->DidAcceptAutofillSuggestion( username_field(), ASCIIToUTF16(kInvalidUsername))); diff --git a/components/autofill/core/browser/test_autofill_driver.cc b/components/autofill/core/browser/test_autofill_driver.cc index a2cd662..780c01a 100644 --- a/components/autofill/core/browser/test_autofill_driver.cc +++ b/components/autofill/core/browser/test_autofill_driver.cc @@ -53,6 +53,10 @@ void TestAutofillDriver::RendererShouldAcceptDataListSuggestion( const base::string16& value) { } +void TestAutofillDriver::RendererShouldAcceptPasswordAutofillSuggestion( + const base::string16& username) { +} + void TestAutofillDriver::RendererShouldClearFilledForm() { } diff --git a/components/autofill/core/browser/test_autofill_driver.h b/components/autofill/core/browser/test_autofill_driver.h index 630ad81..5ebd4d6 100644 --- a/components/autofill/core/browser/test_autofill_driver.h +++ b/components/autofill/core/browser/test_autofill_driver.h @@ -38,6 +38,8 @@ class TestAutofillDriver : public AutofillDriver, const std::vector<FormStructure*>& forms) OVERRIDE; virtual void RendererShouldAcceptDataListSuggestion( const base::string16& value) OVERRIDE; + virtual void RendererShouldAcceptPasswordAutofillSuggestion( + const base::string16& username) OVERRIDE; virtual void RendererShouldClearFilledForm() OVERRIDE; virtual void RendererShouldClearPreviewedForm() OVERRIDE; virtual void RendererShouldSetNodeText(const base::string16& value) OVERRIDE; |