From c4aa8696d0be2c3f6d45478079db973c5808bbd9 Mon Sep 17 00:00:00 2001 From: "brettw@chromium.org" Date: Wed, 24 Nov 2010 23:08:04 +0000 Subject: Revert 67326 - Re-landing original change since it didn't seem to fix the failure. Revert 67281 to fix failing FullLogin test - The earlier TabContentsWrapper changes had incomplete support for password manager. This wires up Password Manager properly, making sure it observes all notifications that it needs to function. http://crbug.com/63664 TEST=see bug Review URL: http://codereview.chromium.org/5311006 TBR=ben@chromium.org Review URL: http://codereview.chromium.org/5291004 TBR=andybons@chromium.org Review URL: http://codereview.chromium.org/5347003 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@67337 0039d316-1c4b-4281-b951-d872f2087c98 --- .../browser/password_manager/password_manager.cc | 7 ++++++ chrome/browser/password_manager/password_manager.h | 3 +++ chrome/browser/tab_contents/tab_contents.cc | 12 ++++++++++ chrome/browser/tab_contents/tab_contents.h | 4 ++++ .../browser/tab_contents/web_navigation_observer.h | 4 +++- chrome/browser/tab_contents_wrapper.cc | 27 ++++++++++++++++------ chrome/browser/tab_contents_wrapper.h | 6 ++++- 7 files changed, 54 insertions(+), 9 deletions(-) diff --git a/chrome/browser/password_manager/password_manager.cc b/chrome/browser/password_manager/password_manager.cc index 3e7e5fd..b5762b8 100644 --- a/chrome/browser/password_manager/password_manager.cc +++ b/chrome/browser/password_manager/password_manager.cc @@ -151,6 +151,13 @@ void PasswordManager::DidStopLoading() { } } +void PasswordManager::DidNavigateAnyFramePostCommit( + const NavigationController::LoadCommittedDetails& details, + const ViewHostMsg_FrameNavigate_Params& params) { + if (params.password_form.origin.is_valid()) + ProvisionallySavePassword(params.password_form); +} + void PasswordManager::PasswordFormsFound( const std::vector& forms) { if (!delegate_->GetProfileForPasswordManager()) diff --git a/chrome/browser/password_manager/password_manager.h b/chrome/browser/password_manager/password_manager.h index 79ea5c9..0ebb451 100644 --- a/chrome/browser/password_manager/password_manager.h +++ b/chrome/browser/password_manager/password_manager.h @@ -50,6 +50,9 @@ class PasswordManager : public LoginModel, // WebNavigationObserver overrides. virtual void DidStopLoading(); + virtual void DidNavigateAnyFramePostCommit( + const NavigationController::LoadCommittedDetails& details, + const ViewHostMsg_FrameNavigate_Params& params); virtual void PasswordFormsFound( const std::vector& forms); virtual void PasswordFormsVisible( diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc index cfa1ac0..9029bd0 100644 --- a/chrome/browser/tab_contents/tab_contents.cc +++ b/chrome/browser/tab_contents/tab_contents.cc @@ -2790,6 +2790,18 @@ void TabContents::ShowModalHTMLDialog(const GURL& url, int width, int height, } } +void TabContents::PasswordFormsFound( + const std::vector& forms) { + FOR_EACH_OBSERVER(WebNavigationObserver, web_navigation_observers_, + PasswordFormsFound(forms)); +} + +void TabContents::PasswordFormsVisible( + const std::vector& visible_forms) { + FOR_EACH_OBSERVER(WebNavigationObserver, web_navigation_observers_, + PasswordFormsVisible(visible_forms)); +} + // Checks to see if we should generate a keyword based on the OSDD, and if // necessary uses TemplateURLFetcher to download the OSDD and create a keyword. void TabContents::PageHasOSDD( diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h index 2a69a14..c3d6556 100644 --- a/chrome/browser/tab_contents/tab_contents.h +++ b/chrome/browser/tab_contents/tab_contents.h @@ -977,6 +977,10 @@ class TabContents : public PageNavigator, virtual void ShowModalHTMLDialog(const GURL& url, int width, int height, const std::string& json_arguments, IPC::Message* reply_msg); + virtual void PasswordFormsFound( + const std::vector& forms); + virtual void PasswordFormsVisible( + const std::vector& visible_forms); virtual void PageHasOSDD(RenderViewHost* render_view_host, int32 page_id, const GURL& url, diff --git a/chrome/browser/tab_contents/web_navigation_observer.h b/chrome/browser/tab_contents/web_navigation_observer.h index b42e658..40981b9 100644 --- a/chrome/browser/tab_contents/web_navigation_observer.h +++ b/chrome/browser/tab_contents/web_navigation_observer.h @@ -32,7 +32,9 @@ class WebNavigationObserver { virtual void DidStartLoading() { } virtual void DidStopLoading() { } - // TODO(pinkerton): Not sure the best place for these. + // TODO(beng): These should move from here once PasswordManager is able to + // establish its own private communication protocol to the + // renderer. virtual void PasswordFormsFound( const std::vector& forms) { } virtual void PasswordFormsVisible( diff --git a/chrome/browser/tab_contents_wrapper.cc b/chrome/browser/tab_contents_wrapper.cc index d1f0a2fb..a7f52f1 100644 --- a/chrome/browser/tab_contents_wrapper.cc +++ b/chrome/browser/tab_contents_wrapper.cc @@ -9,6 +9,8 @@ #include "chrome/browser/password_manager_delegate_impl.h" #include "chrome/browser/tab_contents/tab_contents.h" +//////////////////////////////////////////////////////////////////////////////// +// TabContentsWrapper, public: TabContentsWrapper::TabContentsWrapper(TabContents* contents) : tab_contents_(contents) { @@ -16,6 +18,9 @@ TabContentsWrapper::TabContentsWrapper(TabContents* contents) // Stash this in the property bag so it can be retrieved without having to // go to a Browser. property_accessor()->SetProperty(contents->property_bag(), this); + + // Needed so that we initialize the password manager on first navigation. + tab_contents()->AddNavigationObserver(this); } TabContentsWrapper::~TabContentsWrapper() { @@ -27,6 +32,15 @@ PropertyAccessor* TabContentsWrapper::property_accessor() { return Singleton< PropertyAccessor >::get(); } +TabContentsWrapper* TabContentsWrapper::Clone() { + TabContents* new_contents = tab_contents()->Clone(); + TabContentsWrapper* new_wrapper = new TabContentsWrapper(new_contents); + // Instantiate the passowrd manager if it has been instantiated here. + if (password_manager_.get()) + new_wrapper->GetPasswordManager(); + return new_wrapper; +} + PasswordManager* TabContentsWrapper::GetPasswordManager() { if (!password_manager_.get()) { // Create the delegate then create the manager. @@ -40,11 +54,10 @@ PasswordManager* TabContentsWrapper::GetPasswordManager() { return password_manager_.get(); } -TabContentsWrapper* TabContentsWrapper::Clone() { - TabContents* new_contents = tab_contents()->Clone(); - TabContentsWrapper* new_wrapper = new TabContentsWrapper(new_contents); - // Instantiate the passowrd manager if it has been instantiated here. - if (password_manager_.get()) - new_wrapper->GetPasswordManager(); - return new_wrapper; +//////////////////////////////////////////////////////////////////////////////// +// TabContentsWrapper, WebNavigationObserver implementation: + +void TabContentsWrapper::NavigateToPendingEntry() { + GetPasswordManager(); + tab_contents()->RemoveNavigationObserver(this); } diff --git a/chrome/browser/tab_contents_wrapper.h b/chrome/browser/tab_contents_wrapper.h index abed5f5..4723da9 100644 --- a/chrome/browser/tab_contents_wrapper.h +++ b/chrome/browser/tab_contents_wrapper.h @@ -8,6 +8,7 @@ #include "base/scoped_ptr.h" #include "chrome/browser/tab_contents/tab_contents.h" +#include "chrome/browser/tab_contents/web_navigation_observer.h" class Extension; class NavigationController; @@ -21,7 +22,7 @@ class TabContentsDelegate; // TODO(pinkerton): Eventually, this class will become TabContents as far as // the browser front-end is concerned, and the current TabContents will be // renamed to something like WebPage or WebView (ben's suggestions). -class TabContentsWrapper { +class TabContentsWrapper : public WebNavigationObserver { public: // Takes ownership of |contents|, which must be heap-allocated (as it lives // in a scoped_ptr) and can not be NULL. @@ -60,6 +61,9 @@ class TabContentsWrapper { // Returns the PasswordManager, creating it if necessary. PasswordManager* GetPasswordManager(); + // WebNavigationObserver overrides: + virtual void NavigateToPendingEntry(); + private: // PasswordManager and its delegate, lazily created. The delegate must // outlive the manager, per documentation in password_manager.h. -- cgit v1.1