diff options
| -rw-r--r-- | chrome/browser/autofill/autofill_dialog.h | 22 | ||||
| -rw-r--r-- | chrome/browser/autofill/autofill_dialog_controller_mac.h | 37 | ||||
| -rw-r--r-- | chrome/browser/autofill/autofill_dialog_controller_mac.mm | 164 | ||||
| -rw-r--r-- | chrome/browser/autofill/autofill_dialog_controller_mac_unittest.mm | 278 | ||||
| -rw-r--r-- | chrome/browser/autofill/autofill_dialog_gtk.cc | 9 | ||||
| -rw-r--r-- | chrome/browser/autofill/autofill_dialog_mac.mm | 16 | ||||
| -rw-r--r-- | chrome/browser/autofill/autofill_manager.cc | 47 | ||||
| -rw-r--r-- | chrome/browser/autofill/autofill_manager.h | 4 | ||||
| -rw-r--r-- | chrome/browser/autofill/personal_data_manager.cc | 16 | ||||
| -rw-r--r-- | chrome/browser/autofill/personal_data_manager.h | 18 | ||||
| -rw-r--r-- | chrome/browser/cocoa/browser_test_helper.h | 4 | ||||
| -rw-r--r-- | chrome/browser/cocoa/preferences_window_controller.h | 4 | ||||
| -rw-r--r-- | chrome/browser/cocoa/preferences_window_controller.mm | 109 | ||||
| -rw-r--r-- | chrome/browser/gtk/options/content_page_gtk.cc | 2 | ||||
| -rw-r--r-- | chrome/browser/views/autofill_profiles_view_win.cc | 9 |
15 files changed, 280 insertions, 459 deletions
diff --git a/chrome/browser/autofill/autofill_dialog.h b/chrome/browser/autofill/autofill_dialog.h index 2f02443..2dd3855 100644 --- a/chrome/browser/autofill/autofill_dialog.h +++ b/chrome/browser/autofill/autofill_dialog.h @@ -33,21 +33,19 @@ class AutoFillDialogObserver { // The dialog fills out the profile fields using this data. |observer| will be // notified by OnAutoFillDialogAccept when the user has applied changes. // -// The |parent| parameter (currently only used on Windows) specifies the parent -// view in the view hierarchy. May be NULL on Mac and gtk. -// -// Optional parameters |imported_profile| and |imported_credit_card| may be -// supplied. If they are supplied (non-NULL) they will be used instead of -// the profile and credit card data retrieved from the PersonalDataManager -// associated with the |profile|. -// // The PersonalDataManager owns the contents of these vectors. The lifetime of // the contents is until the PersonalDataManager replaces them with new data // whenever the web database is updated. -void ShowAutoFillDialog(gfx::NativeView parent, +#if defined(OS_MACOSX) +// TODO(dhollowa): update .mm files and remove this. +void ShowAutoFillDialog(AutoFillDialogObserver* observer, + const std::vector<AutoFillProfile*>& profiles, + const std::vector<CreditCard*>& credit_cards, + Profile* profile); +#else +void ShowAutoFillDialog(gfx::NativeWindow parent, AutoFillDialogObserver* observer, - Profile* profile, - AutoFillProfile* imported_profile, - CreditCard* imported_credit_card); + Profile* profile); +#endif #endif // CHROME_BROWSER_AUTOFILL_AUTOFILL_DIALOG_H_ diff --git a/chrome/browser/autofill/autofill_dialog_controller_mac.h b/chrome/browser/autofill/autofill_dialog_controller_mac.h index 220634b..1d90836e 100644 --- a/chrome/browser/autofill/autofill_dialog_controller_mac.h +++ b/chrome/browser/autofill/autofill_dialog_controller_mac.h @@ -8,15 +8,10 @@ #import <Cocoa/Cocoa.h> #include <vector> #include "base/scoped_nsobject.h" -#include "base/scoped_ptr.h" #include "chrome/browser/autofill/autofill_dialog.h" #include "chrome/browser/autofill/autofill_profile.h" #include "chrome/browser/autofill/credit_card.h" -namespace AutoFillDialogControllerInternal { -class PersonalDataManagerObserver; -} // AutoFillDialogControllerInternal - @class AutoFillAddressViewController; @class AutoFillCreditCardViewController; class Profile; @@ -47,17 +42,11 @@ class Profile; scoped_nsobject<NSString> defaultCreditCardLabel_; AutoFillDialogObserver* observer_; // Weak, not retained. - Profile* profile_; // Weak, not retained. - AutoFillProfile* importedProfile_; // Weak, not retained. - CreditCard* importedCreditCard_; // Weak, not retained. std::vector<AutoFillProfile> profiles_; std::vector<CreditCard> creditCards_; + Profile* profile_; // Weak, not retained. BOOL auxiliaryEnabled_; scoped_nsobject<WindowSizeAutosaver> sizeSaver_; - - // Manages PersonalDataManager loading. - scoped_ptr<AutoFillDialogControllerInternal::PersonalDataManagerObserver> - personalDataManagerObserver_; } // Property representing state of Address Book "me" card usage. Checkbox is @@ -75,15 +64,13 @@ class Profile; // call to |save|. If |observer| is non-NULL then its |OnAutoFillDialogApply| // method is invoked during |save| with the new address and credit card // information. +// |profiles| and |creditCards| must have non-NULL entries (zero or more). +// These provide the initial data that is presented to the user. // |profile| must be non-NULL. -// AutoFill profile and credit card data is initialized from the -// |PersonalDataManager| that is associated with the input |profile|. -// If |importedProfile| or |importedCreditCard| parameters are supplied then -// the |PersonalDataManager| data is ignored. Both may be NULL. + (void)showAutoFillDialogWithObserver:(AutoFillDialogObserver*)observer - profile:(Profile*)profile - importedProfile:(AutoFillProfile*)importedProfile - importedCreditCard:(CreditCard*)importedCreditCard; + autoFillProfiles:(const std::vector<AutoFillProfile*>&)profiles + creditCards:(const std::vector<CreditCard*>&)creditCards + profile:(Profile*)profile; // IBActions for the dialog buttons. - (IBAction)save:(id)sender; @@ -118,14 +105,14 @@ class Profile; // Note: controller is autoreleased when |-closeDialog| is called. + (AutoFillDialogController*)controllerWithObserver: (AutoFillDialogObserver*)observer - profile:(Profile*)profile - importedProfile:(AutoFillProfile*)importedProfile - importedCreditCard:(CreditCard*)importedCreditCard; + autoFillProfiles:(const std::vector<AutoFillProfile*>&)profiles + creditCards:(const std::vector<CreditCard*>&)creditCards + profile:(Profile*)profile; - (id)initWithObserver:(AutoFillDialogObserver*)observer - profile:(Profile*)profile - importedProfile:(AutoFillProfile*)importedProfile - importedCreditCard:(CreditCard*)importedCreditCard; + autoFillProfiles:(const std::vector<AutoFillProfile*>&)profiles + creditCards:(const std::vector<CreditCard*>&)creditCards + profile:(Profile*)profile; - (NSMutableArray*)addressFormViewControllers; - (NSMutableArray*)creditCardFormViewControllers; - (void)closeDialog; diff --git a/chrome/browser/autofill/autofill_dialog_controller_mac.mm b/chrome/browser/autofill/autofill_dialog_controller_mac.mm index 2e3a180..2618044 100644 --- a/chrome/browser/autofill/autofill_dialog_controller_mac.mm +++ b/chrome/browser/autofill/autofill_dialog_controller_mac.mm @@ -10,7 +10,6 @@ #import "chrome/browser/autofill/autofill_address_view_controller_mac.h" #import "chrome/browser/autofill/autofill_credit_card_model_mac.h" #import "chrome/browser/autofill/autofill_credit_card_view_controller_mac.h" -#import "chrome/browser/autofill/personal_data_manager.h" #include "chrome/browser/browser_process.h" #import "chrome/browser/cocoa/disclosure_view_controller.h" #import "chrome/browser/cocoa/section_separator_view.h" @@ -20,80 +19,6 @@ #include "chrome/common/pref_names.h" #include "grit/generated_resources.h" -// Private interface. -@interface AutoFillDialogController (PrivateAPI) -// Asyncronous handler for when PersonalDataManager data loads. The -// personal data manager notifies the dialog with this method when the -// data loading is complete and ready to be used. -- (void)onPersonalDataLoaded:(const std::vector<AutoFillProfile*>&)profiles - creditCards:(const std::vector<CreditCard*>&)creditCards; -@end - -namespace AutoFillDialogControllerInternal { - -// PersonalDataManagerObserver facilitates asynchronous loading of -// PersonalDataManager data before showing the AutoFill settings data to the -// user. It acts as a C++-based delegate for the |AutoFillDialogController|. -class PersonalDataManagerObserver : public PersonalDataManager::Observer { - public: - explicit PersonalDataManagerObserver( - AutoFillDialogController* controller, - PersonalDataManager* personal_data_manager, - Profile* profile) - : controller_(controller), - personal_data_manager_(personal_data_manager), - profile_(profile) { - } - - virtual ~PersonalDataManagerObserver(); - - // Notifies the observer that the PersonalDataManager has finished loading. - virtual void OnPersonalDataLoaded(); - - private: - // Utility method to remove |this| from |personal_data_manager_| as an - // observer. - void RemoveObserver(); - - // The dialog controller to be notified when the data loading completes. - // Weak reference. - AutoFillDialogController* controller_; - - // The object in which we are registered as an observer. We hold on to - // it to facilitate un-registering ourself in the destructor and in the - // |OnPersonalDataLoaded| method. This may be NULL. - // Weak reference. - PersonalDataManager* personal_data_manager_; - - // Profile of caller. Held as weak reference. May not be NULL. - Profile* profile_; - - private: - DISALLOW_COPY_AND_ASSIGN(PersonalDataManagerObserver); -}; - -// During destruction ensure that we are removed from the -// |personal_data_manager_| as an observer. -PersonalDataManagerObserver::~PersonalDataManagerObserver() { - RemoveObserver(); -} - -void PersonalDataManagerObserver::RemoveObserver() { - if (personal_data_manager_) { - personal_data_manager_->RemoveObserver(this); - } -} - -// The data is ready so display our data. Notify the dialog controller that -// the data is ready. Once done we clear the observer. -void PersonalDataManagerObserver::OnPersonalDataLoaded() { - RemoveObserver(); - [controller_ onPersonalDataLoaded:personal_data_manager_->web_profiles() - creditCards:personal_data_manager_->credit_cards()]; -} - -} // namespace AutoFillDialogControllerInternal - @interface AutoFillDialogController (PrivateMethods) - (void)runModalDialog; - (void)installChildViews; @@ -104,14 +29,14 @@ void PersonalDataManagerObserver::OnPersonalDataLoaded() { @synthesize auxiliaryEnabled = auxiliaryEnabled_; + (void)showAutoFillDialogWithObserver:(AutoFillDialogObserver*)observer - profile:(Profile*)profile - importedProfile:(AutoFillProfile*) importedProfile - importedCreditCard:(CreditCard*) importedCreditCard { + autoFillProfiles:(const std::vector<AutoFillProfile*>&)profiles + creditCards:(const std::vector<CreditCard*>&)creditCards + profile:(Profile*)profile { AutoFillDialogController* controller = [AutoFillDialogController controllerWithObserver:observer - profile:profile - importedProfile:importedProfile - importedCreditCard:importedCreditCard]; + autoFillProfiles:profiles + creditCards:creditCards + profile:profile]; // Only run modal dialog if it is not already being shown. if (![controller isWindowLoaded]) { @@ -121,23 +46,7 @@ void PersonalDataManagerObserver::OnPersonalDataLoaded() { - (void)awakeFromNib { [addressSectionBox_ setShowTopLine:FALSE]; - - PersonalDataManager* personal_data_manager = - profile_->GetPersonalDataManager(); - DCHECK(personal_data_manager); - - if (personal_data_manager->IsDataLoaded()) { - // |personalDataManager| data is loaded, we can proceed with the contents. - [self onPersonalDataLoaded:personal_data_manager->web_profiles() - creditCards:personal_data_manager->credit_cards()]; - } else { - // |personalDataManager| data is NOT loaded, so we load it here, installing - // our observer. - personalDataManagerObserver_.reset( - new AutoFillDialogControllerInternal::PersonalDataManagerObserver( - self, personal_data_manager, profile_)); - personal_data_manager->SetObserver(personalDataManagerObserver_.get()); - } + [self installChildViews]; } // NSWindow Delegate callback. When the window closes the controller can @@ -456,17 +365,17 @@ void PersonalDataManagerObserver::OnPersonalDataLoaded() { @implementation AutoFillDialogController (ExposedForUnitTests) + (AutoFillDialogController*)controllerWithObserver: - (AutoFillDialogObserver*)observer - profile:(Profile*)profile - importedProfile:(AutoFillProfile*)importedProfile - importedCreditCard:(CreditCard*)importedCreditCard { + (AutoFillDialogObserver*)observer + autoFillProfiles:(const std::vector<AutoFillProfile*>&)profiles + creditCards:(const std::vector<CreditCard*>&)creditCards + profile:(Profile*)profile { // Deallocation is done upon window close. See |windowWillClose:|. AutoFillDialogController* controller = [[self alloc] initWithObserver:observer - profile:profile - importedProfile:importedProfile - importedCreditCard:importedCreditCard]; + autoFillProfiles:profiles + creditCards:creditCards + profile:profile]; return controller; } @@ -475,9 +384,9 @@ void PersonalDataManagerObserver::OnPersonalDataLoaded() { // |profiles| are non-retained immutable list of autofill profiles. // |creditCards| are non-retained immutable list of credit card info. - (id)initWithObserver:(AutoFillDialogObserver*)observer - profile:(Profile*)profile - importedProfile:(AutoFillProfile*)importedProfile - importedCreditCard:(CreditCard*)importedCreditCard { + autoFillProfiles:(const std::vector<AutoFillProfile*>&)profiles + creditCards:(const std::vector<CreditCard*>&)creditCards + profile:(Profile*)profile { CHECK(profile); // Use initWithWindowNibPath: instead of initWithWindowNibName: so we // can override it in a unit test. @@ -486,9 +395,18 @@ void PersonalDataManagerObserver::OnPersonalDataLoaded() { ofType:@"nib"]; if ((self = [super initWithWindowNibPath:nibpath owner:self])) { observer_ = observer; + + // Make local copy of |profiles|. + std::vector<AutoFillProfile*>::const_iterator i; + for (i = profiles.begin(); i != profiles.end(); ++i) + profiles_.push_back(**i); + + // Make local copy of |creditCards|. + std::vector<CreditCard*>::const_iterator j; + for (j = creditCards.begin(); j != creditCards.end(); ++j) + creditCards_.push_back(**j); + profile_ = profile; - importedProfile_ = importedProfile; - importedCreditCard_ = importedCreditCard; // Use property here to trigger KVO binding. [self setAuxiliaryEnabled:profile_->GetPrefs()->GetBoolean( @@ -602,30 +520,4 @@ void PersonalDataManagerObserver::OnPersonalDataLoaded() { [self didChangeValueForKey:@"defaultCreditCardLabel"]; } -- (void)onPersonalDataLoaded:(const std::vector<AutoFillProfile*>&)profiles - creditCards:(const std::vector<CreditCard*>&)creditCards { - if (importedProfile_) { - profiles_.push_back(*importedProfile_); - } - - if (importedCreditCard_) { - creditCards_.push_back(*importedCreditCard_); - } - - // If we're not using imported data then use the data fetch from the web db. - if (!importedProfile_ && !importedCreditCard_) { - // Make local copy of |profiles|. - for (std::vector<AutoFillProfile*>::const_iterator iter = profiles.begin(); - iter != profiles.end(); ++iter) - profiles_.push_back(**iter); - - // Make local copy of |creditCards|. - for (std::vector<CreditCard*>::const_iterator iter = creditCards.begin(); - iter != creditCards.end(); ++iter) - creditCards_.push_back(**iter); - } - - [self installChildViews]; -} - @end diff --git a/chrome/browser/autofill/autofill_dialog_controller_mac_unittest.mm b/chrome/browser/autofill/autofill_dialog_controller_mac_unittest.mm index bd72c29..8668c7a2 100644 --- a/chrome/browser/autofill/autofill_dialog_controller_mac_unittest.mm +++ b/chrome/browser/autofill/autofill_dialog_controller_mac_unittest.mm @@ -8,7 +8,6 @@ #import "chrome/browser/autofill/autofill_credit_card_view_controller_mac.h" #import "chrome/browser/autofill/autofill_dialog_controller_mac.h" #include "chrome/browser/autofill/autofill_profile.h" -#include "chrome/browser/autofill/personal_data_manager.h" #include "chrome/browser/cocoa/browser_test_helper.h" #import "chrome/browser/cocoa/cocoa_test_helper.h" #include "chrome/browser/pref_service.h" @@ -17,98 +16,11 @@ #include "testing/gtest/include/gtest/gtest.h" namespace { - -// Simulated delay (in milliseconds) for web data loading. -const float kWebDataLoadDelayMilliseconds = 10.0; - -// Mock PersonalDataManager that gives back canned profiles and credit cards -// as well as simulating delayed loading of web data using the -// |PersonalDataManager::Observer| interface. -class PersonalDataManagerMock : public PersonalDataManager { - public: - PersonalDataManagerMock() - : observer_(NULL), - test_data_is_loaded_(true) {} - virtual ~PersonalDataManagerMock() {} - - virtual const std::vector<AutoFillProfile*>& web_profiles() { - return test_profiles_; - } - virtual const std::vector<CreditCard*>& credit_cards() { - return test_credit_cards_; - } - virtual bool IsDataLoaded() const { return test_data_is_loaded_; } - virtual void SetObserver(PersonalDataManager::Observer* observer) { - DCHECK(observer); - observer_ = observer; - - // This delay allows the UI loop to run and display intermediate results - // while the data is loading. When notified that the data is available the - // UI updates with the new data. 10ms is a nice short amount of time to - // let the UI thread update but does not slow down the tests too much. - MessageLoop::current()->PostDelayedTask( - FROM_HERE, - new MessageLoop::QuitTask, - kWebDataLoadDelayMilliseconds); - MessageLoop::current()->Run(); - observer_->OnPersonalDataLoaded(); - } - virtual void RemoveObserver(PersonalDataManager::Observer* observer) { - observer_ = NULL; - } - - std::vector<AutoFillProfile*> test_profiles_; - std::vector<CreditCard*> test_credit_cards_; - PersonalDataManager::Observer* observer_; - bool test_data_is_loaded_; - - private: - DISALLOW_COPY_AND_ASSIGN(PersonalDataManagerMock); -}; - -// Mock profile that gives back our own mock |PersonalDataManager|. -class ProfileMock : public TestingProfile { - public: - ProfileMock() { - test_manager_.reset(new PersonalDataManagerMock); - } - virtual ~ProfileMock() {} - - virtual PersonalDataManager* GetPersonalDataManager() { - return test_manager_.get(); - } - - scoped_ptr<PersonalDataManagerMock> test_manager_; - - private: - DISALLOW_COPY_AND_ASSIGN(ProfileMock); -}; - -// Mock browser that gives back our own |BrowserMock| instance as the profile. -class BrowserMock : public BrowserTestHelper { - public: - BrowserMock() { - test_profile_.reset(new ProfileMock); - } - virtual ~BrowserMock() {} - - // Override of |BrowserTestHelper::profile()|. - virtual TestingProfile* profile() const { - return test_profile_.get(); - } - - scoped_ptr<ProfileMock> test_profile_; - - private: - DISALLOW_COPY_AND_ASSIGN(BrowserMock); -}; - -// Mock observer for the AutoFill settings dialog. -class AutoFillDialogObserverMock : public AutoFillDialogObserver { +class AutoFillDialogObserverTester : public AutoFillDialogObserver { public: - AutoFillDialogObserverMock() + AutoFillDialogObserverTester() : hit_(false) {} - virtual ~AutoFillDialogObserverMock() {} + virtual ~AutoFillDialogObserverTester() {} virtual void OnAutoFillDialogApply( std::vector<AutoFillProfile>* profiles, @@ -131,41 +43,27 @@ class AutoFillDialogObserverMock : public AutoFillDialogObserver { std::vector<CreditCard> credit_cards_; private: - DISALLOW_COPY_AND_ASSIGN(AutoFillDialogObserverMock); + DISALLOW_COPY_AND_ASSIGN(AutoFillDialogObserverTester); }; -// Test fixture for setting up and tearing down our dialog controller under -// test. Also provides helper methods to access the source profiles and -// credit card information stored in mock |PersonalDataManager|. class AutoFillDialogControllerTest : public CocoaTest { public: - AutoFillDialogControllerTest() - : controller_(nil), - imported_profile_(NULL), - imported_credit_card_(NULL) { - } + AutoFillDialogControllerTest() {} void LoadDialog() { controller_ = [AutoFillDialogController controllerWithObserver:&observer_ - profile:helper_.profile() - importedProfile:imported_profile_ - importedCreditCard:imported_credit_card_]; + autoFillProfiles:profiles_ + creditCards:credit_cards_ + profile:helper_.profile()]; [controller_ window]; } - std::vector<AutoFillProfile*>& profiles() { - return helper_.test_profile_->test_manager_->test_profiles_; - } - std::vector<CreditCard*>& credit_cards() { - return helper_.test_profile_->test_manager_->test_credit_cards_; - } - - BrowserMock helper_; - AutoFillDialogObserverMock observer_; - AutoFillDialogController* controller_; // weak reference - AutoFillProfile* imported_profile_; // weak reference - CreditCard* imported_credit_card_; // weak reference + BrowserTestHelper helper_; + AutoFillDialogObserverTester observer_; + AutoFillDialogController* controller_; // weak reference + std::vector<AutoFillProfile*> profiles_; // weak references within vector + std::vector<CreditCard*> credit_cards_; // weak references within vector private: DISALLOW_COPY_AND_ASSIGN(AutoFillDialogControllerTest); @@ -185,7 +83,7 @@ TEST_F(AutoFillDialogControllerTest, CancelButtonDoesNotInformObserver) { TEST_F(AutoFillDialogControllerTest, NoEditsGiveBackOriginalProfile) { AutoFillProfile profile; - profiles().push_back(&profile); + profiles_.push_back(&profile); LoadDialog(); [controller_ save:nil]; @@ -193,13 +91,13 @@ TEST_F(AutoFillDialogControllerTest, NoEditsGiveBackOriginalProfile) { ASSERT_TRUE(observer_.hit_); // Sizes should match. - ASSERT_EQ(observer_.profiles_.size(), profiles().size()); + ASSERT_EQ(observer_.profiles_.size(), profiles_.size()); // Contents should match. size_t i = 0; - size_t count = profiles().size(); + size_t count = profiles_.size(); for (i = 0; i < count; i++) - ASSERT_EQ(observer_.profiles_[i], *profiles()[i]); + ASSERT_EQ(observer_.profiles_[i], *profiles_[i]); // Contents should not match a different profile. AutoFillProfile different_profile; @@ -211,7 +109,7 @@ TEST_F(AutoFillDialogControllerTest, NoEditsGiveBackOriginalProfile) { TEST_F(AutoFillDialogControllerTest, NoEditsGiveBackOriginalCreditCard) { CreditCard credit_card(ASCIIToUTF16("myCC"), 345); - credit_cards().push_back(&credit_card); + credit_cards_.push_back(&credit_card); LoadDialog(); [controller_ save:nil]; @@ -219,14 +117,14 @@ TEST_F(AutoFillDialogControllerTest, NoEditsGiveBackOriginalCreditCard) { ASSERT_TRUE(observer_.hit_); // Sizes should match. - ASSERT_EQ(observer_.credit_cards_.size(), credit_cards().size()); + ASSERT_EQ(observer_.credit_cards_.size(), credit_cards_.size()); // Contents should match. With the exception of the |unique_id|. size_t i = 0; - size_t count = credit_cards().size(); + size_t count = credit_cards_.size(); for (i = 0; i < count; i++) { - credit_cards()[i]->set_unique_id(observer_.credit_cards_[i].unique_id()); - ASSERT_EQ(observer_.credit_cards_[i], *credit_cards()[i]); + credit_cards_[i]->set_unique_id(observer_.credit_cards_[i].unique_id()); + ASSERT_EQ(observer_.credit_cards_[i], *credit_cards_[i]); } // Contents should not match a different profile. @@ -255,7 +153,7 @@ TEST_F(AutoFillDialogControllerTest, AutoFillDataMutation) { profile.SetInfo(AutoFillType(ADDRESS_HOME_COUNTRY), ASCIIToUTF16("USA")); profile.SetInfo(AutoFillType(PHONE_HOME_WHOLE_NUMBER), ASCIIToUTF16("014155552258")); profile.SetInfo(AutoFillType(PHONE_FAX_WHOLE_NUMBER), ASCIIToUTF16("024087172258")); - profiles().push_back(&profile); + profiles_.push_back(&profile); LoadDialog(); @@ -280,8 +178,8 @@ TEST_F(AutoFillDialogControllerTest, AutoFillDataMutation) { ASSERT_TRUE(observer_.hit_); ASSERT_TRUE(observer_.profiles_.size() == 1); - profiles()[0]->set_unique_id(observer_.profiles_[0].unique_id()); - ASSERT_EQ(observer_.profiles_[0], *profiles()[0]); + profiles_[0]->set_unique_id(observer_.profiles_[0].unique_id()); + ASSERT_EQ(observer_.profiles_[0], *profiles_[0]); } TEST_F(AutoFillDialogControllerTest, CreditCardDataMutation) { @@ -294,7 +192,7 @@ TEST_F(AutoFillDialogControllerTest, CreditCardDataMutation) { AutoFillType(CREDIT_CARD_EXP_4_DIGIT_YEAR), ASCIIToUTF16("2012")); credit_card.SetInfo( AutoFillType(CREDIT_CARD_VERIFICATION_CODE), ASCIIToUTF16("222")); - credit_cards().push_back(&credit_card); + credit_cards_.push_back(&credit_card); LoadDialog(); @@ -312,17 +210,17 @@ TEST_F(AutoFillDialogControllerTest, CreditCardDataMutation) { ASSERT_TRUE(observer_.hit_); ASSERT_TRUE(observer_.credit_cards_.size() == 1); - credit_cards()[0]->set_unique_id(observer_.credit_cards_[0].unique_id()); - ASSERT_EQ(observer_.credit_cards_[0], *credit_cards()[0]); + credit_cards_[0]->set_unique_id(observer_.credit_cards_[0].unique_id()); + ASSERT_EQ(observer_.credit_cards_[0], *credit_cards_[0]); } TEST_F(AutoFillDialogControllerTest, TwoProfiles) { AutoFillProfile profile1(ASCIIToUTF16("One"), 1); profile1.SetInfo(AutoFillType(NAME_FIRST), ASCIIToUTF16("Joe")); - profiles().push_back(&profile1); + profiles_.push_back(&profile1); AutoFillProfile profile2(ASCIIToUTF16("Two"), 2); profile2.SetInfo(AutoFillType(NAME_FIRST), ASCIIToUTF16("Bob")); - profiles().push_back(&profile2); + profiles_.push_back(&profile2); LoadDialog(); [controller_ save:nil]; @@ -330,23 +228,23 @@ TEST_F(AutoFillDialogControllerTest, TwoProfiles) { ASSERT_TRUE(observer_.hit_); // Sizes should match. And should be 2. - ASSERT_EQ(observer_.profiles_.size(), profiles().size()); + ASSERT_EQ(observer_.profiles_.size(), profiles_.size()); ASSERT_EQ(observer_.profiles_.size(), 2UL); // Contents should match. With the exception of the |unique_id|. - for (size_t i = 0, count = profiles().size(); i < count; i++) { - profiles()[i]->set_unique_id(observer_.profiles_[i].unique_id()); - ASSERT_EQ(observer_.profiles_[i], *profiles()[i]); + for (size_t i = 0, count = profiles_.size(); i < count; i++) { + profiles_[i]->set_unique_id(observer_.profiles_[i].unique_id()); + ASSERT_EQ(observer_.profiles_[i], *profiles_[i]); } } TEST_F(AutoFillDialogControllerTest, TwoCreditCards) { CreditCard credit_card1(ASCIIToUTF16("Visa"), 1); credit_card1.SetInfo(AutoFillType(CREDIT_CARD_NAME), ASCIIToUTF16("Joe")); - credit_cards().push_back(&credit_card1); + credit_cards_.push_back(&credit_card1); CreditCard credit_card2(ASCIIToUTF16("Mastercard"), 2); credit_card2.SetInfo(AutoFillType(CREDIT_CARD_NAME), ASCIIToUTF16("Bob")); - credit_cards().push_back(&credit_card2); + credit_cards_.push_back(&credit_card2); LoadDialog(); [controller_ save:nil]; @@ -354,20 +252,20 @@ TEST_F(AutoFillDialogControllerTest, TwoCreditCards) { ASSERT_TRUE(observer_.hit_); // Sizes should match. And should be 2. - ASSERT_EQ(observer_.credit_cards_.size(), credit_cards().size()); + ASSERT_EQ(observer_.credit_cards_.size(), credit_cards_.size()); ASSERT_EQ(observer_.credit_cards_.size(), 2UL); // Contents should match. With the exception of the |unique_id|. - for (size_t i = 0, count = credit_cards().size(); i < count; i++) { - credit_cards()[i]->set_unique_id(observer_.credit_cards_[i].unique_id()); - ASSERT_EQ(observer_.credit_cards_[i], *credit_cards()[i]); + for (size_t i = 0, count = credit_cards_.size(); i < count; i++) { + credit_cards_[i]->set_unique_id(observer_.credit_cards_[i].unique_id()); + ASSERT_EQ(observer_.credit_cards_[i], *credit_cards_[i]); } } TEST_F(AutoFillDialogControllerTest, AddNewProfile) { AutoFillProfile profile(ASCIIToUTF16("One"), 1); profile.SetInfo(AutoFillType(NAME_FIRST), ASCIIToUTF16("Joe")); - profiles().push_back(&profile); + profiles_.push_back(&profile); LoadDialog(); [controller_ addNewAddress:nil]; [controller_ save:nil]; @@ -376,7 +274,7 @@ TEST_F(AutoFillDialogControllerTest, AddNewProfile) { ASSERT_TRUE(observer_.hit_); // Sizes should match be different. New size should be 2. - ASSERT_NE(observer_.profiles_.size(), profiles().size()); + ASSERT_NE(observer_.profiles_.size(), profiles_.size()); ASSERT_EQ(observer_.profiles_.size(), 2UL); // New address should match. @@ -387,7 +285,7 @@ TEST_F(AutoFillDialogControllerTest, AddNewProfile) { TEST_F(AutoFillDialogControllerTest, AddNewCreditCard) { CreditCard credit_card(ASCIIToUTF16("Visa"), 1); credit_card.SetInfo(AutoFillType(CREDIT_CARD_NAME), ASCIIToUTF16("Joe")); - credit_cards().push_back(&credit_card); + credit_cards_.push_back(&credit_card); LoadDialog(); [controller_ addNewCreditCard:nil]; [controller_ save:nil]; @@ -396,7 +294,7 @@ TEST_F(AutoFillDialogControllerTest, AddNewCreditCard) { ASSERT_TRUE(observer_.hit_); // Sizes should match be different. New size should be 2. - ASSERT_NE(observer_.credit_cards_.size(), credit_cards().size()); + ASSERT_NE(observer_.credit_cards_.size(), credit_cards_.size()); ASSERT_EQ(observer_.credit_cards_.size(), 2UL); // New address should match. @@ -407,7 +305,7 @@ TEST_F(AutoFillDialogControllerTest, AddNewCreditCard) { TEST_F(AutoFillDialogControllerTest, DeleteProfile) { AutoFillProfile profile(ASCIIToUTF16("One"), 1); profile.SetInfo(AutoFillType(NAME_FIRST), ASCIIToUTF16("Joe")); - profiles().push_back(&profile); + profiles_.push_back(&profile); LoadDialog(); EXPECT_EQ([[[controller_ addressFormViewControllers] lastObject] retainCount], 1UL); @@ -419,14 +317,14 @@ TEST_F(AutoFillDialogControllerTest, DeleteProfile) { ASSERT_TRUE(observer_.hit_); // Sizes should match be different. New size should be 0. - ASSERT_NE(observer_.profiles_.size(), profiles().size()); + ASSERT_NE(observer_.profiles_.size(), profiles_.size()); ASSERT_EQ(observer_.profiles_.size(), 0UL); } TEST_F(AutoFillDialogControllerTest, DeleteCreditCard) { CreditCard credit_card(ASCIIToUTF16("Visa"), 1); credit_card.SetInfo(AutoFillType(CREDIT_CARD_NAME), ASCIIToUTF16("Joe")); - credit_cards().push_back(&credit_card); + credit_cards_.push_back(&credit_card); LoadDialog(); EXPECT_EQ([[[controller_ creditCardFormViewControllers] lastObject] retainCount], 1UL); @@ -438,17 +336,17 @@ TEST_F(AutoFillDialogControllerTest, DeleteCreditCard) { ASSERT_TRUE(observer_.hit_); // Sizes should match be different. New size should be 0. - ASSERT_NE(observer_.credit_cards_.size(), credit_cards().size()); + ASSERT_NE(observer_.credit_cards_.size(), credit_cards_.size()); ASSERT_EQ(observer_.credit_cards_.size(), 0UL); } TEST_F(AutoFillDialogControllerTest, TwoProfilesDeleteOne) { AutoFillProfile profile(ASCIIToUTF16("One"), 1); profile.SetInfo(AutoFillType(NAME_FIRST), ASCIIToUTF16("Joe")); - profiles().push_back(&profile); + profiles_.push_back(&profile); AutoFillProfile profile2(ASCIIToUTF16("Two"), 2); profile2.SetInfo(AutoFillType(NAME_FIRST), ASCIIToUTF16("Bob")); - profiles().push_back(&profile2); + profiles_.push_back(&profile2); LoadDialog(); [controller_ deleteAddress:[[controller_ addressFormViewControllers] lastObject]]; @@ -458,21 +356,21 @@ TEST_F(AutoFillDialogControllerTest, TwoProfilesDeleteOne) { ASSERT_TRUE(observer_.hit_); // Sizes should match be different. New size should be 0. - ASSERT_NE(observer_.profiles_.size(), profiles().size()); + ASSERT_NE(observer_.profiles_.size(), profiles_.size()); ASSERT_EQ(observer_.profiles_.size(), 1UL); // First address should match. - profiles()[0]->set_unique_id(observer_.profiles_[0].unique_id()); + profiles_[0]->set_unique_id(observer_.profiles_[0].unique_id()); ASSERT_EQ(observer_.profiles_[0], profile); } TEST_F(AutoFillDialogControllerTest, TwoCreditCardsDeleteOne) { CreditCard credit_card(ASCIIToUTF16("Visa"), 1); credit_card.SetInfo(AutoFillType(CREDIT_CARD_NAME), ASCIIToUTF16("Joe")); - credit_cards().push_back(&credit_card); + credit_cards_.push_back(&credit_card); CreditCard credit_card2(ASCIIToUTF16("Mastercard"), 2); credit_card2.SetInfo(AutoFillType(CREDIT_CARD_NAME), ASCIIToUTF16("Bob")); - credit_cards().push_back(&credit_card2); + credit_cards_.push_back(&credit_card2); LoadDialog(); [controller_ deleteCreditCard:[[controller_ creditCardFormViewControllers] lastObject]]; @@ -482,11 +380,11 @@ TEST_F(AutoFillDialogControllerTest, TwoCreditCardsDeleteOne) { ASSERT_TRUE(observer_.hit_); // Sizes should match be different. New size should be 0. - ASSERT_NE(observer_.credit_cards_.size(), credit_cards().size()); + ASSERT_NE(observer_.credit_cards_.size(), credit_cards_.size()); ASSERT_EQ(observer_.credit_cards_.size(), 1UL); // First credit card should match. - credit_cards()[0]->set_unique_id(observer_.credit_cards_[0].unique_id()); + credit_cards_[0]->set_unique_id(observer_.credit_cards_[0].unique_id()); ASSERT_EQ(observer_.credit_cards_[0], credit_card); } @@ -535,16 +433,16 @@ TEST_F(AutoFillDialogControllerTest, DefaultsChangingLogic) { // Two profiles, two credit cards. AutoFillProfile profile(ASCIIToUTF16("One"), 1); profile.SetInfo(AutoFillType(NAME_FIRST), ASCIIToUTF16("Joe")); - profiles().push_back(&profile); + profiles_.push_back(&profile); AutoFillProfile profile2(ASCIIToUTF16("Two"), 2); profile2.SetInfo(AutoFillType(NAME_FIRST), ASCIIToUTF16("Bob")); - profiles().push_back(&profile2); + profiles_.push_back(&profile2); CreditCard credit_card(ASCIIToUTF16("Visa"), 1); credit_card.SetInfo(AutoFillType(CREDIT_CARD_NAME), ASCIIToUTF16("Joe")); - credit_cards().push_back(&credit_card); + credit_cards_.push_back(&credit_card); CreditCard credit_card2(ASCIIToUTF16("Mastercard"), 2); credit_card2.SetInfo(AutoFillType(CREDIT_CARD_NAME), ASCIIToUTF16("Bob")); - credit_cards().push_back(&credit_card2); + credit_cards_.push_back(&credit_card2); // Invalid defaults for each. helper_.profile()->GetPrefs()->SetString( @@ -597,60 +495,4 @@ TEST_F(AutoFillDialogControllerTest, DefaultsChangingLogic) { GetString(prefs::kAutoFillDefaultCreditCard)); } -TEST_F(AutoFillDialogControllerTest, WaitForDataToLoad) { - AutoFillProfile profile(ASCIIToUTF16("Home"), 0); - profiles().push_back(&profile); - CreditCard credit_card(ASCIIToUTF16("Visa"), 0); - credit_cards().push_back(&credit_card); - helper_.test_profile_->test_manager_->test_data_is_loaded_ = false; - LoadDialog(); - [controller_ save:nil]; - - // Should hit our observer. - ASSERT_TRUE(observer_.hit_); - - // Sizes should match. - ASSERT_EQ(observer_.profiles_.size(), profiles().size()); - ASSERT_EQ(observer_.credit_cards_.size(), credit_cards().size()); - - // Contents should match. - size_t i = 0; - size_t count = profiles().size(); - for (i = 0; i < count; i++) - ASSERT_EQ(observer_.profiles_[i], *profiles()[i]); - count = credit_cards().size(); - for (i = 0; i < count; i++) { - ASSERT_EQ(observer_.credit_cards_[i], *credit_cards()[i]); - } -} - -TEST_F(AutoFillDialogControllerTest, ImportedParameters) { - AutoFillProfile profile(ASCIIToUTF16("Home"), 0); - imported_profile_ = &profile; - CreditCard credit_card(ASCIIToUTF16("Mastercard"), 0); - imported_credit_card_ = &credit_card; - - // Note: when the |imported_*| parameters are supplied the dialog should - // ignore any profile and credit card information in the - // |PersonalDataManager|. - AutoFillProfile profile_ignored(ASCIIToUTF16("Work"), 0); - profiles().push_back(&profile_ignored); - CreditCard credit_card_ignored(ASCIIToUTF16("Visa"), 0); - credit_cards().push_back(&credit_card_ignored); - - LoadDialog(); - [controller_ save:nil]; - - // Should hit our observer. - ASSERT_TRUE(observer_.hit_); - - // Sizes should match. - ASSERT_EQ(1UL, observer_.profiles_.size()); - ASSERT_EQ(1UL, observer_.credit_cards_.size()); - - // Contents should match. - ASSERT_EQ(observer_.profiles_[0], profile); - ASSERT_EQ(observer_.credit_cards_[0], credit_card); } - -} // namespace diff --git a/chrome/browser/autofill/autofill_dialog_gtk.cc b/chrome/browser/autofill/autofill_dialog_gtk.cc index 80fea6a..2369fbd 100644 --- a/chrome/browser/autofill/autofill_dialog_gtk.cc +++ b/chrome/browser/autofill/autofill_dialog_gtk.cc @@ -1066,14 +1066,9 @@ void AutoFillDialog::AddCreditCard(const CreditCard& credit_card, /////////////////////////////////////////////////////////////////////////////// // Factory/finder method: -// TODO(jhawkins): Need to update implementation to match new interface for -// |imported_profile| and |imported_credit_card| parameters. -// See http://crbug.com/41010 -void ShowAutoFillDialog(gfx::NativeView parent, +void ShowAutoFillDialog(gfx::NativeWindow parent, AutoFillDialogObserver* observer, - Profile* profile, - AutoFillProfile* imported_profile, - CreditCard* imported_credit_card) { + Profile* profile) { // It's possible we haven't shown the InfoBar yet, but if the user is in the // AutoFill dialog, she doesn't need to be asked to enable or disable // AutoFill. diff --git a/chrome/browser/autofill/autofill_dialog_mac.mm b/chrome/browser/autofill/autofill_dialog_mac.mm index 63d5478..c011822 100644 --- a/chrome/browser/autofill/autofill_dialog_mac.mm +++ b/chrome/browser/autofill/autofill_dialog_mac.mm @@ -10,11 +10,10 @@ // Mac implementation of |ShowAutoFillDialog| interface defined in // |chrome/browser/autofill/autofill_dialog.h|. -void ShowAutoFillDialog(gfx::NativeView parent, - AutoFillDialogObserver* observer, - Profile* profile, - AutoFillProfile* imported_profile, - CreditCard* imported_credit_card) { +void ShowAutoFillDialog(AutoFillDialogObserver* observer, + const std::vector<AutoFillProfile*>& profiles, + const std::vector<CreditCard*>& credit_cards, + Profile *profile) { // It's possible we haven't shown the InfoBar yet, but if the user is in the // AutoFill dialog, she doesn't need to be asked to enable or disable // AutoFill. @@ -22,7 +21,8 @@ void ShowAutoFillDialog(gfx::NativeView parent, [AutoFillDialogController showAutoFillDialogWithObserver:observer - profile:profile - importedProfile:imported_profile - importedCreditCard:imported_credit_card]; + autoFillProfiles:profiles + creditCards:credit_cards + profile:profile]; } + diff --git a/chrome/browser/autofill/autofill_manager.cc b/chrome/browser/autofill/autofill_manager.cc index 32c4e24..d2ddf6b 100644 --- a/chrome/browser/autofill/autofill_manager.cc +++ b/chrome/browser/autofill/autofill_manager.cc @@ -48,6 +48,9 @@ AutoFillManager::AutoFillManager(TabContents* tab_contents) } AutoFillManager::~AutoFillManager() { + // This is NULL in the MockAutoFillManager. + if (personal_data_) + personal_data_->RemoveObserver(this); download_manager_.SetObserver(NULL); } @@ -261,6 +264,24 @@ bool AutoFillManager::FillAutoFillFormData(int query_id, return true; } +void AutoFillManager::OnPersonalDataLoaded() { + // We might have been alerted that the PersonalDataManager has loaded, so + // remove ourselves as observer. + personal_data_->RemoveObserver(this); + +#if !defined(OS_WIN) +#if defined(OS_MACOSX) + ShowAutoFillDialog(personal_data_, + personal_data_->web_profiles(), + personal_data_->credit_cards(), + tab_contents_->profile()->GetOriginalProfile()); +#else // defined(OS_MACOSX) + ShowAutoFillDialog(NULL, personal_data_, + tab_contents_->profile()->GetOriginalProfile()); +#endif // defined(OS_MACOSX) +#endif // !defined(OS_WIN) +} + void AutoFillManager::OnInfoBarClosed() { PrefService* prefs = tab_contents_->profile()->GetPrefs(); prefs->SetBoolean(prefs::kAutoFillEnabled, true); @@ -274,18 +295,20 @@ void AutoFillManager::OnInfoBarAccepted() { prefs->SetBoolean(prefs::kAutoFillEnabled, true); // This is the first time the user is interacting with AutoFill, so set the - // uploaded form structure as the initial profile and credit card in the - // AutoFillDialog. - AutoFillProfile* profile = NULL; - CreditCard* credit_card = NULL; - // TODO(dhollowa) Now that we aren't immediately saving the imported form - // data, we should store the profile and CC in the AFM instead of the PDM. - personal_data_->GetImportedFormData(&profile, &credit_card); - ShowAutoFillDialog(tab_contents_->GetContentNativeView(), - personal_data_, - tab_contents_->profile()->GetOriginalProfile(), - profile, - credit_card); + // uploaded form structure as the initial profile in the AutoFillDialog. + personal_data_->SaveImportedFormData(); + +#if defined(OS_WIN) + ShowAutoFillDialog(tab_contents_->GetContentNativeView(), personal_data_, + tab_contents_->profile()->GetOriginalProfile()); +#else + // If the personal data manager has not loaded the data yet, set ourselves as + // its observer so that we can listen for the OnPersonalDataLoaded signal. + if (!personal_data_->IsDataLoaded()) + personal_data_->SetObserver(this); + else + OnPersonalDataLoaded(); +#endif } void AutoFillManager::OnInfoBarCancelled() { diff --git a/chrome/browser/autofill/autofill_manager.h b/chrome/browser/autofill/autofill_manager.h index 4eac435..eb0abe7 100644 --- a/chrome/browser/autofill/autofill_manager.h +++ b/chrome/browser/autofill/autofill_manager.h @@ -33,6 +33,7 @@ extern const char* kAutoFillLearnMoreUrl; // Manages saving and restoring the user's personal information entered into web // forms. class AutoFillManager : public RenderViewHostDelegate::AutoFill, + public PersonalDataManager::Observer, public AutoFillDownloadManager::Observer { public: explicit AutoFillManager(TabContents* tab_contents); @@ -55,6 +56,9 @@ class AutoFillManager : public RenderViewHostDelegate::AutoFill, const string16& value, const string16& label); + // PersonalDataManager::Observer implementation: + virtual void OnPersonalDataLoaded(); + // Called by the AutoFillInfoBarDelegate when the user closes the infobar. virtual void OnInfoBarClosed(); diff --git a/chrome/browser/autofill/personal_data_manager.cc b/chrome/browser/autofill/personal_data_manager.cc index 073009ec..254fd06 100644 --- a/chrome/browser/autofill/personal_data_manager.cc +++ b/chrome/browser/autofill/personal_data_manager.cc @@ -234,22 +234,6 @@ void PersonalDataManager::SaveImportedFormData() { } } -void PersonalDataManager::GetImportedFormData(AutoFillProfile** profile, - CreditCard** credit_card) { - DCHECK(profile); - DCHECK(credit_card); - - if (imported_profile_.get()) { - imported_profile_->set_label(ASCIIToUTF16(kUnlabeled)); - } - *profile = imported_profile_.get(); - - if (imported_credit_card_.get()) { - imported_credit_card_->set_label(ASCIIToUTF16(kUnlabeled)); - } - *credit_card = imported_credit_card_.get(); -} - void PersonalDataManager::SetProfiles(std::vector<AutoFillProfile>* profiles) { if (profile_->IsOffTheRecord()) return; diff --git a/chrome/browser/autofill/personal_data_manager.h b/chrome/browser/autofill/personal_data_manager.h index 9b5ac12..a5903a9 100644 --- a/chrome/browser/autofill/personal_data_manager.h +++ b/chrome/browser/autofill/personal_data_manager.h @@ -50,10 +50,10 @@ class PersonalDataManager : public WebDataServiceConsumer, std::vector<CreditCard>* credit_cards); // Sets the listener to be notified of PersonalDataManager events. - virtual void SetObserver(PersonalDataManager::Observer* observer); + void SetObserver(PersonalDataManager::Observer* observer); // Removes |observer| as the observer of this PersonalDataManager. - virtual void RemoveObserver(PersonalDataManager::Observer* observer); + void RemoveObserver(PersonalDataManager::Observer* observer); // If AutoFill is able to determine the field types of a significant number // of field types that contain information in the FormStructures and the user @@ -67,14 +67,6 @@ class PersonalDataManager : public WebDataServiceConsumer, // exist. void SaveImportedFormData(); - // Gets |imported_profile_| and |imported_credit_card_| and returns their - // values in |profile| and |credit_card| parameters respectively. One or - // both may return NULL. The objects returned are owned by the - // PersonalDataManager, so should be considered weak references by caller. - // TODO(dhollowa) Now that we aren't immediately saving the imported form - // data, we should store the profile and CC in the AFM instead of the PDM. - void GetImportedFormData(AutoFillProfile** profile, CreditCard** credit_card); - // Sets |web_profiles_| to the contents of |profiles| and updates the web // database by adding, updating and removing profiles. Sets the unique ID of // newly-added profiles. @@ -110,10 +102,8 @@ class PersonalDataManager : public WebDataServiceConsumer, // card information, respectively. |profiles()| returns both web and // auxiliary profiles. |web_profiles()| returns only web profiles. const std::vector<AutoFillProfile*>& profiles(); - virtual const std::vector<AutoFillProfile*>& web_profiles(); - virtual const std::vector<CreditCard*>& credit_cards() { - return credit_cards_.get(); - } + const std::vector<AutoFillProfile*>& web_profiles(); + const std::vector<CreditCard*>& credit_cards() { return credit_cards_.get(); } // Returns the index of the default profile within the vector returned by // |web_profiles()|, or -1 if there are no profiles. diff --git a/chrome/browser/cocoa/browser_test_helper.h b/chrome/browser/cocoa/browser_test_helper.h index 8869aad..b7e3e64 100644 --- a/chrome/browser/cocoa/browser_test_helper.h +++ b/chrome/browser/cocoa/browser_test_helper.h @@ -37,7 +37,7 @@ class BrowserTestHelper { browser_.reset(new Browser(Browser::TYPE_NORMAL, profile_.get())); } - virtual ~BrowserTestHelper() { + ~BrowserTestHelper() { // Delete the testing profile on the UI thread. But first release the // browser, since it may trigger accesses to the profile upon destruction. browser_.reset(NULL); @@ -45,7 +45,7 @@ class BrowserTestHelper { message_loop_.RunAllPending(); } - virtual TestingProfile* profile() const { return profile_.get(); } + TestingProfile* profile() const { return profile_.get(); } Browser* browser() const { return browser_.get(); } // Creates the browser window. To close this window call |CloseBrowserWindow|. diff --git a/chrome/browser/cocoa/preferences_window_controller.h b/chrome/browser/cocoa/preferences_window_controller.h index 97d8a14..c49a879 100644 --- a/chrome/browser/cocoa/preferences_window_controller.h +++ b/chrome/browser/cocoa/preferences_window_controller.h @@ -10,6 +10,7 @@ #include "chrome/browser/pref_member.h" namespace PreferencesWindowControllerInternal { +class PersonalDataManagerObserver; class PrefObserverBridge; } @@ -84,6 +85,9 @@ class ProfileSyncService; // User Data panel BooleanPrefMember askSavePasswords_; BooleanPrefMember formAutofill_; + // Manages PersonalDataManager loading. + scoped_ptr<PreferencesWindowControllerInternal::PersonalDataManagerObserver> + personalDataManagerObserver_; IBOutlet NSButton* autoFillSettingsButton_; IBOutlet NSButton* syncButton_; IBOutlet NSButton* syncCustomizeButton_; diff --git a/chrome/browser/cocoa/preferences_window_controller.mm b/chrome/browser/cocoa/preferences_window_controller.mm index a27a9cf..dd37f62 100644 --- a/chrome/browser/cocoa/preferences_window_controller.mm +++ b/chrome/browser/cocoa/preferences_window_controller.mm @@ -375,6 +375,100 @@ class PrefObserverBridge : public NotificationObserver, PreferencesWindowController* controller_; // weak, owns us }; +// PersonalDataManagerObserver facilitates asynchronous loading of +// PersonalDataManager data before showing the auto fill settings dialog to the +// user. It acts as a C++-based delegate for the |PreferencesWindowController|. +class PersonalDataManagerObserver : public PersonalDataManager::Observer { + public: + explicit PersonalDataManagerObserver( + PersonalDataManager* personal_data_manager, + Profile* profile) + : personal_data_manager_(personal_data_manager), + profile_(profile) { + } + + virtual ~PersonalDataManagerObserver(); + + // Notifies the observer that the PersonalDataManager has finished loading. + virtual void OnPersonalDataLoaded(); + + // Static method to dispatch to |ShowAutoFillDialog| method in autofill + // module. This is public to facilitate direct external call when the + // data manager has already loaded its data. + static void ShowAutoFillDialog(PersonalDataManager* personal_data_manager, + Profile* profile); + + private: + // Utility method to remove |this| from |personal_data_manager_| as an + // observer. + void RemoveObserver(); + + // The object in which we are registered as an observer. We hold on to + // it to facilitate un-registering ourself in the destructor and in the + // |OnPersonalDataLoaded| method. This may be NULL. + // Weak reference. + PersonalDataManager* personal_data_manager_; + + // Profile of caller. Held as weak reference. May not be NULL. + Profile* profile_; + + private: + DISALLOW_COPY_AND_ASSIGN(PersonalDataManagerObserver); +}; + +// During destruction ensure that we are removed from the +// |personal_data_manager_| as an observer. +PersonalDataManagerObserver::~PersonalDataManagerObserver() { + RemoveObserver(); +} + +void PersonalDataManagerObserver::RemoveObserver() { + if (personal_data_manager_) { + personal_data_manager_->RemoveObserver(this); + } +} + +// The data is ready so display our dialog. Recursively call +// |showAutoFillSettings:| to try again now knowing that the +// |PersonalDataManager| is ready. Once done we clear the observer +// (deleting |this| in the process). +void PersonalDataManagerObserver::OnPersonalDataLoaded() { + RemoveObserver(); + PersonalDataManagerObserver::ShowAutoFillDialog(personal_data_manager_, + profile_); +} + +// Dispatches request to show the autofill dialog. If there are no profiles +// in the |personal_data_manager| the we create a new one here. Similary with +// credit card info. +void PersonalDataManagerObserver::ShowAutoFillDialog( + PersonalDataManager* personal_data_manager, Profile* profile) { + DCHECK(profile); + if (!personal_data_manager) + return; + + std::vector<AutoFillProfile*> profiles = + personal_data_manager->web_profiles(); + AutoFillProfile autofill_profile(ASCIIToUTF16(""), 0); + if (profiles.size() == 0) { + string16 new_profile_name = + l10n_util::GetStringUTF16(IDS_AUTOFILL_NEW_ADDRESS); + autofill_profile.set_label(new_profile_name); + profiles.push_back(&autofill_profile); + } + + std::vector<CreditCard*> credit_cards = personal_data_manager->credit_cards(); + CreditCard credit_card(ASCIIToUTF16(""), 0); + if (credit_cards.size() == 0) { + string16 new_credit_card_name = + l10n_util::GetStringUTF16(IDS_AUTOFILL_NEW_CREDITCARD); + credit_card.set_label(new_credit_card_name); + credit_cards.push_back(&credit_card); + } + + ::ShowAutoFillDialog(personal_data_manager, profiles, credit_cards, profile); +} + } // namespace PreferencesWindowControllerInternal @implementation PreferencesWindowController @@ -657,6 +751,7 @@ class PrefObserverBridge : public NotificationObserver, [customPagesSource_ removeObserver:self forKeyPath:@"customHomePages"]; [[NSNotificationCenter defaultCenter] removeObserver:self]; [self unregisterPrefObservers]; + personalDataManagerObserver_.reset(); [super dealloc]; } @@ -1157,7 +1252,19 @@ const int kDisabledIndex = 1; return; } - ShowAutoFillDialog(NULL, personalDataManager, profile_, NULL, NULL); + if (personalDataManager->IsDataLoaded()) { + // |personalDataManager| data is loaded, we can proceed with the dialog. + PreferencesWindowControllerInternal:: + PersonalDataManagerObserver::ShowAutoFillDialog(personalDataManager, + profile_); + } else { + // |personalDataManager| data is NOT loaded, so we load it here, installing + // our observer. + personalDataManagerObserver_.reset( + new PreferencesWindowControllerInternal::PersonalDataManagerObserver( + personalDataManager, profile_)); + personalDataManager->SetObserver(personalDataManagerObserver_.get()); + } } // Called to import data from other browsers (Safari, Firefox, etc). diff --git a/chrome/browser/gtk/options/content_page_gtk.cc b/chrome/browser/gtk/options/content_page_gtk.cc index 770476d..3b92201 100644 --- a/chrome/browser/gtk/options/content_page_gtk.cc +++ b/chrome/browser/gtk/options/content_page_gtk.cc @@ -215,7 +215,7 @@ void ContentPageGtk::OnPersonalDataLoaded() { // remove ourselves as observer. personal_data_->RemoveObserver(this); - ShowAutoFillDialog(NULL, personal_data_, profile(), NULL, NULL); + ShowAutoFillDialog(NULL, personal_data_, profile()); } GtkWidget* ContentPageGtk::InitPasswordSavingGroup() { diff --git a/chrome/browser/views/autofill_profiles_view_win.cc b/chrome/browser/views/autofill_profiles_view_win.cc index 3f348bb..0830b6e 100644 --- a/chrome/browser/views/autofill_profiles_view_win.cc +++ b/chrome/browser/views/autofill_profiles_view_win.cc @@ -1435,14 +1435,9 @@ void AutoFillProfilesView::AutoFillScrollView::Layout() { } // Declared in "chrome/browser/autofill/autofill_dialog.h" -// TODO(georgey): Need to update implementation to match new interface for -// |imported_profile| and |imported_credit_card| parameters. -// See http://crbug.com/41010 -void ShowAutoFillDialog(gfx::NativeView parent, +void ShowAutoFillDialog(gfx::NativeWindow parent, AutoFillDialogObserver* observer, - Profile* profile, - AutoFillProfile* imported_profile, - CreditCard* imported_credit_card) { + Profile* profile) { DCHECK(profile); // It's possible we haven't shown the InfoBar yet, but if the user is in the |