diff options
5 files changed, 97 insertions, 4 deletions
diff --git a/base/timer/elapsed_timer.h b/base/timer/elapsed_timer.h index 3ea2bfa..1cb9f11 100644 --- a/base/timer/elapsed_timer.h +++ b/base/timer/elapsed_timer.h @@ -15,9 +15,10 @@ namespace base { class BASE_EXPORT ElapsedTimer { public: ElapsedTimer(); + virtual ~ElapsedTimer() {} // Returns the time elapsed since object construction. - TimeDelta Elapsed() const; + virtual TimeDelta Elapsed() const; private: TimeTicks begin_; diff --git a/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc b/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc index 9cbd290..89c4c8a 100644 --- a/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc +++ b/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc @@ -171,10 +171,19 @@ void ManagePasswordsUIController::UnblacklistSite() { void ManagePasswordsUIController::DidNavigateMainFrame( const content::LoadCommittedDetails& details, const content::FrameNavigateParams& params) { + // Don't react to in-page (fragment) navigations. if (details.is_in_page) return; + + // Don't do anything if a navigation occurs before a user could reasonably + // interact with the password bubble. + if (timer_ && timer_->Elapsed() < base::TimeDelta::FromSeconds(1)) + return; + + // Otherwise, reset the password manager and the timer. state_ = password_manager::ui::INACTIVE_STATE; UpdateBubbleAndIconVisibility(); + timer_.reset(new base::ElapsedTimer()); } const autofill::PasswordForm& ManagePasswordsUIController:: diff --git a/chrome/browser/ui/passwords/manage_passwords_ui_controller.h b/chrome/browser/ui/passwords/manage_passwords_ui_controller.h index 7e1b67e..bae6f23 100644 --- a/chrome/browser/ui/passwords/manage_passwords_ui_controller.h +++ b/chrome/browser/ui/passwords/manage_passwords_ui_controller.h @@ -5,7 +5,9 @@ #ifndef CHROME_BROWSER_UI_PASSWORDS_MANAGE_PASSWORDS_UI_CONTROLLER_H_ #define CHROME_BROWSER_UI_PASSWORDS_MANAGE_PASSWORDS_UI_CONTROLLER_H_ +#include "base/gtest_prod_util.h" #include "base/memory/scoped_vector.h" +#include "base/timer/elapsed_timer.h" #include "components/autofill/core/common/password_form.h" #include "components/password_manager/core/browser/password_form_manager.h" #include "components/password_manager/core/browser/password_store.h" @@ -91,6 +93,11 @@ class ManagePasswordsUIController explicit ManagePasswordsUIController( content::WebContents* web_contents); + // content::WebContentsObserver: + virtual void DidNavigateMainFrame( + const content::LoadCommittedDetails& details, + const content::FrameNavigateParams& params) OVERRIDE; + // All previously stored credentials for a specific site. Set by // OnPasswordSubmitted(), OnPasswordAutofilled(), or // OnBlacklistBlockedAutofill(). Protected, not private, so we can mess with @@ -106,6 +113,11 @@ class ManagePasswordsUIController // the value in tests. password_manager::ui::State state_; + // Used to measure the amount of time on a page; if it's less than some + // reasonable limit, then don't close the bubble upon navigation. We create + // (and destroy) the timer in DidNavigateMainFrame. + scoped_ptr<base::ElapsedTimer> timer_; + private: friend class content::WebContentsUserData<ManagePasswordsUIController>; @@ -119,9 +131,6 @@ class ManagePasswordsUIController void UpdateBubbleAndIconVisibility(); // content::WebContentsObserver: - virtual void DidNavigateMainFrame( - const content::LoadCommittedDetails& details, - const content::FrameNavigateParams& params) OVERRIDE; virtual void WebContentsDestroyed() OVERRIDE; // Set by OnPasswordSubmitted() when the user submits a form containing login diff --git a/chrome/browser/ui/passwords/manage_passwords_ui_controller_mock.h b/chrome/browser/ui/passwords/manage_passwords_ui_controller_mock.h index d2d7de0..cdeb5b8 100644 --- a/chrome/browser/ui/passwords/manage_passwords_ui_controller_mock.h +++ b/chrome/browser/ui/passwords/manage_passwords_ui_controller_mock.h @@ -8,6 +8,7 @@ #include "base/basictypes.h" #include "chrome/browser/ui/passwords/manage_passwords_ui_controller.h" #include "components/password_manager/core/common/password_manager_ui.h" +#include "content/public/browser/navigation_details.h" namespace content { class WebContents; @@ -47,9 +48,13 @@ class ManagePasswordsUIControllerMock } void SetState(password_manager::ui::State state) { state_ = state; } + void SetTimer(base::ElapsedTimer* timer) { timer_.reset(timer); } + // True if this controller is installed on |web_contents()|. bool IsInstalled() const; + using ManagePasswordsUIController::DidNavigateMainFrame; + private: bool navigated_to_settings_page_; bool saved_password_; diff --git a/chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc b/chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc index d4bb9e7..08f78fd 100644 --- a/chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc +++ b/chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc @@ -6,6 +6,7 @@ #include "base/prefs/pref_service.h" #include "base/strings/utf_string_conversions.h" #include "base/test/statistics_delta_reader.h" +#include "base/time/time.h" #include "chrome/browser/ui/passwords/manage_passwords_bubble.h" #include "chrome/browser/ui/passwords/manage_passwords_bubble_model.h" #include "chrome/browser/ui/passwords/manage_passwords_icon.h" @@ -23,6 +24,26 @@ #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" +namespace { + +const int64 kSlowNavigationDelayInMS = 2000; +const int64 kQuickNavigationDelayInMS = 500; + +class MockElapsedTimer : public base::ElapsedTimer { + public: + MockElapsedTimer() {} + virtual base::TimeDelta Elapsed() const OVERRIDE { return delta_; } + + void Advance(int64 ms) { delta_ = base::TimeDelta::FromMilliseconds(ms); } + + private: + base::TimeDelta delta_; + + DISALLOW_COPY_AND_ASSIGN(MockElapsedTimer); +}; + +} // namespace + class ManagePasswordsUIControllerTest : public ChromeRenderViewHostTestHarness { public: ManagePasswordsUIControllerTest() {} @@ -99,6 +120,54 @@ TEST_F(ManagePasswordsUIControllerTest, PasswordSubmitted) { EXPECT_EQ(password_manager::ui::PENDING_PASSWORD_STATE, mock.state()); } +TEST_F(ManagePasswordsUIControllerTest, QuickNavigations) { + password_manager::StubPasswordManagerClient client; + password_manager::StubPasswordManagerDriver driver; + password_manager::PasswordFormManager* test_form_manager = + new password_manager::PasswordFormManager( + NULL, &client, &driver, test_form(), false); + controller()->OnPasswordSubmitted(test_form_manager); + ManagePasswordsIconMock mock; + controller()->UpdateIconAndBubbleState(&mock); + EXPECT_EQ(password_manager::ui::PENDING_PASSWORD_STATE, mock.state()); + + // Fake-navigate within a second. We expect the bubble's state to persist + // if a navigation occurs too quickly for a user to reasonably have been + // able to interact with the bubble. This happens on `accounts.google.com`, + // for instance. + scoped_ptr<MockElapsedTimer> timer(new MockElapsedTimer()); + timer->Advance(kQuickNavigationDelayInMS); + controller()->SetTimer(timer.release()); + controller()->DidNavigateMainFrame(content::LoadCommittedDetails(), + content::FrameNavigateParams()); + controller()->UpdateIconAndBubbleState(&mock); + + EXPECT_EQ(password_manager::ui::PENDING_PASSWORD_STATE, mock.state()); +} + +TEST_F(ManagePasswordsUIControllerTest, SlowNavigations) { + password_manager::StubPasswordManagerClient client; + password_manager::StubPasswordManagerDriver driver; + password_manager::PasswordFormManager* test_form_manager = + new password_manager::PasswordFormManager( + NULL, &client, &driver, test_form(), false); + controller()->OnPasswordSubmitted(test_form_manager); + ManagePasswordsIconMock mock; + controller()->UpdateIconAndBubbleState(&mock); + EXPECT_EQ(password_manager::ui::PENDING_PASSWORD_STATE, mock.state()); + + // Fake-navigate after a second. We expect the bubble's state to be reset + // if a navigation occurs after this limit. + scoped_ptr<MockElapsedTimer> timer(new MockElapsedTimer()); + timer->Advance(kSlowNavigationDelayInMS); + controller()->SetTimer(timer.release()); + controller()->DidNavigateMainFrame(content::LoadCommittedDetails(), + content::FrameNavigateParams()); + controller()->UpdateIconAndBubbleState(&mock); + + EXPECT_EQ(password_manager::ui::INACTIVE_STATE, mock.state()); +} + TEST_F(ManagePasswordsUIControllerTest, PasswordSubmittedToNonWebbyURL) { // Navigate to a non-webby URL, then see what happens! content::WebContentsTester::For(web_contents()) |