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 | ||||
| -rw-r--r-- | chrome/browser/views/options/content_page_view.cc | 4 |
16 files changed, 462 insertions, 281 deletions
diff --git a/chrome/browser/autofill/autofill_dialog.h b/chrome/browser/autofill/autofill_dialog.h index 2dd3855..2f02443 100644 --- a/chrome/browser/autofill/autofill_dialog.h +++ b/chrome/browser/autofill/autofill_dialog.h @@ -33,19 +33,21 @@ 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. -#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, +void ShowAutoFillDialog(gfx::NativeView parent, AutoFillDialogObserver* observer, - Profile* profile); -#endif + Profile* profile, + AutoFillProfile* imported_profile, + CreditCard* imported_credit_card); #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 1d90836e..220634b 100644 --- a/chrome/browser/autofill/autofill_dialog_controller_mac.h +++ b/chrome/browser/autofill/autofill_dialog_controller_mac.h @@ -8,10 +8,15 @@ #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; @@ -42,11 +47,17 @@ 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 @@ -64,13 +75,15 @@ 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 - autoFillProfiles:(const std::vector<AutoFillProfile*>&)profiles - creditCards:(const std::vector<CreditCard*>&)creditCards - profile:(Profile*)profile; + profile:(Profile*)profile + importedProfile:(AutoFillProfile*)importedProfile + importedCreditCard:(CreditCard*)importedCreditCard; // IBActions for the dialog buttons. - (IBAction)save:(id)sender; @@ -105,14 +118,14 @@ class Profile; // Note: controller is autoreleased when |-closeDialog| is called. + (AutoFillDialogController*)controllerWithObserver: (AutoFillDialogObserver*)observer - autoFillProfiles:(const std::vector<AutoFillProfile*>&)profiles - creditCards:(const std::vector<CreditCard*>&)creditCards - profile:(Profile*)profile; + profile:(Profile*)profile + importedProfile:(AutoFillProfile*)importedProfile + importedCreditCard:(CreditCard*)importedCreditCard; - (id)initWithObserver:(AutoFillDialogObserver*)observer - autoFillProfiles:(const std::vector<AutoFillProfile*>&)profiles - creditCards:(const std::vector<CreditCard*>&)creditCards - profile:(Profile*)profile; + profile:(Profile*)profile + importedProfile:(AutoFillProfile*)importedProfile + importedCreditCard:(CreditCard*)importedCreditCard; - (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 2618044..2e3a180 100644 --- a/chrome/browser/autofill/autofill_dialog_controller_mac.mm +++ b/chrome/browser/autofill/autofill_dialog_controller_mac.mm @@ -10,6 +10,7 @@ #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" @@ -19,6 +20,80 @@ #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; @@ -29,14 +104,14 @@ @synthesize auxiliaryEnabled = auxiliaryEnabled_; + (void)showAutoFillDialogWithObserver:(AutoFillDialogObserver*)observer - autoFillProfiles:(const std::vector<AutoFillProfile*>&)profiles - creditCards:(const std::vector<CreditCard*>&)creditCards - profile:(Profile*)profile { + profile:(Profile*)profile + importedProfile:(AutoFillProfile*) importedProfile + importedCreditCard:(CreditCard*) importedCreditCard { AutoFillDialogController* controller = [AutoFillDialogController controllerWithObserver:observer - autoFillProfiles:profiles - creditCards:creditCards - profile:profile]; + profile:profile + importedProfile:importedProfile + importedCreditCard:importedCreditCard]; // Only run modal dialog if it is not already being shown. if (![controller isWindowLoaded]) { @@ -46,7 +121,23 @@ - (void)awakeFromNib { [addressSectionBox_ setShowTopLine:FALSE]; - [self installChildViews]; + + 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()); + } } // NSWindow Delegate callback. When the window closes the controller can @@ -365,17 +456,17 @@ @implementation AutoFillDialogController (ExposedForUnitTests) + (AutoFillDialogController*)controllerWithObserver: - (AutoFillDialogObserver*)observer - autoFillProfiles:(const std::vector<AutoFillProfile*>&)profiles - creditCards:(const std::vector<CreditCard*>&)creditCards - profile:(Profile*)profile { + (AutoFillDialogObserver*)observer + profile:(Profile*)profile + importedProfile:(AutoFillProfile*)importedProfile + importedCreditCard:(CreditCard*)importedCreditCard { // Deallocation is done upon window close. See |windowWillClose:|. AutoFillDialogController* controller = [[self alloc] initWithObserver:observer - autoFillProfiles:profiles - creditCards:creditCards - profile:profile]; + profile:profile + importedProfile:importedProfile + importedCreditCard:importedCreditCard]; return controller; } @@ -384,9 +475,9 @@ // |profiles| are non-retained immutable list of autofill profiles. // |creditCards| are non-retained immutable list of credit card info. - (id)initWithObserver:(AutoFillDialogObserver*)observer - autoFillProfiles:(const std::vector<AutoFillProfile*>&)profiles - creditCards:(const std::vector<CreditCard*>&)creditCards - profile:(Profile*)profile { + profile:(Profile*)profile + importedProfile:(AutoFillProfile*)importedProfile + importedCreditCard:(CreditCard*)importedCreditCard { CHECK(profile); // Use initWithWindowNibPath: instead of initWithWindowNibName: so we // can override it in a unit test. @@ -395,18 +486,9 @@ 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( @@ -520,4 +602,30 @@ [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 8668c7a2..bd72c29 100644 --- a/chrome/browser/autofill/autofill_dialog_controller_mac_unittest.mm +++ b/chrome/browser/autofill/autofill_dialog_controller_mac_unittest.mm @@ -8,6 +8,7 @@ #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" @@ -16,11 +17,98 @@ #include "testing/gtest/include/gtest/gtest.h" namespace { -class AutoFillDialogObserverTester : public AutoFillDialogObserver { + +// 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 { public: - AutoFillDialogObserverTester() + AutoFillDialogObserverMock() : hit_(false) {} - virtual ~AutoFillDialogObserverTester() {} + virtual ~AutoFillDialogObserverMock() {} virtual void OnAutoFillDialogApply( std::vector<AutoFillProfile>* profiles, @@ -43,27 +131,41 @@ class AutoFillDialogObserverTester : public AutoFillDialogObserver { std::vector<CreditCard> credit_cards_; private: - DISALLOW_COPY_AND_ASSIGN(AutoFillDialogObserverTester); + DISALLOW_COPY_AND_ASSIGN(AutoFillDialogObserverMock); }; +// 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() {} + AutoFillDialogControllerTest() + : controller_(nil), + imported_profile_(NULL), + imported_credit_card_(NULL) { + } void LoadDialog() { controller_ = [AutoFillDialogController controllerWithObserver:&observer_ - autoFillProfiles:profiles_ - creditCards:credit_cards_ - profile:helper_.profile()]; + profile:helper_.profile() + importedProfile:imported_profile_ + importedCreditCard:imported_credit_card_]; [controller_ window]; } - 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 + 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 private: DISALLOW_COPY_AND_ASSIGN(AutoFillDialogControllerTest); @@ -83,7 +185,7 @@ TEST_F(AutoFillDialogControllerTest, CancelButtonDoesNotInformObserver) { TEST_F(AutoFillDialogControllerTest, NoEditsGiveBackOriginalProfile) { AutoFillProfile profile; - profiles_.push_back(&profile); + profiles().push_back(&profile); LoadDialog(); [controller_ save:nil]; @@ -91,13 +193,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; @@ -109,7 +211,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]; @@ -117,14 +219,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. @@ -153,7 +255,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(); @@ -178,8 +280,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) { @@ -192,7 +294,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(); @@ -210,17 +312,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]; @@ -228,23 +330,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]; @@ -252,20 +354,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]; @@ -274,7 +376,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. @@ -285,7 +387,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]; @@ -294,7 +396,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. @@ -305,7 +407,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); @@ -317,14 +419,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); @@ -336,17 +438,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]]; @@ -356,21 +458,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]]; @@ -380,11 +482,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); } @@ -433,16 +535,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( @@ -495,4 +597,60 @@ 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 2369fbd..80fea6a 100644 --- a/chrome/browser/autofill/autofill_dialog_gtk.cc +++ b/chrome/browser/autofill/autofill_dialog_gtk.cc @@ -1066,9 +1066,14 @@ void AutoFillDialog::AddCreditCard(const CreditCard& credit_card, /////////////////////////////////////////////////////////////////////////////// // Factory/finder method: -void ShowAutoFillDialog(gfx::NativeWindow parent, +// 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, AutoFillDialogObserver* observer, - Profile* profile) { + Profile* profile, + AutoFillProfile* imported_profile, + CreditCard* imported_credit_card) { // 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 c011822..63d5478 100644 --- a/chrome/browser/autofill/autofill_dialog_mac.mm +++ b/chrome/browser/autofill/autofill_dialog_mac.mm @@ -10,10 +10,11 @@ // Mac implementation of |ShowAutoFillDialog| interface defined in // |chrome/browser/autofill/autofill_dialog.h|. -void ShowAutoFillDialog(AutoFillDialogObserver* observer, - const std::vector<AutoFillProfile*>& profiles, - const std::vector<CreditCard*>& credit_cards, - Profile *profile) { +void ShowAutoFillDialog(gfx::NativeView parent, + AutoFillDialogObserver* observer, + Profile* profile, + AutoFillProfile* imported_profile, + CreditCard* imported_credit_card) { // 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. @@ -21,8 +22,7 @@ void ShowAutoFillDialog(AutoFillDialogObserver* observer, [AutoFillDialogController showAutoFillDialogWithObserver:observer - autoFillProfiles:profiles - creditCards:credit_cards - profile:profile]; + profile:profile + importedProfile:imported_profile + importedCreditCard:imported_credit_card]; } - diff --git a/chrome/browser/autofill/autofill_manager.cc b/chrome/browser/autofill/autofill_manager.cc index d2ddf6b..32c4e24 100644 --- a/chrome/browser/autofill/autofill_manager.cc +++ b/chrome/browser/autofill/autofill_manager.cc @@ -48,9 +48,6 @@ AutoFillManager::AutoFillManager(TabContents* tab_contents) } AutoFillManager::~AutoFillManager() { - // This is NULL in the MockAutoFillManager. - if (personal_data_) - personal_data_->RemoveObserver(this); download_manager_.SetObserver(NULL); } @@ -264,24 +261,6 @@ 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); @@ -295,20 +274,18 @@ 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 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 + // 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); } void AutoFillManager::OnInfoBarCancelled() { diff --git a/chrome/browser/autofill/autofill_manager.h b/chrome/browser/autofill/autofill_manager.h index eb0abe7..4eac435 100644 --- a/chrome/browser/autofill/autofill_manager.h +++ b/chrome/browser/autofill/autofill_manager.h @@ -33,7 +33,6 @@ 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); @@ -56,9 +55,6 @@ 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 254fd06..073009ec 100644 --- a/chrome/browser/autofill/personal_data_manager.cc +++ b/chrome/browser/autofill/personal_data_manager.cc @@ -234,6 +234,22 @@ 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 a5903a9..9b5ac12 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. - void SetObserver(PersonalDataManager::Observer* observer); + virtual void SetObserver(PersonalDataManager::Observer* observer); // Removes |observer| as the observer of this PersonalDataManager. - void RemoveObserver(PersonalDataManager::Observer* observer); + virtual 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,6 +67,14 @@ 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. @@ -102,8 +110,10 @@ 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(); - const std::vector<AutoFillProfile*>& web_profiles(); - const std::vector<CreditCard*>& credit_cards() { return credit_cards_.get(); } + virtual const std::vector<AutoFillProfile*>& web_profiles(); + virtual 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 b7e3e64..8869aad 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())); } - ~BrowserTestHelper() { + virtual ~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(); } - TestingProfile* profile() const { return profile_.get(); } + virtual 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 c49a879..97d8a14 100644 --- a/chrome/browser/cocoa/preferences_window_controller.h +++ b/chrome/browser/cocoa/preferences_window_controller.h @@ -10,7 +10,6 @@ #include "chrome/browser/pref_member.h" namespace PreferencesWindowControllerInternal { -class PersonalDataManagerObserver; class PrefObserverBridge; } @@ -85,9 +84,6 @@ 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 dd37f62..a27a9cf 100644 --- a/chrome/browser/cocoa/preferences_window_controller.mm +++ b/chrome/browser/cocoa/preferences_window_controller.mm @@ -375,100 +375,6 @@ 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 @@ -751,7 +657,6 @@ void PersonalDataManagerObserver::ShowAutoFillDialog( [customPagesSource_ removeObserver:self forKeyPath:@"customHomePages"]; [[NSNotificationCenter defaultCenter] removeObserver:self]; [self unregisterPrefObservers]; - personalDataManagerObserver_.reset(); [super dealloc]; } @@ -1252,19 +1157,7 @@ const int kDisabledIndex = 1; return; } - 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()); - } + ShowAutoFillDialog(NULL, personalDataManager, profile_, NULL, NULL); } // 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 3b92201..770476d 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()); + ShowAutoFillDialog(NULL, personal_data_, profile(), NULL, NULL); } GtkWidget* ContentPageGtk::InitPasswordSavingGroup() { diff --git a/chrome/browser/views/autofill_profiles_view_win.cc b/chrome/browser/views/autofill_profiles_view_win.cc index 0830b6e..3f348bb 100644 --- a/chrome/browser/views/autofill_profiles_view_win.cc +++ b/chrome/browser/views/autofill_profiles_view_win.cc @@ -1435,9 +1435,14 @@ void AutoFillProfilesView::AutoFillScrollView::Layout() { } // Declared in "chrome/browser/autofill/autofill_dialog.h" -void ShowAutoFillDialog(gfx::NativeWindow parent, +// 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, AutoFillDialogObserver* observer, - Profile* profile) { + Profile* profile, + AutoFillProfile* imported_profile, + CreditCard* imported_credit_card) { DCHECK(profile); // It's possible we haven't shown the InfoBar yet, but if the user is in the diff --git a/chrome/browser/views/options/content_page_view.cc b/chrome/browser/views/options/content_page_view.cc index 22b1b83..5a84125 100644 --- a/chrome/browser/views/options/content_page_view.cc +++ b/chrome/browser/views/options/content_page_view.cc @@ -122,7 +122,9 @@ void ContentPageView::ButtonPressed( DCHECK(profile()->GetPersonalDataManager()); ShowAutoFillDialog(GetWindow()->GetNativeWindow(), profile()->GetPersonalDataManager(), - profile()); + profile(), + NULL, + NULL); } else if (sender == themes_reset_button_) { UserMetricsRecordAction(UserMetricsAction("Options_ThemesReset"), profile()->GetPrefs()); |