Fix DCHECK() when updating instruments with no phone number.

Also refactors SaveToWallet() to take a |reference_{instrument,address}| instead of setting an object_id in the controller as well as refactors the way that WalletClient queues pending requests to be less error prone and maybe copy less memory? (definitely simpler)

R=estade@chromium.org
BUG=326671
TEST=unit_tests
NOTRY=true

Review URL: https://codereview.chromium.org/100743006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@240873 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 233 insertions, 193 deletions

diff --git a/components/autofill/content/browser/wallet/instrument.h b/components/autofill/content/browser/wallet/instrument.h
index b706a7e..01857f8 100644
--- a/components/autofill/content/browser/wallet/instrument.h
+++ b/components/autofill/content/browser/wallet/instrument.h
@@ -67,7 +67,6 @@ class Instrument {
   FormOfPayment form_of_payment() const { return form_of_payment_; }
   const base::string16& last_four_digits() const { return last_four_digits_; }
   const std::string& object_id() const { return object_id_; }
-  void set_object_id(const std::string& object_id) { object_id_ = object_id; }
 
  private:
   void Init();
diff --git a/components/autofill/content/browser/wallet/mock_wallet_client.cc b/components/autofill/content/browser/wallet/mock_wallet_client.cc
index 37955188..10e8422 100644
--- a/components/autofill/content/browser/wallet/mock_wallet_client.cc
+++ b/components/autofill/content/browser/wallet/mock_wallet_client.cc
@@ -14,9 +14,15 @@ MockWalletClient::MockWalletClient(net::URLRequestContextGetter* context,
 
 MockWalletClient::~MockWalletClient() {}
 
-void MockWalletClient::SaveToWallet(scoped_ptr<wallet::Instrument> instrument,
-                                    scoped_ptr<wallet::Address> address) {
-  SaveToWalletMock(instrument.get(), address.get());
+void MockWalletClient::SaveToWallet(
+    scoped_ptr<Instrument> instrument,
+    scoped_ptr<Address> address,
+    const WalletItems::MaskedInstrument* reference_instrument,
+    const Address* reference_address) {
+  SaveToWalletMock(instrument.get(),
+                   address.get(),
+                   reference_instrument,
+                   reference_address);
 }
 
 }  // namespace wallet
diff --git a/components/autofill/content/browser/wallet/mock_wallet_client.h b/components/autofill/content/browser/wallet/mock_wallet_client.h
index b6751c0..629a9df 100644
--- a/components/autofill/content/browser/wallet/mock_wallet_client.h
+++ b/components/autofill/content/browser/wallet/mock_wallet_client.h
@@ -11,6 +11,7 @@
 #include "components/autofill/content/browser/wallet/instrument.h"
 #include "components/autofill/content/browser/wallet/wallet_address.h"
 #include "components/autofill/content/browser/wallet/wallet_client.h"
+#include "components/autofill/content/browser/wallet/wallet_items.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
 namespace autofill {
@@ -40,11 +41,17 @@ class MockWalletClient : public WalletClient {
 
   // Methods with scoped_ptrs can't be mocked but by using the implementation
   // below the same effect can be achieved.
-  virtual void SaveToWallet(scoped_ptr<wallet::Instrument> instrument,
-                            scoped_ptr<wallet::Address> address) OVERRIDE;
-
-  MOCK_METHOD2(SaveToWalletMock,
-               void(Instrument* instrument, Address* address));
+  virtual void SaveToWallet(
+      scoped_ptr<Instrument> instrument,
+      scoped_ptr<Address> address,
+      const WalletItems::MaskedInstrument* reference_instrument,
+      const Address* reference_address) OVERRIDE;
+
+  MOCK_METHOD4(SaveToWalletMock,
+      void(Instrument* instrument,
+           Address* address,
+           const WalletItems::MaskedInstrument* reference_instrument,
+           const Address* reference_address));
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockWalletClient);
diff --git a/components/autofill/content/browser/wallet/wallet_client.cc b/components/autofill/content/browser/wallet/wallet_client.cc
index b2bb40c..b0e45887 100644
--- a/components/autofill/content/browser/wallet/wallet_client.cc
+++ b/components/autofill/content/browser/wallet/wallet_client.cc
@@ -283,17 +283,6 @@ void WalletClient::AcceptLegalDocuments(
 void WalletClient::AuthenticateInstrument(
     const std::string& instrument_id,
     const std::string& card_verification_number) {
-  if (HasRequestInProgress()) {
-    pending_requests_.push(base::Bind(&WalletClient::AuthenticateInstrument,
-                                      base::Unretained(this),
-                                      instrument_id,
-                                      card_verification_number));
-    return;
-  }
-
-  DCHECK_EQ(NO_PENDING_REQUEST, request_type_);
-  request_type_ = AUTHENTICATE_INSTRUMENT;
-
   base::DictionaryValue request_dict;
   request_dict.SetString(kApiKeyKey, google_apis::GetAPIKey());
   request_dict.SetString(kRiskParamsKey, delegate_->GetRiskData());
@@ -312,20 +301,11 @@ void WalletClient::AuthenticateInstrument(
 
   MakeWalletRequest(GetAuthenticateInstrumentUrl(user_index_),
                     post_body,
-                    kFormEncodedMimeType);
+                    kFormEncodedMimeType,
+                    AUTHENTICATE_INSTRUMENT);
 }
 
 void WalletClient::GetFullWallet(const FullWalletRequest& full_wallet_request) {
-  if (HasRequestInProgress()) {
-    pending_requests_.push(base::Bind(&WalletClient::GetFullWallet,
-                                      base::Unretained(this),
-                                      full_wallet_request));
-    return;
-  }
-
-  DCHECK_EQ(NO_PENDING_REQUEST, request_type_);
-  request_type_ = GET_FULL_WALLET;
-
   base::DictionaryValue request_dict;
   request_dict.SetString(kApiKeyKey, google_apis::GetAPIKey());
   request_dict.SetString(kRiskParamsKey, delegate_->GetRiskData());
@@ -369,22 +349,16 @@ void WalletClient::GetFullWallet(const FullWalletRequest& full_wallet_request) {
 
   MakeWalletRequest(GetGetFullWalletUrl(user_index_),
                     post_body,
-                    kFormEncodedMimeType);
+                    kFormEncodedMimeType,
+                    GET_FULL_WALLET);
 }
 
-void WalletClient::SaveToWallet(scoped_ptr<Instrument> instrument,
-                                scoped_ptr<Address> address) {
+void WalletClient::SaveToWallet(
+    scoped_ptr<Instrument> instrument,
+    scoped_ptr<Address> address,
+    const WalletItems::MaskedInstrument* reference_instrument,
+    const Address* reference_address) {
   DCHECK(instrument || address);
-  if (HasRequestInProgress()) {
-    pending_requests_.push(base::Bind(&WalletClient::SaveToWallet,
-                                      base::Unretained(this),
-                                      base::Passed(&instrument),
-                                      base::Passed(&address)));
-    return;
-  }
-
-  DCHECK_EQ(NO_PENDING_REQUEST, request_type_);
-  request_type_ = SAVE_TO_WALLET;
 
   base::DictionaryValue request_dict;
   request_dict.SetString(kApiKeyKey, google_apis::GetAPIKey());
@@ -402,17 +376,23 @@ void WalletClient::SaveToWallet(scoped_ptr<Instrument> instrument,
     card_verification_number = net::EscapeUrlEncodedData(
         UTF16ToUTF8(instrument->card_verification_number()), true);
 
-    if (instrument->object_id().empty()) {
+    if (!reference_instrument) {
       request_dict.Set(kInstrumentKey, instrument->ToDictionary().release());
       request_dict.SetString(kInstrumentPhoneNumberKey,
                              instrument->address()->phone_number());
     } else {
-      DCHECK(instrument->address() ||
-             (instrument->expiration_month() > 0 &&
-              instrument->expiration_year() > 0));
+      DCHECK(!reference_instrument->object_id().empty());
+
+      int new_month = instrument->expiration_month();
+      int new_year = instrument->expiration_year();
+      bool expiration_date_changed =
+          new_month != reference_instrument->expiration_month() ||
+          new_year != reference_instrument->expiration_year();
+
+      DCHECK(instrument->address() || expiration_date_changed);
 
       request_dict.SetString(kUpgradedInstrumentIdKey,
-                             instrument->object_id());
+                             reference_instrument->object_id());
 
       if (instrument->address()) {
         request_dict.SetString(kInstrumentPhoneNumberKey,
@@ -422,8 +402,8 @@ void WalletClient::SaveToWallet(scoped_ptr<Instrument> instrument,
             instrument->address()->ToDictionaryWithoutID().release());
       }
 
-      if (instrument->expiration_month() > 0 &&
-          instrument->expiration_year() > 0) {
+      if (expiration_date_changed) {
+        // Updating expiration date requires a CVC.
         DCHECK(!instrument->card_verification_number().empty());
         request_dict.SetInteger(kInstrumentExpMonthKey,
                                 instrument->expiration_month());
@@ -436,6 +416,10 @@ void WalletClient::SaveToWallet(scoped_ptr<Instrument> instrument,
     }
   }
   if (address) {
+    if (reference_address) {
+      address->set_object_id(reference_address->object_id());
+      DCHECK(!address->object_id().empty());
+    }
     request_dict.Set(kShippingAddressKey,
                      address->ToDictionaryWithID().release());
   }
@@ -459,24 +443,17 @@ void WalletClient::SaveToWallet(scoped_ptr<Instrument> instrument,
     }
     MakeWalletRequest(GetSaveToWalletUrl(user_index_),
                       post_body,
-                      kFormEncodedMimeType);
+                      kFormEncodedMimeType,
+                      SAVE_TO_WALLET);
   } else {
     MakeWalletRequest(GetSaveToWalletNoEscrowUrl(user_index_),
                       json_payload,
-                      kJsonMimeType);
+                      kJsonMimeType,
+                      SAVE_TO_WALLET);
   }
 }
 
 void WalletClient::GetWalletItems() {
-  if (HasRequestInProgress()) {
-    pending_requests_.push(base::Bind(&WalletClient::GetWalletItems,
-                                      base::Unretained(this)));
-    return;
-  }
-
-  DCHECK_EQ(NO_PENDING_REQUEST, request_type_);
-  request_type_ = GET_WALLET_ITEMS;
-
   base::DictionaryValue request_dict;
   request_dict.SetString(kApiKeyKey, google_apis::GetAPIKey());
   request_dict.SetString(kMerchantDomainKey,
@@ -491,7 +468,8 @@ void WalletClient::GetWalletItems() {
 
   MakeWalletRequest(GetGetWalletItemsUrl(user_index_),
                     post_body,
-                    kJsonMimeType);
+                    kJsonMimeType,
+                    GET_WALLET_ITEMS);
 }
 
 bool WalletClient::HasRequestInProgress() const {
@@ -514,17 +492,6 @@ void WalletClient::SetUserIndex(size_t user_index) {
 void WalletClient::DoAcceptLegalDocuments(
     const std::vector<std::string>& document_ids,
     const std::string& google_transaction_id) {
-  if (HasRequestInProgress()) {
-    pending_requests_.push(base::Bind(&WalletClient::DoAcceptLegalDocuments,
-                                      base::Unretained(this),
-                                      document_ids,
-                                      google_transaction_id));
-    return;
-  }
-
-  DCHECK_EQ(NO_PENDING_REQUEST, request_type_);
-  request_type_ = ACCEPT_LEGAL_DOCUMENTS;
-
   base::DictionaryValue request_dict;
   request_dict.SetString(kApiKeyKey, google_apis::GetAPIKey());
   request_dict.SetString(kGoogleTransactionIdKey, google_transaction_id);
@@ -543,13 +510,26 @@ void WalletClient::DoAcceptLegalDocuments(
 
   MakeWalletRequest(GetAcceptLegalDocumentsUrl(user_index_),
                     post_body,
-                    kJsonMimeType);
+                    kJsonMimeType,
+                    ACCEPT_LEGAL_DOCUMENTS);
 }
 
 void WalletClient::MakeWalletRequest(const GURL& url,
                                      const std::string& post_body,
-                                     const std::string& mime_type) {
-  DCHECK(!HasRequestInProgress());
+                                     const std::string& mime_type,
+                                     RequestType request_type) {
+  if (HasRequestInProgress()) {
+    pending_requests_.push(base::Bind(&WalletClient::MakeWalletRequest,
+                                      base::Unretained(this),
+                                      url,
+                                      post_body,
+                                      mime_type,
+                                      request_type));
+    return;
+  }
+
+  DCHECK_EQ(request_type_, NO_PENDING_REQUEST);
+  request_type_ = request_type;
 
   request_.reset(net::URLFetcher::Create(
       0, url, net::URLFetcher::POST, this));
diff --git a/components/autofill/content/browser/wallet/wallet_client.h b/components/autofill/content/browser/wallet/wallet_client.h
index 54476f0..bfd4feb 100644
--- a/components/autofill/content/browser/wallet/wallet_client.h
+++ b/components/autofill/content/browser/wallet/wallet_client.h
@@ -165,10 +165,16 @@ class WalletClient : public net::URLFetcherDelegate {
   virtual void GetFullWallet(const FullWalletRequest& full_wallet_request);
 
   // Saves the data in |instrument| and/or |address| to Wallet. |instrument|
-  // does not have to be complete if its being used to update an existing
+  // does not have to be complete if it's being used to update an existing
   // instrument, like in the case of expiration date or address only updates.
-  virtual void SaveToWallet(scoped_ptr<Instrument> instrument,
-                            scoped_ptr<Address> address);
+  // |reference_instrument| and |reference_address| are the original instrument
+  // and address to be updated on the server (and should be NULL if |instrument|
+  // or |address| are new data).
+  virtual void SaveToWallet(
+      scoped_ptr<Instrument> instrument,
+      scoped_ptr<Address> address,
+      const WalletItems::MaskedInstrument* reference_instrument,
+      const Address* reference_address);
 
   bool HasRequestInProgress() const;
 
@@ -201,7 +207,8 @@ class WalletClient : public net::URLFetcherDelegate {
   // |delegate_| when the request is complete.
   void MakeWalletRequest(const GURL& url,
                          const std::string& post_body,
-                         const std::string& mime_type);
+                         const std::string& mime_type,
+                         RequestType request_type);
 
   // Performs bookkeeping tasks for any invalid requests.
   void HandleMalformedResponse(RequestType request_type,
diff --git a/components/autofill/content/browser/wallet/wallet_client_unittest.cc b/components/autofill/content/browser/wallet/wallet_client_unittest.cc
index 625edec..2d3aac4 100644
--- a/components/autofill/content/browser/wallet/wallet_client_unittest.cc
+++ b/components/autofill/content/browser/wallet/wallet_client_unittest.cc
@@ -492,7 +492,7 @@ const char kUpdateAddressValidRequest[] =
         "\"risk_params\":\"risky business\","
         "\"shipping_address\":"
         "{"
-            "\"id\":\"shipping_address_id\","
+            "\"id\":\"address_id\","
             "\"phone_number\":\"ship_phone_number\","
             "\"postal_address\":"
             "{"
@@ -530,7 +530,7 @@ const char kUpdateInstrumentAddressValidRequest[] =
             "\"postal_code_number\":\"postal_code_number\","
             "\"recipient_name\":\"recipient_name\""
         "},"
-        "\"upgraded_instrument_id\":\"instrument_id\","
+        "\"upgraded_instrument_id\":\"default_instrument_id\","
         "\"use_minimal_addresses\":false"
     "}";
 
@@ -553,7 +553,7 @@ const char kUpdateInstrumentAddressWithNameChangeValidRequest[] =
             "\"postal_code_number\":\"postal_code_number\","
             "\"recipient_name\":\"recipient_name\""
         "},"
-        "\"upgraded_instrument_id\":\"instrument_id\","
+        "\"upgraded_instrument_id\":\"default_instrument_id\","
         "\"use_minimal_addresses\":false"
     "}";
 
@@ -564,7 +564,7 @@ const char kUpdateInstrumentExpirationDateValidRequest[] =
             "\"credit_card\":"
             "{"
                 "\"exp_month\":12,"
-                "\"exp_year\":3000"
+                "\"exp_year\":3001"
             "},"
             "\"type\":\"CREDIT_CARD\""
         "},"
@@ -1194,7 +1194,9 @@ TEST_F(WalletClientTest, SaveAddressSucceeded) {
 
   scoped_ptr<Address> address = GetTestSaveableAddress();
   wallet_client_->SaveToWallet(scoped_ptr<Instrument>(),
-                               address.Pass());
+                               address.Pass(),
+                               NULL,
+                               NULL);
   VerifyAndFinishRequest(net::HTTP_OK,
                          kSaveAddressValidRequest,
                          kSaveAddressValidResponse);
@@ -1224,7 +1226,9 @@ TEST_F(WalletClientTest, SaveAddressWithRequiredActionsSucceeded) {
 
   scoped_ptr<Address> address = GetTestSaveableAddress();
   wallet_client_->SaveToWallet(scoped_ptr<Instrument>(),
-                               address.Pass());
+                               address.Pass(),
+                               NULL,
+                               NULL);
   VerifyAndFinishRequest(net::HTTP_OK,
                          kSaveAddressValidRequest,
                          kSaveAddressWithRequiredActionsValidResponse);
@@ -1240,7 +1244,9 @@ TEST_F(WalletClientTest, SaveAddressFailedInvalidRequiredAction) {
 
   scoped_ptr<Address> address = GetTestSaveableAddress();
   wallet_client_->SaveToWallet(scoped_ptr<Instrument>(),
-                               address.Pass());
+                               address.Pass(),
+                               NULL,
+                               NULL);
   VerifyAndFinishRequest(net::HTTP_OK,
                          kSaveAddressValidRequest,
                          kSaveWithInvalidRequiredActionsResponse);
@@ -1256,7 +1262,9 @@ TEST_F(WalletClientTest, SaveAddressFailedMalformedResponse) {
 
   scoped_ptr<Address> address = GetTestSaveableAddress();
   wallet_client_->SaveToWallet(scoped_ptr<Instrument>(),
-                               address.Pass());
+                               address.Pass(),
+                               NULL,
+                               NULL);
   VerifyAndFinishRequest(net::HTTP_OK,
                          kSaveAddressValidRequest,
                          kSaveInvalidResponse);
@@ -1273,7 +1281,9 @@ TEST_F(WalletClientTest, SaveInstrumentSucceeded) {
 
   scoped_ptr<Instrument> instrument = GetTestInstrument();
   wallet_client_->SaveToWallet(instrument.Pass(),
-                               scoped_ptr<Address>());
+                               scoped_ptr<Address>(),
+                               NULL,
+                               NULL);
 
   VerifyAndFinishFormEncodedRequest(net::HTTP_OK,
                                     kSaveInstrumentValidRequest,
@@ -1305,7 +1315,9 @@ TEST_F(WalletClientTest, SaveInstrumentWithRequiredActionsSucceeded) {
 
   scoped_ptr<Instrument> instrument = GetTestInstrument();
   wallet_client_->SaveToWallet(instrument.Pass(),
-                               scoped_ptr<Address>());
+                               scoped_ptr<Address>(),
+                               NULL,
+                               NULL);
 
   VerifyAndFinishFormEncodedRequest(
       net::HTTP_OK,
@@ -1325,7 +1337,9 @@ TEST_F(WalletClientTest, SaveInstrumentFailedInvalidRequiredActions) {
 
   scoped_ptr<Instrument> instrument = GetTestInstrument();
   wallet_client_->SaveToWallet(instrument.Pass(),
-                               scoped_ptr<Address>());
+                               scoped_ptr<Address>(),
+                               NULL,
+                               NULL);
 
   VerifyAndFinishFormEncodedRequest(net::HTTP_OK,
                                     kSaveInstrumentValidRequest,
@@ -1343,7 +1357,9 @@ TEST_F(WalletClientTest, SaveInstrumentFailedMalformedResponse) {
 
   scoped_ptr<Instrument> instrument = GetTestInstrument();
   wallet_client_->SaveToWallet(instrument.Pass(),
-                               scoped_ptr<Address>());
+                               scoped_ptr<Address>(),
+                               NULL,
+                               NULL);
 
   VerifyAndFinishFormEncodedRequest(net::HTTP_OK,
                                     kSaveInstrumentValidRequest,
@@ -1364,8 +1380,7 @@ TEST_F(WalletClientTest, SaveInstrumentAndAddressSucceeded) {
 
   scoped_ptr<Instrument> instrument = GetTestInstrument();
   scoped_ptr<Address> address = GetTestSaveableAddress();
-  wallet_client_->SaveToWallet(instrument.Pass(),
-                               address.Pass());
+  wallet_client_->SaveToWallet(instrument.Pass(), address.Pass(), NULL, NULL);
 
   VerifyAndFinishFormEncodedRequest(net::HTTP_OK,
                                     kSaveInstrumentAndAddressValidRequest,
@@ -1399,8 +1414,7 @@ TEST_F(WalletClientTest, SaveInstrumentAndAddressWithRequiredActionsSucceeded) {
 
   scoped_ptr<Instrument> instrument = GetTestInstrument();
   scoped_ptr<Address> address = GetTestSaveableAddress();
-  wallet_client_->SaveToWallet(instrument.Pass(),
-                               address.Pass());
+  wallet_client_->SaveToWallet(instrument.Pass(), address.Pass(), NULL, NULL);
 
   VerifyAndFinishFormEncodedRequest(
       net::HTTP_OK,
@@ -1421,8 +1435,7 @@ TEST_F(WalletClientTest, SaveInstrumentAndAddressFailedInvalidRequiredAction) {
 
   scoped_ptr<Instrument> instrument = GetTestInstrument();
   scoped_ptr<Address> address = GetTestSaveableAddress();
-  wallet_client_->SaveToWallet(instrument.Pass(),
-                               address.Pass());
+  wallet_client_->SaveToWallet(instrument.Pass(), address.Pass(), NULL, NULL);
 
   VerifyAndFinishFormEncodedRequest(net::HTTP_OK,
                                     kSaveInstrumentAndAddressValidRequest,
@@ -1439,11 +1452,12 @@ TEST_F(WalletClientTest, UpdateAddressSucceeded) {
   delegate_.ExpectLogWalletApiCallDuration(AutofillMetrics::SAVE_TO_WALLET, 1);
   delegate_.ExpectBaselineMetrics();
 
-  scoped_ptr<Address> address = GetTestShippingAddress();
-  address->set_object_id("shipping_address_id");
-
+  scoped_ptr<Address> reference_address = GetTestNonDefaultShippingAddress();
   wallet_client_->SaveToWallet(scoped_ptr<Instrument>(),
-                               address.Pass());
+                               GetTestShippingAddress(),
+                               NULL,
+                               reference_address.get());
+
   VerifyAndFinishRequest(net::HTTP_OK,
                          kUpdateAddressValidRequest,
                          kUpdateAddressValidResponse);
@@ -1470,11 +1484,12 @@ TEST_F(WalletClientTest, UpdateAddressWithRequiredActionsSucceeded) {
                                            required_actions,
                                            form_errors)).Times(1);
 
-  scoped_ptr<Address> address = GetTestShippingAddress();
-  address->set_object_id("shipping_address_id");
-
+  scoped_ptr<Address> reference_address = GetTestNonDefaultShippingAddress();
   wallet_client_->SaveToWallet(scoped_ptr<Instrument>(),
-                               address.Pass());
+                               GetTestShippingAddress(),
+                               NULL,
+                               reference_address.get());
+
   VerifyAndFinishRequest(net::HTTP_OK,
                          kUpdateAddressValidRequest,
                          kUpdateWithRequiredActionsValidResponse);
@@ -1488,11 +1503,12 @@ TEST_F(WalletClientTest, UpdateAddressFailedInvalidRequiredAction) {
   delegate_.ExpectWalletErrorMetric(AutofillMetrics::WALLET_MALFORMED_RESPONSE);
   delegate_.ExpectLogWalletMalformedResponse(AutofillMetrics::SAVE_TO_WALLET);
 
-  scoped_ptr<Address> address = GetTestShippingAddress();
-  address->set_object_id("shipping_address_id");
-
+  scoped_ptr<Address> reference_address = GetTestNonDefaultShippingAddress();
   wallet_client_->SaveToWallet(scoped_ptr<Instrument>(),
-                               address.Pass());
+                               GetTestShippingAddress(),
+                               NULL,
+                               reference_address.get());
+
   VerifyAndFinishRequest(net::HTTP_OK,
                          kUpdateAddressValidRequest,
                          kSaveWithInvalidRequiredActionsResponse);
@@ -1506,11 +1522,12 @@ TEST_F(WalletClientTest, UpdateAddressMalformedResponse) {
   delegate_.ExpectWalletErrorMetric(AutofillMetrics::WALLET_MALFORMED_RESPONSE);
   delegate_.ExpectLogWalletMalformedResponse(AutofillMetrics::SAVE_TO_WALLET);
 
-  scoped_ptr<Address> address = GetTestShippingAddress();
-  address->set_object_id("shipping_address_id");
-
+  scoped_ptr<Address> reference_address = GetTestNonDefaultShippingAddress();
   wallet_client_->SaveToWallet(scoped_ptr<Instrument>(),
-                               address.Pass());
+                               GetTestShippingAddress(),
+                               NULL,
+                               reference_address.get());
+
   VerifyAndFinishRequest(net::HTTP_OK,
                          kUpdateAddressValidRequest,
                          kUpdateMalformedResponse);
@@ -1526,8 +1543,12 @@ TEST_F(WalletClientTest, UpdateInstrumentAddressSucceeded) {
                                            1);
   delegate_.ExpectBaselineMetrics();
 
+  scoped_ptr<WalletItems::MaskedInstrument> reference_instrument =
+      GetTestMaskedInstrument();
   wallet_client_->SaveToWallet(GetTestAddressUpgradeInstrument(),
-                               scoped_ptr<Address>());
+                               scoped_ptr<Address>(),
+                               reference_instrument.get(),
+                               NULL);
 
   VerifyAndFinishRequest(net::HTTP_OK,
                          kUpdateInstrumentAddressValidRequest,
@@ -1544,8 +1565,19 @@ TEST_F(WalletClientTest, UpdateInstrumentExpirationDateSuceeded) {
                                            1);
   delegate_.ExpectBaselineMetrics();
 
-  wallet_client_->SaveToWallet(GetTestExpirationDateChangeInstrument(),
-                               scoped_ptr<Address>());
+  scoped_ptr<Instrument> instrument = GetTestExpirationDateChangeInstrument();
+  scoped_ptr<WalletItems::MaskedInstrument> reference_instrument =
+      GetTestMaskedInstrumentWithId("instrument_id");
+
+  int new_month = instrument->expiration_month();
+  int new_year = instrument->expiration_year();
+  ASSERT_TRUE(new_month != reference_instrument->expiration_month() ||
+              new_year != reference_instrument->expiration_year());
+
+  wallet_client_->SaveToWallet(instrument.Pass(),
+                               scoped_ptr<Address>(),
+                               reference_instrument.get(),
+                               NULL);
 
   VerifyAndFinishFormEncodedRequest(net::HTTP_OK,
                                     kUpdateInstrumentExpirationDateValidRequest,
@@ -1563,8 +1595,12 @@ TEST_F(WalletClientTest, UpdateInstrumentAddressWithNameChangeSucceeded) {
                                            1);
   delegate_.ExpectBaselineMetrics();
 
+  scoped_ptr<WalletItems::MaskedInstrument> reference_instrument =
+      GetTestMaskedInstrument();
   wallet_client_->SaveToWallet(GetTestAddressNameChangeInstrument(),
-                               scoped_ptr<Address>());
+                               scoped_ptr<Address>(),
+                               reference_instrument.get(),
+                               NULL);
 
   VerifyAndFinishFormEncodedRequest(
       net::HTTP_OK,
@@ -1596,8 +1632,12 @@ TEST_F(WalletClientTest, UpdateInstrumentWithRequiredActionsSucceeded) {
                                 required_actions,
                                 form_errors)).Times(1);
 
+  scoped_ptr<WalletItems::MaskedInstrument> reference_instrument =
+      GetTestMaskedInstrument();
   wallet_client_->SaveToWallet(GetTestAddressUpgradeInstrument(),
-                               scoped_ptr<Address>());
+                               scoped_ptr<Address>(),
+                               reference_instrument.get(),
+                               NULL);
 
   VerifyAndFinishRequest(net::HTTP_OK,
                          kUpdateInstrumentAddressValidRequest,
@@ -1613,8 +1653,12 @@ TEST_F(WalletClientTest, UpdateInstrumentFailedInvalidRequiredAction) {
   delegate_.ExpectWalletErrorMetric(AutofillMetrics::WALLET_MALFORMED_RESPONSE);
   delegate_.ExpectLogWalletMalformedResponse(AutofillMetrics::SAVE_TO_WALLET);
 
+  scoped_ptr<WalletItems::MaskedInstrument> reference_instrument =
+      GetTestMaskedInstrument();
   wallet_client_->SaveToWallet(GetTestAddressUpgradeInstrument(),
-                               scoped_ptr<Address>());
+                               scoped_ptr<Address>(),
+                               reference_instrument.get(),
+                               NULL);
 
   VerifyAndFinishRequest(net::HTTP_OK,
                          kUpdateInstrumentAddressValidRequest,
@@ -1630,8 +1674,12 @@ TEST_F(WalletClientTest, UpdateInstrumentMalformedResponse) {
   delegate_.ExpectWalletErrorMetric(AutofillMetrics::WALLET_MALFORMED_RESPONSE);
   delegate_.ExpectLogWalletMalformedResponse(AutofillMetrics::SAVE_TO_WALLET);
 
+  scoped_ptr<WalletItems::MaskedInstrument> reference_instrument =
+      GetTestMaskedInstrument();
   wallet_client_->SaveToWallet(GetTestAddressUpgradeInstrument(),
-                               scoped_ptr<Address>());
+                               scoped_ptr<Address>(),
+                               reference_instrument.get(),
+                               NULL);
 
   VerifyAndFinishRequest(net::HTTP_OK,
                          kUpdateInstrumentAddressValidRequest,
diff --git a/components/autofill/content/browser/wallet/wallet_test_util.cc b/components/autofill/content/browser/wallet/wallet_test_util.cc
index 9ded530..939e217 100644
--- a/components/autofill/content/browser/wallet/wallet_test_util.cc
+++ b/components/autofill/content/browser/wallet/wallet_test_util.cc
@@ -34,7 +34,7 @@ scoped_ptr<WalletItems::MaskedInstrument> GetTestMaskedInstrumentWithDetails(
     scoped_ptr<Address> address,
     WalletItems::MaskedInstrument::Type type,
     WalletItems::MaskedInstrument::Status status) {
-  return scoped_ptr<WalletItems::MaskedInstrument>(
+  return make_scoped_ptr(
       new WalletItems::MaskedInstrument(ASCIIToUTF16("descriptive_name"),
                                         type,
                                         std::vector<base::string16>(),
@@ -66,20 +66,22 @@ GetTestMaskedInstrumentWithIdAndAddress(
 }
 
 scoped_ptr<GaiaAccount> GetTestGaiaAccount() {
-  return scoped_ptr<GaiaAccount>(GaiaAccount::CreateForTesting(
-      "user@chromium.org", "obfuscated_id", 0, true));
+  return GaiaAccount::CreateForTesting("user@chromium.org",
+                                       "obfuscated_id",
+                                       0,
+                                       true);
 }
 
 scoped_ptr<Address> GetTestAddress() {
-  return scoped_ptr<Address>(new Address("US",
-                                         ASCIIToUTF16("recipient_name"),
-                                         ASCIIToUTF16("address_line_1"),
-                                         ASCIIToUTF16("address_line_2"),
-                                         ASCIIToUTF16("locality_name"),
-                                         ASCIIToUTF16("admin_area_name"),
-                                         ASCIIToUTF16("postal_code_number"),
-                                         ASCIIToUTF16("phone_number"),
-                                         std::string()));
+  return make_scoped_ptr(new Address("US",
+                                     ASCIIToUTF16("recipient_name"),
+                                     ASCIIToUTF16("address_line_1"),
+                                     ASCIIToUTF16("address_line_2"),
+                                     ASCIIToUTF16("locality_name"),
+                                     ASCIIToUTF16("admin_area_name"),
+                                     ASCIIToUTF16("postal_code_number"),
+                                     ASCIIToUTF16("phone_number"),
+                                     std::string()));
 }
 
 scoped_ptr<Address> GetTestMinimalAddress() {
@@ -122,45 +124,39 @@ scoped_ptr<FullWallet> GetTestFullWalletInstrumentOnly() {
 }
 
 scoped_ptr<Instrument> GetTestInstrument() {
-  return scoped_ptr<Instrument>(new Instrument(ASCIIToUTF16("4444444444444448"),
-                                               ASCIIToUTF16("123"),
-                                               12,
-                                               FutureYear(),
-                                               Instrument::VISA,
-                                               GetTestAddress()));
+  return make_scoped_ptr(new Instrument(ASCIIToUTF16("4444444444444448"),
+                                        ASCIIToUTF16("123"),
+                                        12,
+                                        FutureYear(),
+                                        Instrument::VISA,
+                                        GetTestAddress()));
 }
 
 scoped_ptr<Instrument> GetTestAddressUpgradeInstrument() {
-  scoped_ptr<Instrument> instrument(new Instrument(base::string16(),
-                                                   base::string16(),
-                                                   0,
-                                                   0,
-                                                   Instrument::UNKNOWN,
-                                                   GetTestAddress()));
-  instrument->set_object_id("instrument_id");
-  return instrument.Pass();
+  return make_scoped_ptr(new Instrument(base::string16(),
+                                        base::string16(),
+                                        12,
+                                        FutureYear(),
+                                        Instrument::UNKNOWN,
+                                        GetTestAddress()));
 }
 
 scoped_ptr<Instrument> GetTestExpirationDateChangeInstrument() {
-  scoped_ptr<Instrument> instrument(new Instrument(base::string16(),
-                                                   ASCIIToUTF16("123"),
-                                                   12,
-                                                   FutureYear(),
-                                                   Instrument::UNKNOWN,
-                                                   scoped_ptr<Address>()));
-  instrument->set_object_id("instrument_id");
-  return instrument.Pass();
+  return make_scoped_ptr(new Instrument(base::string16(),
+                                        ASCIIToUTF16("123"),
+                                        12,
+                                        FutureYear() + 1,
+                                        Instrument::UNKNOWN,
+                                        scoped_ptr<Address>()));
 }
 
 scoped_ptr<Instrument> GetTestAddressNameChangeInstrument() {
-  scoped_ptr<Instrument> instrument(new Instrument(base::string16(),
-                                                   ASCIIToUTF16("123"),
-                                                   0,
-                                                   0,
-                                                   Instrument::UNKNOWN,
-                                                   GetTestAddress()));
-  instrument->set_object_id("instrument_id");
-  return instrument.Pass();
+  return make_scoped_ptr(new Instrument(base::string16(),
+                                        ASCIIToUTF16("123"),
+                                        12,
+                                        FutureYear(),
+                                        Instrument::UNKNOWN,
+                                        GetTestAddress()));
 }
 
 scoped_ptr<WalletItems::LegalDocument> GetTestLegalDocument() {
@@ -196,9 +192,9 @@ scoped_ptr<WalletItems::MaskedInstrument> GetTestMaskedInstrumentAmex(
       "default_instrument_id",
       GetTestAddress(),
       WalletItems::MaskedInstrument::AMEX,
-      (amex_permission == AMEX_ALLOWED)
-         ? WalletItems::MaskedInstrument::VALID
-         : WalletItems::MaskedInstrument::AMEX_NOT_SUPPORTED);
+      amex_permission == AMEX_ALLOWED ?
+          WalletItems::MaskedInstrument::VALID :
+          WalletItems::MaskedInstrument::AMEX_NOT_SUPPORTED);
 }
 
 scoped_ptr<WalletItems::MaskedInstrument> GetTestNonDefaultMaskedInstrument() {
@@ -206,29 +202,27 @@ scoped_ptr<WalletItems::MaskedInstrument> GetTestNonDefaultMaskedInstrument() {
 }
 
 scoped_ptr<Address> GetTestSaveableAddress() {
-  return scoped_ptr<Address>(new Address(
-      "US",
-      ASCIIToUTF16("save_recipient_name"),
-      ASCIIToUTF16("save_address_line_1"),
-      ASCIIToUTF16("save_address_line_2"),
-      ASCIIToUTF16("save_locality_name"),
-      ASCIIToUTF16("save_admin_area_name"),
-      ASCIIToUTF16("save_postal_code_number"),
-      ASCIIToUTF16("save_phone_number"),
-      std::string()));
+  return make_scoped_ptr(new Address("US",
+                                     ASCIIToUTF16("save_recipient_name"),
+                                     ASCIIToUTF16("save_address_line_1"),
+                                     ASCIIToUTF16("save_address_line_2"),
+                                     ASCIIToUTF16("save_locality_name"),
+                                     ASCIIToUTF16("save_admin_area_name"),
+                                     ASCIIToUTF16("save_postal_code_number"),
+                                     ASCIIToUTF16("save_phone_number"),
+                                     std::string()));
 }
 
 scoped_ptr<Address> GetTestShippingAddress() {
-  return scoped_ptr<Address>(new Address(
-      "US",
-      ASCIIToUTF16("ship_recipient_name"),
-      ASCIIToUTF16("ship_address_line_1"),
-      ASCIIToUTF16("ship_address_line_2"),
-      ASCIIToUTF16("ship_locality_name"),
-      ASCIIToUTF16("ship_admin_area_name"),
-      ASCIIToUTF16("ship_postal_code_number"),
-      ASCIIToUTF16("ship_phone_number"),
-      "default_address_id"));
+  return make_scoped_ptr(new Address("US",
+                                     ASCIIToUTF16("ship_recipient_name"),
+                                     ASCIIToUTF16("ship_address_line_1"),
+                                     ASCIIToUTF16("ship_address_line_2"),
+                                     ASCIIToUTF16("ship_locality_name"),
+                                     ASCIIToUTF16("ship_admin_area_name"),
+                                     ASCIIToUTF16("ship_postal_code_number"),
+                                     ASCIIToUTF16("ship_phone_number"),
+                                     "default_address_id"));
 }
 
 scoped_ptr<Address> GetTestNonDefaultShippingAddress() {
@@ -242,12 +236,11 @@ scoped_ptr<WalletItems> GetTestWalletItemsWithDetails(
     const std::string& default_instrument_id,
     const std::string& default_address_id,
     AmexPermission amex_permission) {
-  return scoped_ptr<WalletItems>(
-      new wallet::WalletItems(required_actions,
-                              "google_transaction_id",
-                              default_instrument_id,
-                              default_address_id,
-                              amex_permission));
+  return make_scoped_ptr(new wallet::WalletItems(required_actions,
+                                                 "google_transaction_id",
+                                                 default_instrument_id,
+                                                 default_address_id,
+                                                 amex_permission));
 }
 
 scoped_ptr<WalletItems> GetTestWalletItemsWithRequiredAction(