diff options
author | dhollowa@chromium.org <dhollowa@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-05-05 02:47:37 +0000 |
---|---|---|
committer | dhollowa@chromium.org <dhollowa@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-05-05 02:47:37 +0000 |
commit | f749da537323d5dd900e636edeb111d8f52e7e9d (patch) | |
tree | 2d1da8ab40e8dc3aec06d53f29d2423702021d02 | |
parent | cd69619bc053d527b7c82aac81c605157b28d01f (diff) | |
download | chromium_src-f749da537323d5dd900e636edeb111d8f52e7e9d.zip chromium_src-f749da537323d5dd900e636edeb111d8f52e7e9d.tar.gz chromium_src-f749da537323d5dd900e636edeb111d8f52e7e9d.tar.bz2 |
AutoFill profile shouldn't be saved when cancelled during initial setup.
Relanding after revert of: 46424
http://codereview.chromium.org/1902003
Fixing compile error on Windows.
For first encounter with fillable form, the AutoFillManager::OnInfoBarAccepted()
call now passes the new profile and credit card information to the dialog directly
instead of saving it to the database and then invoking the dialog. This facilitates
"Cancel" in the dialog where the new information is not persisted.
This was a good opportunity to refactor the deferred PersonalDataManager::Observer() logic
out of the preferences dialog and into the AutoFillDialogController itself.
This also consolidates the Windows, Mac, and Linux interfaces for the ShowAutoFillDialog()
call. More work is required on Linux and Windows to properly conform to this interface and
fix bug 41010. The Linux and Windows implementations will need to respect the new input
parameters |imported_profile| and |imported_credit_card|.
BUG=41010
TEST=AutoFillDialogControllerTest.WaitForDataToLoad, AutoFillDialogControllerTest.ImportedParameters
Review URL: http://codereview.chromium.org/1952002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@46430 0039d316-1c4b-4281-b951-d872f2087c98
-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()); |