diff options
author | mihaip@chromium.org <mihaip@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-03-30 22:00:51 +0000 |
---|---|---|
committer | mihaip@chromium.org <mihaip@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-03-30 22:00:51 +0000 |
commit | ace62514423ffe9c55eeb3c5b8069e87d1a30cc2 (patch) | |
tree | 548d6ed3b6976c4302138806ea26fc8153d19dea | |
parent | 4699f1faf715bc2714e0d6b0ca8b939b6cefcef7 (diff) | |
download | chromium_src-ace62514423ffe9c55eeb3c5b8069e87d1a30cc2.zip chromium_src-ace62514423ffe9c55eeb3c5b8069e87d1a30cc2.tar.gz chromium_src-ace62514423ffe9c55eeb3c5b8069e87d1a30cc2.tar.bz2 |
Revert 129946 - Enable password generation only if sync for passwords is enabled.
BUG=114092
TEST=Ran corresponding unit and browser tests.
Review URL: https://chromiumcodereview.appspot.com/9834082
TBR=gcasto@chromium.org
Review URL: https://chromiumcodereview.appspot.com/9956041
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@129963 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/browser/autofill/autofill_manager.cc | 42 | ||||
-rw-r--r-- | chrome/browser/autofill/autofill_manager.h | 27 | ||||
-rw-r--r-- | chrome/browser/autofill/autofill_manager_unittest.cc | 61 | ||||
-rw-r--r-- | chrome/common/autofill_messages.h | 5 | ||||
-rw-r--r-- | chrome/renderer/autofill/password_generation_manager.cc | 18 | ||||
-rw-r--r-- | chrome/renderer/autofill/password_generation_manager.h | 6 | ||||
-rw-r--r-- | chrome/renderer/autofill/password_generation_manager_browsertest.cc | 57 |
7 files changed, 25 insertions, 191 deletions
diff --git a/chrome/browser/autofill/autofill_manager.cc b/chrome/browser/autofill/autofill_manager.cc index f1894ac..5b475a1 100644 --- a/chrome/browser/autofill/autofill_manager.cc +++ b/chrome/browser/autofill/autofill_manager.cc @@ -35,8 +35,6 @@ #include "chrome/browser/infobars/infobar_tab_helper.h" #include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/profiles/profile.h" -#include "chrome/browser/sync/profile_sync_service_factory.h" -#include "chrome/browser/sync/profile_sync_service.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_list.h" #include "chrome/browser/ui/browser_window.h" @@ -185,17 +183,13 @@ AutofillManager::AutofillManager(TabContentsWrapper* tab_contents) user_did_type_(false), user_did_autofill_(false), user_did_edit_autofilled_field_(false), - password_sync_enabled_(false), external_delegate_(NULL) { // |personal_data_| is NULL when using test-enabled WebContents. personal_data_ = PersonalDataManagerFactory::GetForProfile( tab_contents->profile()->GetOriginalProfile()); - RegisterWithSyncService(); } AutofillManager::~AutofillManager() { - if (sync_service_ && sync_service_->HasObserver(this)) - sync_service_->RemoveObserver(this); } // static @@ -220,40 +214,6 @@ void AutofillManager::RegisterUserPrefs(PrefService* prefs) { PrefService::UNSYNCABLE_PREF); } -void AutofillManager::RegisterWithSyncService() { - sync_service_ = ProfileSyncServiceFactory::GetForProfile( - tab_contents_wrapper_->profile()); - if (sync_service_) - sync_service_->AddObserver(this); -} - -void AutofillManager::SendPasswordSyncStateToRenderer( - content::RenderViewHost* host, bool enabled) { - host->Send(new AutofillMsg_PasswordSyncEnabled(host->GetRoutingID(), - enabled)); -} - -void AutofillManager::UpdatePasswordSyncState(content::RenderViewHost* host) { - if (!sync_service_) - return; - - syncable::ModelTypeSet sync_set = sync_service_->GetPreferredDataTypes(); - bool new_password_sync_enabled = (sync_service_->HasSyncSetupCompleted() && - sync_set.Has(syncable::PASSWORDS)); - if (new_password_sync_enabled != password_sync_enabled_) { - password_sync_enabled_ = new_password_sync_enabled; - SendPasswordSyncStateToRenderer(host, password_sync_enabled_); - } -} - -void AutofillManager::RenderViewCreated(content::RenderViewHost* host) { - UpdatePasswordSyncState(host); -} - -void AutofillManager::OnStateChanged() { - UpdatePasswordSyncState(web_contents()->GetRenderViewHost()); -} - void AutofillManager::DidNavigateMainFrame( const content::LoadCommittedDetails& details, const content::FrameNavigateParams& params) { @@ -796,10 +756,8 @@ AutofillManager::AutofillManager(TabContentsWrapper* tab_contents, user_did_type_(false), user_did_autofill_(false), user_did_edit_autofilled_field_(false), - password_sync_enabled_(false), external_delegate_(NULL) { DCHECK(tab_contents); - RegisterWithSyncService(); } void AutofillManager::set_metric_logger(const AutofillMetrics* metric_logger) { diff --git a/chrome/browser/autofill/autofill_manager.h b/chrome/browser/autofill/autofill_manager.h index 0872500..b750703 100644 --- a/chrome/browser/autofill/autofill_manager.h +++ b/chrome/browser/autofill/autofill_manager.h @@ -22,7 +22,6 @@ #include "chrome/browser/autofill/autofill_download.h" #include "chrome/browser/autofill/field_types.h" #include "chrome/browser/autofill/form_structure.h" -#include "chrome/browser/sync/profile_sync_service_observer.h" #include "content/public/browser/web_contents_observer.h" class AutofillExternalDelegate; @@ -32,7 +31,6 @@ class AutofillMetrics; class CreditCard; class PersonalDataManager; class PrefService; -class ProfileSyncService; class TabContentsWrapper; struct ViewHostMsg_FrameNavigate_Params; @@ -60,7 +58,6 @@ struct FormField; // forms. class AutofillManager : public content::WebContentsObserver, public AutofillDownloadManager::Observer, - public ProfileSyncServiceObserver, public base::RefCounted<AutofillManager> { public: explicit AutofillManager(TabContentsWrapper* tab_contents); @@ -110,11 +107,6 @@ class AutofillManager : public content::WebContentsObserver, // Reset cache. void Reset(); - // Informs the renderers of the current password sync state for use in - // password generation. This is a separate function to aid with testing. - virtual void SendPasswordSyncStateToRenderer(content::RenderViewHost* host, - bool enabled); - // Logs quality metrics for the |submitted_form| and uploads the form data // to the crowdsourcing server, if appropriate. virtual void UploadFormDataAsyncCallback( @@ -151,7 +143,6 @@ class AutofillManager : public content::WebContentsObserver, private: // content::WebContentsObserver: - virtual void RenderViewCreated(content::RenderViewHost* host) OVERRIDE; virtual void DidNavigateMainFrame( const content::LoadCommittedDetails& details, const content::FrameNavigateParams& params) OVERRIDE; @@ -161,17 +152,6 @@ class AutofillManager : public content::WebContentsObserver, virtual void OnLoadedServerPredictions( const std::string& response_xml) OVERRIDE; - // ProfileSyncServiceObserver: - virtual void OnStateChanged() OVERRIDE; - - // Register as an observer with the sync service. - void RegisterWithSyncService(); - - // Determines what the current state of password sync is, and if it has - // changed from password_sync_enabled_. If it has changed, it notifies - // the renderers of this change via SendPasswordSyncStateToRenderer. - void UpdatePasswordSyncState(content::RenderViewHost* host); - void OnFormsSeen(const std::vector<webkit::forms::FormData>& forms, const base::TimeTicks& timestamp); void OnTextFieldDidChange(const webkit::forms::FormData& form, @@ -323,13 +303,6 @@ class AutofillManager : public content::WebContentsObserver, // When the user first interacted with a potentially fillable form on this // page. base::TimeTicks initial_interaction_timestamp_; - // If password sync is enabled. We cache this value so that we don't - // spam the renderer with messages during startup when the sync state - // is changing rapidly. - bool password_sync_enabled_; - // The ProfileSyncService associated with this tab. This may be NULL in - // testing. - ProfileSyncService* sync_service_; // Our copy of the form data. ScopedVector<FormStructure> form_structures_; diff --git a/chrome/browser/autofill/autofill_manager_unittest.cc b/chrome/browser/autofill/autofill_manager_unittest.cc index 8695d44..a50cff2 100644 --- a/chrome/browser/autofill/autofill_manager_unittest.cc +++ b/chrome/browser/autofill/autofill_manager_unittest.cc @@ -23,7 +23,6 @@ #include "chrome/browser/autofill/test_autofill_external_delegate.h" #include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/profiles/profile.h" -#include "chrome/browser/sync/profile_sync_service.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h" #include "chrome/browser/ui/tab_contents/test_tab_contents_wrapper.h" @@ -487,19 +486,6 @@ class TestAutofillManager : public AutofillManager { submitted_form_signature_ = submitted_form.FormSignature(); } - virtual void SendPasswordSyncStateToRenderer( - content::RenderViewHost* host, bool enabled) OVERRIDE { - sent_states_.push_back(enabled); - } - - const std::vector<bool>& GetSentStates() { - return sent_states_; - } - - void ClearSentStates() { - sent_states_.clear(); - } - const std::string GetSubmittedFormSignature() { return submitted_form_signature_; } @@ -541,7 +527,6 @@ class TestAutofillManager : public AutofillManager { std::string submitted_form_signature_; std::vector<FieldTypeSet> expected_submitted_field_types_; - std::vector<bool> sent_states_; DISALLOW_COPY_AND_ASSIGN(TestAutofillManager); }; @@ -582,10 +567,6 @@ class AutofillManagerTest : public TabContentsWrapperTestHarness { TabContentsWrapperTestHarness::TearDown(); } - void UpdatePasswordSyncState() { - autofill_manager_->UpdatePasswordSyncState(NULL); - } - void GetAutofillSuggestions(int query_id, const webkit::forms::FormData& form, const webkit::forms::FormField& field) { @@ -672,10 +653,6 @@ class AutofillManagerTest : public TabContentsWrapperTestHarness { return true; } - ProfileSyncService* GetProfileSyncService() { - return autofill_manager_->sync_service_; - } - protected: content::TestBrowserThread ui_thread_; content::TestBrowserThread file_thread_; @@ -2885,44 +2862,6 @@ TEST_F(AutofillManagerTest, DeterminePossibleFieldTypesForUpload) { FormSubmitted(form); } -TEST_F(AutofillManagerTest, UpdatePasswordSyncState) { - // Allow this test to control what should get synced. - profile()->GetPrefs()->SetBoolean(prefs::kSyncKeepEverythingSynced, false); - - // Sync some things, but not passwords. Shouldn't send anything since - // password generation is disabled by default. - ProfileSyncService* sync_service = GetProfileSyncService(); - sync_service->SetSyncSetupCompleted(); - syncable::ModelTypeSet preferred_set; - preferred_set.Put(syncable::EXTENSIONS); - preferred_set.Put(syncable::PREFERENCES); - sync_service->ChangePreferredDataTypes(preferred_set); - syncable::ModelTypeSet new_set = sync_service->GetPreferredDataTypes(); - UpdatePasswordSyncState(); - EXPECT_EQ(0u, autofill_manager_->GetSentStates().size()); - - // Now sync passwords. - preferred_set.Put(syncable::PASSWORDS); - sync_service->ChangePreferredDataTypes(preferred_set); - UpdatePasswordSyncState(); - EXPECT_EQ(1u, autofill_manager_->GetSentStates().size()); - EXPECT_EQ(true, autofill_manager_->GetSentStates()[0]); - autofill_manager_->ClearSentStates(); - - // Add some additional synced state. Nothing should be sent. - preferred_set.Put(syncable::THEMES); - sync_service->ChangePreferredDataTypes(preferred_set); - UpdatePasswordSyncState(); - EXPECT_EQ(0u, autofill_manager_->GetSentStates().size()); - - // Disable syncing. This should be sent. - sync_service->DisableForUser(); - UpdatePasswordSyncState(); - EXPECT_EQ(1u, autofill_manager_->GetSentStates().size()); - EXPECT_EQ(false, autofill_manager_->GetSentStates()[0]); - autofill_manager_->ClearSentStates(); -} - namespace { class MockAutofillExternalDelegate : public TestAutofillExternalDelegate { diff --git a/chrome/common/autofill_messages.h b/chrome/common/autofill_messages.h index 473271a..b915ed2 100644 --- a/chrome/common/autofill_messages.h +++ b/chrome/common/autofill_messages.h @@ -116,11 +116,6 @@ IPC_MESSAGE_ROUTED1(AutofillMsg_SetNodeText, IPC_MESSAGE_ROUTED1(AutofillMsg_GeneratedPasswordAccepted, string16 /* generated_password */) -// Sends the state of password sync to the renderer. Used to determine if -// password generation should be enabled. -IPC_MESSAGE_ROUTED1(AutofillMsg_PasswordSyncEnabled, - bool /* is_enabled */) - // Autofill messages sent from the renderer to the browser. // Notification that forms have been seen that are candidates for diff --git a/chrome/renderer/autofill/password_generation_manager.cc b/chrome/renderer/autofill/password_generation_manager.cc index 8f47947..a5b7dc3 100644 --- a/chrome/renderer/autofill/password_generation_manager.cc +++ b/chrome/renderer/autofill/password_generation_manager.cc @@ -19,15 +19,10 @@ namespace autofill { PasswordGenerationManager::PasswordGenerationManager( content::RenderView* render_view) - : content::RenderViewObserver(render_view), - sync_enabled_(false) {} + : content::RenderViewObserver(render_view) {} PasswordGenerationManager::~PasswordGenerationManager() {} void PasswordGenerationManager::DidFinishDocumentLoad(WebKit::WebFrame* frame) { - // We don't want to generate passwords if password sync isn't enabled. - if (!sync_enabled_) - return; - if (!ShouldAnalyzeFrame(*frame)) return; @@ -68,7 +63,7 @@ bool PasswordGenerationManager::ShouldAnalyzeFrame( DVLOG(1) << "No PasswordManager access"; return false; } - + // TODO(gcasto): Query the browser to see if password sync is enabled. return true; } @@ -76,8 +71,7 @@ void PasswordGenerationManager::FocusedNodeChanged( const WebKit::WebNode& node) { WebKit::WebInputElement input_element = node.toConst<WebKit::WebInputElement>(); - if (!input_element.isNull() && - account_creation_elements_.first == input_element) { + if (account_creation_elements_.first == input_element) { gfx::Rect rect(input_element.boundsInViewportSpace()); Send(new AutofillHostMsg_ShowPasswordGenerationPopup(routing_id(), rect)); @@ -89,8 +83,6 @@ bool PasswordGenerationManager::OnMessageReceived(const IPC::Message& message) { IPC_BEGIN_MESSAGE_MAP(PasswordGenerationManager, message) IPC_MESSAGE_HANDLER(AutofillMsg_GeneratedPasswordAccepted, OnPasswordAccepted) - IPC_MESSAGE_HANDLER(AutofillMsg_PasswordSyncEnabled, - OnPasswordSyncEnabled) IPC_MESSAGE_UNHANDLED(handled = false) IPC_END_MESSAGE_MAP() return handled; @@ -105,8 +97,4 @@ void PasswordGenerationManager::OnPasswordAccepted(const string16& password) { } } -void PasswordGenerationManager::OnPasswordSyncEnabled(bool enabled) { - sync_enabled_ = enabled; -} - } // namespace autofill diff --git a/chrome/renderer/autofill/password_generation_manager.h b/chrome/renderer/autofill/password_generation_manager.h index a8c4bd8..a93eedd 100644 --- a/chrome/renderer/autofill/password_generation_manager.h +++ b/chrome/renderer/autofill/password_generation_manager.h @@ -38,13 +38,7 @@ class PasswordGenerationManager : public content::RenderViewObserver { virtual void DidFinishDocumentLoad(WebKit::WebFrame* frame) OVERRIDE; virtual void FocusedNodeChanged(const WebKit::WebNode& node) OVERRIDE; - // Message handlers. void OnPasswordAccepted(const string16& password); - void OnPasswordSyncEnabled(bool enabled); - - // True if password sync is enabled for the profile associated with this - // renderer. - bool sync_enabled_; std::pair<WebKit::WebInputElement, std::vector<WebKit::WebInputElement> > account_creation_elements_; diff --git a/chrome/renderer/autofill/password_generation_manager_browsertest.cc b/chrome/renderer/autofill/password_generation_manager_browsertest.cc index 1119bd4..72e612b 100644 --- a/chrome/renderer/autofill/password_generation_manager_browsertest.cc +++ b/chrome/renderer/autofill/password_generation_manager_browsertest.cc @@ -72,22 +72,14 @@ class PasswordGenerationManagerTest : public ChromeRenderViewTest { DISALLOW_COPY_AND_ASSIGN(PasswordGenerationManagerTest); }; -const char kSigninFormHTML[] = - "<FORM name = 'blah' action = 'www.random.com/'> " - " <INPUT type = 'text' id = 'username'/> " - " <INPUT type = 'password' id = 'password'/> " - " <INPUT type = 'submit' value = 'LOGIN' />" - "</FORM>"; - -const char kAccountCreationFormHTML[] = - "<FORM name = 'blah' action = 'www.random.com/'> " - " <INPUT type = 'text' id = 'username'/> " - " <INPUT type = 'password' id = 'first_password'/> " - " <INPUT type = 'password' id = 'second_password'/> " - " <INPUT type = 'submit' value = 'LOGIN' />" - "</FORM>"; TEST_F(PasswordGenerationManagerTest, DetectionTest) { + const char kSigninFormHTML[] = + "<FORM name = 'blah' action = 'www.random.com/'> " + " <INPUT type = 'text' id = 'username'/> " + " <INPUT type = 'password' id = 'password'/> " + " <INPUT type = 'submit' value = 'LOGIN' />" + "</FORM>"; LoadHTML(kSigninFormHTML); WebDocument document = GetMainFrame()->document(); @@ -97,9 +89,16 @@ TEST_F(PasswordGenerationManagerTest, DetectionTest) { WebInputElement password_element = element.to<WebInputElement>(); EXPECT_EQ(0u, generation_manager_->messages().size()); + const char kAccountCreationFormHTML[] = + "<FORM name = 'blah' action = 'www.random.com/'> " + " <INPUT type = 'text' id = 'username'/> " + " <INPUT type = 'password' id = 'first_password'/> " + " <INPUT type = 'password' id = 'second_password'/> " + " <INPUT type = 'submit' value = 'LOGIN' />" + "</FORM>"; LoadHTML(kAccountCreationFormHTML); - // We don't do anything yet because the feature is disabled. + // We don't autofill at first. document = GetMainFrame()->document(); element = document.getElementById(WebString::fromUTF8("first_password")); ASSERT_FALSE(element.isNull()); @@ -109,24 +108,8 @@ TEST_F(PasswordGenerationManagerTest, DetectionTest) { ASSERT_FALSE(element.isNull()); WebInputElement second_password_element = element.to<WebInputElement>(); EXPECT_EQ(0u, generation_manager_->messages().size()); - SetFocused(first_password_element.to<WebNode>()); - EXPECT_EQ(0u, generation_manager_->messages().size()); - - // Pretend like sync was enabled. - AutofillMsg_PasswordSyncEnabled msg(0, true); - generation_manager_->OnMessageReceived(msg); - // Now we will send a message once the first password feld is focused. - LoadHTML(kAccountCreationFormHTML); - document = GetMainFrame()->document(); - element = document.getElementById(WebString::fromUTF8("first_password")); - ASSERT_FALSE(element.isNull()); - first_password_element = element.to<WebInputElement>(); - EXPECT_EQ(0u, generation_manager_->messages().size()); - element = document.getElementById(WebString::fromUTF8("second_password")); - ASSERT_FALSE(element.isNull()); - second_password_element = element.to<WebInputElement>(); - EXPECT_EQ(0u, generation_manager_->messages().size()); + // After first element is focused, then we autofill. SetFocused(first_password_element.to<WebNode>()); EXPECT_EQ(1u, generation_manager_->messages().size()); EXPECT_EQ(AutofillHostMsg_ShowPasswordGenerationPopup::ID, @@ -134,9 +117,13 @@ TEST_F(PasswordGenerationManagerTest, DetectionTest) { } TEST_F(PasswordGenerationManagerTest, FillTest) { - // Make sure that we are enabled before loading HTML. - AutofillMsg_PasswordSyncEnabled enabled_msg(0, true); - generation_manager_->OnMessageReceived(enabled_msg); + const char kAccountCreationFormHTML[] = + "<FORM name = 'blah' action = 'www.random.com/'> " + " <INPUT type = 'text' id = 'username'/> " + " <INPUT type = 'password' id = 'first_password'/> " + " <INPUT type = 'password' id = 'second_password'/> " + " <INPUT type = 'submit' value = 'LOGIN' />" + "</FORM>"; LoadHTML(kAccountCreationFormHTML); WebDocument document = GetMainFrame()->document(); |