Only enable password generation if password manager and autofill are both

chrome/browser/autofill/autofill_manager_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <vector>
 
 #include "base/command_line.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
 #include "base/string16.h"
@@ skipping 470 matching lines @@
       MessageLoop::current()->Run();
     } else {
       did_finish_async_form_submit_ = false;
     }
   }
 
   virtual void UploadFormData(const FormStructure& submitted_form) OVERRIDE {
     submitted_form_signature_ = submitted_form.FormSignature();
   }
 
-  virtual void SendPasswordSyncStateToRenderer(
+  virtual void SendPasswordGenerationStateToRenderer(
       content::RenderViewHost* host, bool enabled) OVERRIDE {
     sent_states_.push_back(enabled);
   }
 
   const std::vector<bool>& GetSentStates() {
     return sent_states_;
   }
 
   void ClearSentStates() {
     sent_states_.clear();
@@ skipping 57 matching lines @@
         file_thread_(BrowserThread::FILE) {
   }
 
   virtual ~AutofillManagerTest() {
     // Order of destruction is important as AutofillManager relies on
     // PersonalDataManager to be around when it gets destroyed.
     autofill_manager_ = NULL;
   }
 
   virtual void SetUp() OVERRIDE {
-    Profile* profile = new TestingProfile();
-    browser_context_.reset(profile);
-    PersonalDataManagerFactory::GetInstance()->SetTestingFactory(
-        profile, TestPersonalDataManager::Build);
+    if (!browser_context_.get()) {
+      Profile* profile = new TestingProfile();
+      browser_context_.reset(profile);
+      PersonalDataManagerFactory::GetInstance()->SetTestingFactory(
+          profile, TestPersonalDataManager::Build);
+    }
 
     TabContentsWrapperTestHarness::SetUp();
     autofill_manager_ = new TestAutofillManager(contents_wrapper(),
                                                 &personal_data_);
 
     file_thread_.Start();
   }
 
   virtual void TearDown() OVERRIDE {
     file_thread_.Stop();
     TabContentsWrapperTestHarness::TearDown();
   }
 
-  void UpdatePasswordSyncState(bool new_renderer) {
-    autofill_manager_->UpdatePasswordSyncState(NULL, new_renderer);
+  void GoOffTheRecord() {
+    TearDown();

[Ilya Sherman, 2012/04/24 00:46:54]

Hmm, I'm not sure this is safe.  For example,
chrome_render_view_host_test_harness.cc:49 calls "ash::Shell::DeleteInstance();"
if ash is enabled -- are you sure that's safe to call into multiple times, and
to run test code after calling it the first time?

I think instead of re-using SetUp and TearDown, you'll want to extract just the
parts specific to this test, and call those bits directly.

[Garrett Casto, 2012/04/24 18:49:56]

I'm not sure how this is different from the normal state of affairs. My
understanding is that between tests in a testcase, all that happens is
TearDown/SetUp. This passes the default trybots plus linux_asan, though
admittedly I don't know if any of them have ash enabled.
So it actually turns out that there is a much easier way to do this.
TestingProfile::set_incognito(). I missed it on my first read through :(. So
this is all a moot point.

+    other_browser_context_.reset(new TestingProfile());
+    Profile* incognito =
+        Profile::FromBrowserContext(
+            other_browser_context_.get())->GetOffTheRecordProfile();
+    browser_context_.reset(incognito);
+    SetUp();
+  }
+
+  void UpdatePasswordGenerationState(bool new_renderer) {
+    autofill_manager_->UpdatePasswordGenerationState(NULL, new_renderer);
   }
 
   void GetAutofillSuggestions(int query_id,
                               const webkit::forms::FormData& form,
                               const webkit::forms::FormField& field) {
     autofill_manager_->OnQueryFormFieldAutofill(query_id,
                                                 form,
                                                 field,
                                                 gfx::Rect(),
                                                 false);
@@ skipping 79 matching lines @@
     return autofill_manager_->sync_service_;
   }
 
  protected:
   content::TestBrowserThread ui_thread_;
   content::TestBrowserThread file_thread_;
 
   scoped_refptr<TestAutofillManager> autofill_manager_;
   TestPersonalDataManager personal_data_;
 
+  // Used when we want an off the record profile. This will store the original
+  // profile from which the off the record profile is derived.
+  scoped_ptr<Profile> other_browser_context_;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(AutofillManagerTest);
 };
 
 class TestFormStructure : public FormStructure {
  public:
   explicit TestFormStructure(const FormData& form) : FormStructure(form) {}
   virtual ~TestFormStructure() {}
 
   void SetFieldTypes(const std::vector<AutofillFieldType>& heuristic_types,
@@ skipping 2183 matching lines @@
   autofill_test::CreateTestFormField("", "20", "901", "text", &field);
   types.clear();
   types.insert(UNKNOWN_TYPE);
   form.fields.push_back(field);
   expected_types.push_back(types);
 
   autofill_manager_->set_expected_submitted_field_types(expected_types);
   FormSubmitted(form);
 }
 
-TEST_F(AutofillManagerTest, UpdatePasswordSyncState) {
+TEST_F(AutofillManagerTest, UpdatePasswordGenerationState) {
   // 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(false);
+  UpdatePasswordGenerationState(false);
   EXPECT_EQ(0u, autofill_manager_->GetSentStates().size());
 
   // Now sync passwords.
   preferred_set.Put(syncable::PASSWORDS);
   sync_service->ChangePreferredDataTypes(preferred_set);
-  UpdatePasswordSyncState(false);
+  UpdatePasswordGenerationState(false);
   EXPECT_EQ(1u, autofill_manager_->GetSentStates().size());
   EXPECT_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(false);
+  UpdatePasswordGenerationState(false);
   EXPECT_EQ(0u, autofill_manager_->GetSentStates().size());
 
-  // Disable syncing. This should be sent.

[Ilya Sherman, 2012/04/24 00:46:54]

Hmm, why is this case (disabling sync) removed from the test?

[Garrett Casto, 2012/04/24 18:49:56]

Re-added. Just got lost in the shuffle.

-  sync_service->DisableForUser();
-  UpdatePasswordSyncState(false);
+  // Disable autofill. This should be sent.
+  autofill_manager_->set_autofill_enabled(false);
+  UpdatePasswordGenerationState(false);
   EXPECT_EQ(1u, autofill_manager_->GetSentStates().size());
   EXPECT_FALSE(autofill_manager_->GetSentStates()[0]);
   autofill_manager_->ClearSentStates();
 
+  // Now re-enable autofill, but disable password manager by going incognito.
+  // This should still leave the feature disabled, and shouldn't send anything.

[Ilya Sherman, 2012/04/24 00:46:54]

nit: The "still" in this comment seems a little bit misleading to me, since
you're actually creating a brand new AutofillManager as part of
GoOffTheRecord().

[Garrett Casto, 2012/04/24 18:49:56]

N/A

+  GoOffTheRecord();
+  autofill_manager_->set_autofill_enabled(true);
+  UpdatePasswordGenerationState(false);
+  EXPECT_EQ(0u, autofill_manager_->GetSentStates().size());
+
   // When a new render_view is created, we send the state even if it's the
   // same.
-  UpdatePasswordSyncState(true);
+  UpdatePasswordGenerationState(true);
   EXPECT_EQ(1u, autofill_manager_->GetSentStates().size());
   EXPECT_FALSE(autofill_manager_->GetSentStates()[0]);
   autofill_manager_->ClearSentStates();
 }
 
 namespace {
 
 class MockAutofillExternalDelegate : public TestAutofillExternalDelegate {
  public:
   explicit MockAutofillExternalDelegate(TabContentsWrapper* wrapper,
@@ skipping 52 matching lines @@
 
   AutofillManager* autofill_manager = contents_wrapper()->autofill_manager();
   EXPECT_TRUE(autofill_manager->external_delegate());
 
   AutocompleteHistoryManager* autocomplete_history_manager =
       contents_wrapper()->autocomplete_history_manager();
   EXPECT_TRUE(autocomplete_history_manager->external_delegate());
 }
 
 #endif  // OS_ANDROID