diff options
author | blundell@chromium.org <blundell@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-06-18 13:31:36 +0000 |
---|---|---|
committer | blundell@chromium.org <blundell@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-06-18 13:31:36 +0000 |
commit | 1483f9d99443d2d70105885c265fed874cdba12d (patch) | |
tree | a3f90c5bbf50d7fc06157b17a46067ea05c76676 | |
parent | 991bf04e0511b28e1417f528942add9d07ebeaa5 (diff) | |
download | chromium_src-1483f9d99443d2d70105885c265fed874cdba12d.zip chromium_src-1483f9d99443d2d70105885c265fed874cdba12d.tar.gz chromium_src-1483f9d99443d2d70105885c265fed874cdba12d.tar.bz2 |
Eliminate AutofillExternalDelegate being a WebContentsUserData
This CL changes AutofillExternalDelegate from being a WebContentsUserData to
being owned by AutofillDriverImpl as part of making Autofill a layered
component.
BUG=247015
Review URL: https://chromiumcodereview.appspot.com/17225008
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@206967 0039d316-1c4b-4281-b951-d872f2087c98
6 files changed, 56 insertions, 51 deletions
diff --git a/chrome/browser/autofill/autofill_browsertest.cc b/chrome/browser/autofill/autofill_browsertest.cc index ff5bd4a..d2d213f 100644 --- a/chrome/browser/autofill/autofill_browsertest.cc +++ b/chrome/browser/autofill/autofill_browsertest.cc @@ -229,12 +229,13 @@ class AutofillTest : public InProcessBrowserTest { // allows us to forward keyboard events to the popup directly. content::WebContents* web_contents = browser()->tab_strip_model()->GetActiveWebContents(); - AutofillManager* autofill_manager = - AutofillDriverImpl::FromWebContents(web_contents)->autofill_manager(); + AutofillDriverImpl* autofill_driver = + AutofillDriverImpl::FromWebContents(web_contents); + AutofillManager* autofill_manager = autofill_driver->autofill_manager(); if (autofill_manager->IsNativeUiEnabled()) { - external_delegate_.reset( + scoped_ptr<AutofillExternalDelegate> external_delegate( new TestAutofillExternalDelegate(web_contents, autofill_manager)); - autofill_manager->SetExternalDelegate(external_delegate_.get()); + autofill_driver->SetAutofillExternalDelegate(external_delegate.Pass()); } autofill_manager->SetTestDelegate(&test_delegate_); } @@ -472,7 +473,7 @@ class AutofillTest : public InProcessBrowserTest { void SendKeyToPopupAndWait(ui::KeyboardCode key) { // TODO(isherman): Remove this condition once the WebKit popup UI code is // removed. - if (!external_delegate_) { + if (!external_delegate()) { // When testing the WebKit-based UI, route all keys to the page. SendKeyToPageAndWait(key); return; @@ -483,7 +484,7 @@ class AutofillTest : public InProcessBrowserTest { content::NativeWebKeyboardEvent event; event.windowsKeyCode = key; test_delegate_.Reset(); - external_delegate_->keyboard_listener()->HandleKeyPressEvent(event); + external_delegate()->keyboard_listener()->HandleKeyPressEvent(event); test_delegate_.Wait(); } @@ -522,14 +523,18 @@ class AutofillTest : public InProcessBrowserTest { } TestAutofillExternalDelegate* external_delegate() { - return external_delegate_.get(); + content::WebContents* web_contents = + browser()->tab_strip_model()->GetActiveWebContents(); + AutofillDriverImpl* autofill_driver = + AutofillDriverImpl::FromWebContents(web_contents); + return static_cast<TestAutofillExternalDelegate*>( + autofill_driver->autofill_external_delegate()); } AutofillManagerTestDelegateImpl test_delegate_; private: net::TestURLFetcherFactory url_fetcher_factory_; - scoped_ptr<TestAutofillExternalDelegate> external_delegate_; }; // http://crbug.com/150084 diff --git a/chrome/browser/autofill/autofill_interactive_uitest.cc b/chrome/browser/autofill/autofill_interactive_uitest.cc index 242eccc..2199cb5 100644 --- a/chrome/browser/autofill/autofill_interactive_uitest.cc +++ b/chrome/browser/autofill/autofill_interactive_uitest.cc @@ -206,12 +206,13 @@ class AutofillInteractiveTest : public InProcessBrowserTest { // allows us to forward keyboard events to the popup directly. content::WebContents* web_contents = browser()->tab_strip_model()->GetActiveWebContents(); - AutofillManager* autofill_manager = - AutofillDriverImpl::FromWebContents(web_contents)->autofill_manager(); + AutofillDriverImpl* autofill_driver = + AutofillDriverImpl::FromWebContents(web_contents); + AutofillManager* autofill_manager = autofill_driver->autofill_manager(); if (autofill_manager->IsNativeUiEnabled()) { - external_delegate_.reset( + scoped_ptr<AutofillExternalDelegate> external_delegate( new TestAutofillExternalDelegate(web_contents, autofill_manager)); - autofill_manager->SetExternalDelegate(external_delegate_.get()); + autofill_driver->SetAutofillExternalDelegate(external_delegate.Pass()); } autofill_manager->SetTestDelegate(&test_delegate_); } @@ -302,7 +303,7 @@ class AutofillInteractiveTest : public InProcessBrowserTest { void SendKeyToPopupAndWait(ui::KeyboardCode key) { // TODO(isherman): Remove this condition once the WebKit popup UI code is // removed. - if (!external_delegate_) { + if (!external_delegate()) { // When testing the WebKit-based UI, route all keys to the page. SendKeyToPageAndWait(key); return; @@ -313,18 +314,20 @@ class AutofillInteractiveTest : public InProcessBrowserTest { content::NativeWebKeyboardEvent event; event.windowsKeyCode = key; test_delegate_.Reset(); - external_delegate_->keyboard_listener()->HandleKeyPressEvent(event); + external_delegate()->keyboard_listener()->HandleKeyPressEvent(event); test_delegate_.Wait(); } TestAutofillExternalDelegate* external_delegate() { - return external_delegate_.get(); + content::WebContents* web_contents = + browser()->tab_strip_model()->GetActiveWebContents(); + AutofillDriverImpl* autofill_driver = + AutofillDriverImpl::FromWebContents(web_contents); + return static_cast<TestAutofillExternalDelegate*>( + autofill_driver->autofill_external_delegate()); } AutofillManagerTestDelegateImpl test_delegate_; - - private: - scoped_ptr<TestAutofillExternalDelegate> external_delegate_; }; IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, DISABLED_AutofillSelectViaTab) { diff --git a/components/autofill/browser/autofill_external_delegate.cc b/components/autofill/browser/autofill_external_delegate.cc index d5ac726..a2d3cf7 100644 --- a/components/autofill/browser/autofill_external_delegate.cc +++ b/components/autofill/browser/autofill_external_delegate.cc @@ -24,21 +24,8 @@ using content::RenderViewHost; using WebKit::WebAutofillClient; -DEFINE_WEB_CONTENTS_USER_DATA_KEY(autofill::AutofillExternalDelegate); - namespace autofill { -void AutofillExternalDelegate::CreateForWebContentsAndManager( - content::WebContents* web_contents, - AutofillManager* autofill_manager) { - if (FromWebContents(web_contents)) - return; - - web_contents->SetUserData( - UserDataKey(), - new AutofillExternalDelegate(web_contents, autofill_manager)); -} - AutofillExternalDelegate::AutofillExternalDelegate( content::WebContents* web_contents, AutofillManager* autofill_manager) diff --git a/components/autofill/browser/autofill_external_delegate.h b/components/autofill/browser/autofill_external_delegate.h index 1e1acef..cbf5f9d 100644 --- a/components/autofill/browser/autofill_external_delegate.h +++ b/components/autofill/browser/autofill_external_delegate.h @@ -17,7 +17,6 @@ #include "components/autofill/common/password_form_fill_data.h" #include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_registrar.h" -#include "content/public/browser/web_contents_user_data.h" #include "ui/gfx/rect.h" namespace gfx { @@ -25,6 +24,7 @@ class Rect; } namespace content { +class RenderViewHost; class WebContents; } @@ -38,15 +38,14 @@ class AutofillManager; // Delegate for in-browser Autocomplete and Autofill display and selection. class AutofillExternalDelegate - : public content::WebContentsUserData<AutofillExternalDelegate>, - public content::NotificationObserver, + : public content::NotificationObserver, public AutofillPopupDelegate { public: - // Creates an AutofillExternalDelegate and attaches it to the specified - // contents; the second argument is an AutofillManager managing Autofill for - // that WebContents. - static void CreateForWebContentsAndManager(content::WebContents* web_contents, - AutofillManager* autofill_manager); + // Creates an AutofillExternalDelegate for the specified contents; the second + // argument is an AutofillManager managing Autofill for that WebContents. + AutofillExternalDelegate(content::WebContents* web_contents, + AutofillManager* autofill_manager); + virtual ~AutofillExternalDelegate(); // AutofillPopupDelegate implementation. virtual void OnPopupShown(content::KeyboardListener* listener) OVERRIDE; @@ -106,11 +105,6 @@ class AutofillExternalDelegate const PasswordFormFillData& fill_data); protected: - friend class content::WebContentsUserData<AutofillExternalDelegate>; - AutofillExternalDelegate(content::WebContents* web_contents, - AutofillManager* autofill_manager); - virtual ~AutofillExternalDelegate(); - content::WebContents* web_contents() { return web_contents_; } base::WeakPtr<AutofillExternalDelegate> GetWeakPtr(); diff --git a/components/autofill/content/browser/autofill_driver_impl.cc b/components/autofill/content/browser/autofill_driver_impl.cc index 0296352..4b76fb5 100644 --- a/components/autofill/content/browser/autofill_driver_impl.cc +++ b/components/autofill/content/browser/autofill_driver_impl.cc @@ -57,12 +57,8 @@ AutofillDriverImpl::AutofillDriverImpl( : content::WebContentsObserver(web_contents), autofill_manager_(this, delegate, app_locale, enable_download_manager) { if (enable_native_ui) { - // TODO(blundell): Eliminate AutofillExternalDelegate being a WCUD and - // transfer ownership of it to this class. - AutofillExternalDelegate::CreateForWebContentsAndManager( - web_contents, &autofill_manager_); - autofill_manager_.SetExternalDelegate( - AutofillExternalDelegate::FromWebContents(web_contents)); + SetAutofillExternalDelegate(scoped_ptr<AutofillExternalDelegate>( + new AutofillExternalDelegate(web_contents, &autofill_manager_))); } } @@ -84,4 +80,10 @@ void AutofillDriverImpl::DidNavigateMainFrame( autofill_manager_.DidNavigateMainFrame(details, params); } +void AutofillDriverImpl::SetAutofillExternalDelegate( + scoped_ptr<AutofillExternalDelegate> delegate) { + autofill_external_delegate_.reset(delegate.release()); + autofill_manager_.SetExternalDelegate(autofill_external_delegate_.get()); +} + } // namespace autofill diff --git a/components/autofill/content/browser/autofill_driver_impl.h b/components/autofill/content/browser/autofill_driver_impl.h index 9b8a107..ffce5ef 100644 --- a/components/autofill/content/browser/autofill_driver_impl.h +++ b/components/autofill/content/browser/autofill_driver_impl.h @@ -7,8 +7,10 @@ #include <string> +#include "base/memory/scoped_ptr.h" #include "base/supports_user_data.h" #include "components/autofill/browser/autofill_driver.h" +#include "components/autofill/browser/autofill_external_delegate.h" #include "components/autofill/browser/autofill_manager.h" #include "content/public/browser/web_contents_observer.h" @@ -23,7 +25,6 @@ class Message; namespace autofill { class AutofillContext; -class AutofillExternalDelegate; class AutofillManagerDelegate; // Class that drives autofill flow in the browser process based on @@ -44,6 +45,15 @@ class AutofillDriverImpl : public AutofillDriver, // AutofillDriver: virtual content::WebContents* GetWebContents() OVERRIDE; + AutofillExternalDelegate* autofill_external_delegate() { + return autofill_external_delegate_.get(); + } + + // Sets the external delegate to |delegate| both within this class and in the + // shared Autofill code. Takes ownership of |delegate|. + void SetAutofillExternalDelegate( + scoped_ptr<AutofillExternalDelegate> delegate); + AutofillManager* autofill_manager() { return &autofill_manager_; } private: @@ -61,6 +71,10 @@ class AutofillDriverImpl : public AutofillDriver, const content::FrameNavigateParams& params) OVERRIDE; virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE; + // AutofillExternalDelegate instance that this object instantiates in the + // case where the autofill native UI is enabled. + scoped_ptr<AutofillExternalDelegate> autofill_external_delegate_; + // AutofillManager instance via which this object drives the shared Autofill // code. AutofillManager autofill_manager_; |