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

components/autofill/content/browser/wallet/wallet_client.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/autofill/content/browser/wallet/wallet_client.h"
 
 #include "base/bind.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/logging.h"
@@ skipping 354 matching lines @@
       kGetFullWalletRequestFormat,
       net::EscapeUrlEncodedData(json_payload, true).c_str(),
       base::HexEncode(&num_bits, 1).c_str(),
       base::HexEncode(&(one_time_pad_[0]), one_time_pad_.size()).c_str());
 
   MakeWalletRequest(GetGetFullWalletUrl(user_index_),
                     post_body,
                     kFormEncodedMimeType);
 }
 
-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* server_instrument,
+    const Address* server_address) {
   DCHECK(instrument || address);
   if (HasRequestInProgress()) {
     pending_requests_.push(base::Bind(&WalletClient::SaveToWallet,
                                       base::Unretained(this),
                                       base::Passed(&instrument),
-                                      base::Passed(&address)));
+                                      base::Passed(&address),
+                                      base::Unretained(server_instrument),

[Evan Stade, 2013/12/12 02:33:57]

this is no bueno... you need to make a copy (add a copy constructor or factory
fn)

option 2: split this function L458, do everything you need to do to get the
request ready, but don't actually send the request if there are other pending
requests in the queue (defer that till later).

[Dan Beam, 2013/12/13 04:06:24]

did option 2 and updated all the other methods

+                                      base::Unretained(server_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());
   request_dict.SetString(kRiskParamsKey, delegate_->GetRiskData());
   request_dict.SetString(kMerchantDomainKey,
                          source_url_.GetWithEmptyPath().spec());
   request_dict.SetBoolean(kUseMinimalAddresses, false);
   request_dict.SetBoolean(kPhoneNumberRequired, true);
 
   std::string primary_account_number;
   std::string card_verification_number;
   if (instrument) {
     primary_account_number = net::EscapeUrlEncodedData(
         UTF16ToUTF8(instrument->primary_account_number()), true);
     card_verification_number = net::EscapeUrlEncodedData(
         UTF16ToUTF8(instrument->card_verification_number()), true);
 
-    if (instrument->object_id().empty()) {
+    if (!server_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(!server_instrument->object_id().empty());
+
+      int new_month = instrument->expiration_month();
+      int new_year = instrument->expiration_year();
+      bool expiration_differs_from_server =

[Evan Stade, 2013/12/12 02:33:57]

nit: expiration_date_changed

[Dan Beam, 2013/12/13 04:06:24]

Done.

+          new_month != server_instrument->expiration_month() ||
+          new_year != server_instrument->expiration_year();
+
+      DCHECK(instrument->address() || expiration_differs_from_server);
 
       request_dict.SetString(kUpgradedInstrumentIdKey,
-                             instrument->object_id());
+                             server_instrument->object_id());
 
       if (instrument->address()) {
         request_dict.SetString(kInstrumentPhoneNumberKey,
                                instrument->address()->phone_number());
         request_dict.Set(
             kUpgradedBillingAddressKey,
             instrument->address()->ToDictionaryWithoutID().release());
       }
 
-      if (instrument->expiration_month() > 0 &&
-          instrument->expiration_year() > 0) {
+      if (expiration_differs_from_server) {
+        // Updating expiration date requires a CVC.
         DCHECK(!instrument->card_verification_number().empty());
         request_dict.SetInteger(kInstrumentExpMonthKey,
                                 instrument->expiration_month());
         request_dict.SetInteger(kInstrumentExpYearKey,
                                 instrument->expiration_year());
       }
 
       if (request_dict.HasKey(kInstrumentKey))
         request_dict.SetString(kInstrumentType, "CREDIT_CARD");
     }
   }
   if (address) {
+    if (server_address) {
+      address->set_object_id(server_address->object_id());
+      DCHECK(!address->object_id().empty());
+    }
     request_dict.Set(kShippingAddressKey,
                      address->ToDictionaryWithID().release());
   }
 
   std::string json_payload;
   base::JSONWriter::Write(&request_dict, &json_payload);
 
   if (!card_verification_number.empty()) {
     std::string post_body;
     if (!primary_account_number.empty()) {
@@ skipping 372 matching lines @@
       NOTREACHED();
       return AutofillMetrics::UNKNOWN_API_CALL;
   }
 
   NOTREACHED();
   return AutofillMetrics::UNKNOWN_API_CALL;
 }
 
 }  // namespace wallet
 }  // namespace autofill