Allowing PersonalDataManagerFactory to create a service while incognito.

components/autofill/browser/personal_data_manager.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 "components/autofill/browser/personal_data_manager.h"
 
 #include <algorithm>
 #include <functional>
 #include <iterator>
 
@@ skipping 131 matching lines @@
 
 void PersonalDataManager::Init(BrowserContext* browser_context) {
   browser_context_ = browser_context;
   metric_logger_->LogIsAutofillEnabledAtStartup(IsAutofillEnabled());
 
   scoped_refptr<AutofillWebDataService> autofill_data(
       AutofillWebDataService::FromBrowserContext(browser_context_));
 
   // WebDataService may not be available in tests.
   if (!autofill_data.get())
     return;

[Ilya Sherman, 2013/04/25 04:12:52]

If I'm jumping through code correctly, it looks like the AutofillWebDataService
will be NULL in Incognito mode, which means that we still won't have any
Autofill data available with this CL in place.

If you want to implement this change, please carefully go through the code and
make sure that (a) it actually shows Autofill data in Incognito mode, and (b)
the pursuit of (a) doesn't cause us to allow any data to be written to disk in
Incognito mode.

Keep in mind that it's probably not acceptable to have multiple connections to
the backing database, so you'll need to be careful to avoid that case.

[Dan Beam, 2013/04/25 05:30:34]

so, if the first part of this maybe isn't right (as our dialog works for me with
just this patch), do I need to worry about the rest of what you said?

[Dan Beam, 2013/04/25 06:37:11]

so if you're not worried about ^ this, CQ da patch, else I'll worry about it
tomorrow.

 
   LoadProfiles();
   LoadCreditCards();
 
   autofill_data->AddObserver(this);
 }
 
 PersonalDataManager::~PersonalDataManager() {
   CancelPendingQuery(&pending_profiles_query_);
   CancelPendingQuery(&pending_creditcards_query_);
@@ skipping 350 matching lines @@
     (*iter)->GetNonEmptyTypes(app_locale_, non_empty_types);
   }
 }
 
 bool PersonalDataManager::IsDataLoaded() const {
   return is_data_loaded_;
 }
 
 const std::vector<AutofillProfile*>& PersonalDataManager::GetProfiles() {
   if (!components::UserPrefs::Get(browser_context_)->GetBoolean(
           prefs::kAutofillAuxiliaryProfilesEnabled)) {

[Ilya Sherman, 2013/04/25 04:12:52]

Will this do the right thing if browser_context_ is an Incognito window?

[Dan Beam, 2013/04/25 05:30:34]

this seems to be agnostic to whether the profile is OTR or not

     return web_profiles();
   }
 
   profiles_.clear();
 
   // Populates |auxiliary_profiles_|.
   LoadAuxiliaryProfiles();
 
   profiles_.insert(profiles_.end(), web_profiles_.begin(), web_profiles_.end());
   profiles_.insert(profiles_.end(),
@@ skipping 131 matching lines @@
       values->push_back(creditcard_field_value);
       labels->push_back(label);
       icons->push_back(UTF8ToUTF16(credit_card->type()));
       guid_pairs->push_back(GUIDPair(credit_card->guid(), 0));
     }
   }
 }
 
 bool PersonalDataManager::IsAutofillEnabled() const {
   return components::UserPrefs::Get(browser_context_)->GetBoolean(
       prefs::kAutofillEnabled);

[Ilya Sherman, 2013/04/25 04:12:52]

Will this do the right thing if browser_context_ is Incognito?

[Dan Beam, 2013/04/25 05:30:34]

this code seems to agnostic to OTR-ness as well, additionally our dialog doesn't
show when autofill is disabled -- it just errors out and dispatches an event.

 }
 
 // static
 bool PersonalDataManager::IsValidLearnableProfile(
     const AutofillProfile& profile,
     const std::string& app_locale) {
   if (!IsMinimumAddress(profile, app_locale))
     return false;
 
   base::string16 email = profile.GetRawInfo(EMAIL_ADDRESS);
@@ skipping 243 matching lines @@
       NOTREACHED();
       return;
     }
     autofill_data->CancelRequest(*handle);
   }
   *handle = 0;
 }
 
 void PersonalDataManager::SaveImportedProfile(
     const AutofillProfile& imported_profile) {
-  if (browser_context_->IsOffTheRecord()) {
-    // The |IsOffTheRecord| check should happen earlier in the import process,
-    // upon form submission.
-    NOTREACHED();
+  if (browser_context_->IsOffTheRecord())
     return;
-  }
 
   // Don't save a web profile if the data in the profile is a subset of an
   // auxiliary profile.
   for (std::vector<AutofillProfile*>::const_iterator iter =
            auxiliary_profiles_.begin();
        iter != auxiliary_profiles_.end(); ++iter) {
     if (imported_profile.IsSubsetOf(**iter, app_locale_))
       return;
   }
 
   std::vector<AutofillProfile> profiles;
   MergeProfile(imported_profile, web_profiles_.get(), app_locale_, &profiles);
   SetProfiles(&profiles);
 }
 
 
 void PersonalDataManager::SaveImportedCreditCard(
     const CreditCard& imported_card) {
   DCHECK(!imported_card.number().empty());
-  if (browser_context_->IsOffTheRecord()) {
-    // The |IsOffTheRecord| check should happen earlier in the import process,
-    // upon form submission.
-    NOTREACHED();
+  if (browser_context_->IsOffTheRecord())
     return;
-  }
 
   // Set to true if |imported_card| is merged into the credit card list.
   bool merged = false;
 
   std::vector<CreditCard> credit_cards;
   for (std::vector<CreditCard*>::const_iterator card = credit_cards_.begin();
        card != credit_cards_.end();
        ++card) {
     // If |imported_card| has not yet been merged, check whether it should be
     // with the current |card|.
@@ skipping 24 matching lines @@
     const AutofillMetrics* metric_logger) {
   metric_logger_.reset(metric_logger);
 }
 
 void PersonalDataManager::set_browser_context(
     content::BrowserContext* context) {
   browser_context_ = context;
 }
 
 }  // namespace autofill