Change WalletItems::LegalDocument a little.

make it possible to have a legal document that the server doesn't know about (so we can put Privacy Policy document logic with other document logic).

BUG=168704


Review URL: https://chromiumcodereview.appspot.com/12892003

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

6 files changed, 184 insertions, 91 deletions

diff --git a/components/autofill/browser/wallet/wallet_client.cc b/components/autofill/browser/wallet/wallet_client.cc
index 6104da3..ce30cd6 100644
--- a/components/autofill/browser/wallet/wallet_client.cc
+++ b/components/autofill/browser/wallet/wallet_client.cc
@@ -233,38 +233,17 @@ WalletClient::WalletClient(net::URLRequestContextGetter* context_getter,
 WalletClient::~WalletClient() {}
 
 void WalletClient::AcceptLegalDocuments(
-    const std::vector<std::string>& document_ids,
+    const std::vector<WalletItems::LegalDocument*>& documents,
     const std::string& google_transaction_id,
     const GURL& source_url) {
-  if (HasRequestInProgress()) {
-    pending_requests_.push(base::Bind(&WalletClient::AcceptLegalDocuments,
-                                      base::Unretained(this),
-                                      document_ids,
-                                      google_transaction_id,
-                                      source_url));
+  if (documents.empty())
     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);
-  request_dict.SetString(kMerchantDomainKey,
-                         source_url.GetWithEmptyPath().spec());
-  scoped_ptr<base::ListValue> docs_list(new base::ListValue());
-  for (std::vector<std::string>::const_iterator it = document_ids.begin();
-       it != document_ids.end();
-       ++it) {
-    docs_list->AppendString(*it);
+  std::vector<std::string> document_ids;
+  for (size_t i = 0; i < documents.size(); ++i) {
+    document_ids.push_back(documents[i]->id());
   }
-  request_dict.Set(kAcceptedLegalDocumentKey, docs_list.release());
-
-  std::string post_body;
-  base::JSONWriter::Write(&request_dict, &post_body);
-
-  MakeWalletRequest(GetAcceptLegalDocumentsUrl(), post_body);
+  DoAcceptLegalDocuments(document_ids, google_transaction_id, source_url);
 }
 
 void WalletClient::AuthenticateInstrument(
@@ -569,6 +548,41 @@ void WalletClient::CancelPendingRequests() {
   }
 }
 
+void WalletClient::DoAcceptLegalDocuments(
+    const std::vector<std::string>& document_ids,
+    const std::string& google_transaction_id,
+    const GURL& source_url) {
+  if (HasRequestInProgress()) {
+    pending_requests_.push(base::Bind(&WalletClient::DoAcceptLegalDocuments,
+                                      base::Unretained(this),
+                                      document_ids,
+                                      google_transaction_id,
+                                      source_url));
+    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);
+  request_dict.SetString(kMerchantDomainKey,
+                         source_url.GetWithEmptyPath().spec());
+  scoped_ptr<base::ListValue> docs_list(new base::ListValue());
+  for (std::vector<std::string>::const_iterator it = document_ids.begin();
+       it != document_ids.end(); ++it) {
+    if (!it->empty())
+      docs_list->AppendString(*it);
+  }
+  request_dict.Set(kAcceptedLegalDocumentKey, docs_list.release());
+
+  std::string post_body;
+  base::JSONWriter::Write(&request_dict, &post_body);
+
+  MakeWalletRequest(GetAcceptLegalDocumentsUrl(), post_body);
+}
+
 void WalletClient::MakeWalletRequest(const GURL& url,
                                      const std::string& post_body) {
   DCHECK(!HasRequestInProgress());
diff --git a/components/autofill/browser/wallet/wallet_client.h b/components/autofill/browser/wallet/wallet_client.h
index f12d82b..1fa291f 100644
--- a/components/autofill/browser/wallet/wallet_client.h
+++ b/components/autofill/browser/wallet/wallet_client.h
@@ -17,6 +17,7 @@
 #include "components/autofill/browser/wallet/encryption_escrow_client.h"
 #include "components/autofill/browser/wallet/encryption_escrow_client_observer.h"
 #include "components/autofill/browser/wallet/full_wallet.h"
+#include "components/autofill/browser/wallet/wallet_items.h"
 #include "components/autofill/common/autocheckout_status.h"
 #include "googleurl/src/gurl.h"
 #include "net/url_request/url_fetcher_delegate.h"
@@ -34,7 +35,6 @@ class Address;
 class FullWallet;
 class Instrument;
 class WalletClientDelegate;
-class WalletItems;
 
 // WalletClient is responsible for making calls to the Online Wallet backend on
 // the user's behalf. The normal flow for using this class is as follows:
@@ -145,10 +145,10 @@ class WalletClient
 
   // The GetWalletItems call to the Online Wallet backend may require the user
   // to accept various legal documents before a FullWallet can be generated.
-  // The |document_ids| and |google_transaction_id| are provided in the response
-  // to the GetWalletItems call.
+  // The |google_transaction_id| is provided in the response to the
+  // GetWalletItems call.
   virtual void AcceptLegalDocuments(
-      const std::vector<std::string>& document_ids,
+      const std::vector<WalletItems::LegalDocument*>& documents,
       const std::string& google_transaction_id,
       const GURL& source_url);
 
@@ -222,6 +222,12 @@ class WalletClient
     UPDATE_INSTRUMENT,
   };
 
+  // Like AcceptLegalDocuments, but takes a vector of document ids.
+  void DoAcceptLegalDocuments(
+      const std::vector<std::string>& document_ids,
+      const std::string& google_transaction_id,
+      const GURL& source_url);
+
   // Posts |post_body| to |url| and notifies |observer| when the request is
   // complete.
   void MakeWalletRequest(const GURL& url, const std::string& post_body);
diff --git a/components/autofill/browser/wallet/wallet_client_unittest.cc b/components/autofill/browser/wallet/wallet_client_unittest.cc
index 41546af..267ef5d 100644
--- a/components/autofill/browser/wallet/wallet_client_unittest.cc
+++ b/components/autofill/browser/wallet/wallet_client_unittest.cc
@@ -278,8 +278,8 @@ const char kAcceptLegalDocumentsValidRequest[] =
     "{"
         "\"accepted_legal_document\":"
         "["
-            "\"doc_1\","
-            "\"doc_2\""
+            "\"doc_id_1\","
+            "\"doc_id_2\""
         "],"
         "\"google_transaction_id\":\"google-transaction-id\","
         "\"merchant_domain\":\"https://example.com/\""
@@ -471,7 +471,6 @@ const char kSendAutocheckoutStatusOfFailureValidRequest[] =
         "\"success\":false"
     "}";
 
-
 const char kUpdateAddressValidRequest[] =
     "{"
         "\"merchant_domain\":\"https://example.com/\","
@@ -820,10 +819,19 @@ TEST_F(WalletClientTest, GetFullWalletMalformedResponse) {
 TEST_F(WalletClientTest, AcceptLegalDocuments) {
   EXPECT_CALL(delegate_, OnDidAcceptLegalDocuments()).Times(1);
 
-  std::vector<std::string> doc_ids;
-  doc_ids.push_back("doc_1");
-  doc_ids.push_back("doc_2");
-  wallet_client_->AcceptLegalDocuments(doc_ids,
+  ScopedVector<WalletItems::LegalDocument> docs;
+  base::DictionaryValue document;
+  document.SetString("legal_document_id", "doc_id_1");
+  document.SetString("display_name", "doc_1");
+  docs.push_back(
+      WalletItems::LegalDocument::CreateLegalDocument(document).release());
+  document.SetString("legal_document_id", "doc_id_2");
+  document.SetString("display_name", "doc_2");
+  docs.push_back(
+      WalletItems::LegalDocument::CreateLegalDocument(document).release());
+  docs.push_back(
+      WalletItems::LegalDocument::CreatePrivacyPolicyDocument().release());
+  wallet_client_->AcceptLegalDocuments(docs.get(),
                                        kGoogleTransactionId,
                                        GURL(kMerchantUrl));
   net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0);
diff --git a/components/autofill/browser/wallet/wallet_items.cc b/components/autofill/browser/wallet/wallet_items.cc
index 405ffd4..19f2daa 100644
--- a/components/autofill/browser/wallet/wallet_items.cc
+++ b/components/autofill/browser/wallet/wallet_items.cc
@@ -9,7 +9,9 @@
 #include "base/values.h"
 #include "components/autofill/browser/autofill_type.h"
 #include "googleurl/src/gurl.h"
+#include "grit/generated_resources.h"
 #include "grit/webkit_resources.h"
+#include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/image/image.h"
 
@@ -20,6 +22,21 @@ namespace {
 
 const char kLegalDocumentUrl[] =
     "https://wallet.google.com/legaldocument?docId=";
+const char kPrivacyNoticeUrl[] = "https://wallet.google.com/files/privacy.html";
+
+// TODO(estade): move to base/.
+template<class T>
+bool VectorsAreEqual(const std::vector<T*>& a, const std::vector<T*>& b) {
+  if (a.size() != b.size())
+    return false;
+
+  for (size_t i = 0; i < a.size(); ++i) {
+    if (*a[i] != *b[i])
+      return false;
+  }
+
+  return true;
+}
 
 WalletItems::MaskedInstrument::Type
     TypeFromString(const std::string& type_string) {
@@ -286,38 +303,36 @@ string16 WalletItems::MaskedInstrument::GetInfo(AutofillFieldType type) const {
   return string16();
 }
 
-WalletItems::LegalDocument::LegalDocument(const std::string& document_id,
-                                          const std::string& display_name)
-    : document_id_(document_id),
-      display_name_(display_name) {}
-
 WalletItems::LegalDocument::~LegalDocument() {}
 
 scoped_ptr<WalletItems::LegalDocument>
     WalletItems::LegalDocument::CreateLegalDocument(
     const base::DictionaryValue& dictionary) {
-  std::string document_id;
-  if (!dictionary.GetString("legal_document_id", &document_id)) {
+  std::string id;
+  if (!dictionary.GetString("legal_document_id", &id)) {
     DLOG(ERROR) << "Response from Google Wallet missing legal document id";
     return scoped_ptr<LegalDocument>();
   }
 
-  std::string display_name;
+  string16 display_name;
   if (!dictionary.GetString("display_name", &display_name)) {
     DLOG(ERROR) << "Response from Google Wallet missing display name";
     return scoped_ptr<LegalDocument>();
   }
 
-  return scoped_ptr<LegalDocument>(new LegalDocument(document_id,
-                                                     display_name));
+  return scoped_ptr<LegalDocument>(new LegalDocument(id, display_name));
 }
 
-GURL WalletItems::LegalDocument::GetUrl() {
-  return GURL(kLegalDocumentUrl + document_id_);
+scoped_ptr<WalletItems::LegalDocument>
+    WalletItems::LegalDocument::CreatePrivacyPolicyDocument() {
+  return scoped_ptr<LegalDocument>(new LegalDocument(
+      GURL(kPrivacyNoticeUrl),
+      l10n_util::GetStringUTF16(IDS_AUTOFILL_DIALOG_PRIVACY_POLICY_LINK)));
 }
 
 bool WalletItems::LegalDocument::operator==(const LegalDocument& other) const {
-  return document_id_ == other.document_id_ &&
+  return id_ == other.id_ &&
+         url_ == other.url_ &&
          display_name_ == other.display_name_;
 }
 
@@ -325,6 +340,17 @@ bool WalletItems::LegalDocument::operator!=(const LegalDocument& other) const {
   return !(*this == other);
 }
 
+WalletItems::LegalDocument::LegalDocument(const std::string& id,
+                                          const string16& display_name)
+    : id_(id),
+      url_(kLegalDocumentUrl + id),
+      display_name_(display_name) {}
+
+WalletItems::LegalDocument::LegalDocument(const GURL& url,
+                                          const string16& display_name)
+    : url_(url),
+      display_name_(display_name) {}
+
 WalletItems::WalletItems(const std::vector<RequiredAction>& required_actions,
                          const std::string& google_transaction_id,
                          const std::string& default_instrument_id,
@@ -400,6 +426,12 @@ scoped_ptr<WalletItems>
         }
       }
     }
+
+    if (!legal_docs->empty()) {
+      // Always append the privacy policy link as well.
+      wallet_items->AddLegalDocument(
+          LegalDocument::CreatePrivacyPolicyDocument());
+    }
   } else {
     DVLOG(1) << "Response from Google wallet missing legal docs";
   }
@@ -442,12 +474,16 @@ scoped_ptr<WalletItems>
 }
 
 bool WalletItems::operator==(const WalletItems& other) const {
-  // TODO(ahutter): Check scoped vector equality.
   return google_transaction_id_ == other.google_transaction_id_ &&
          default_instrument_id_ == other.default_instrument_id_ &&
          default_address_id_ == other.default_address_id_ &&
          required_actions_ == other.required_actions_ &&
-         obfuscated_gaia_id_ == other.obfuscated_gaia_id_;
+         obfuscated_gaia_id_ == other.obfuscated_gaia_id_ &&
+         VectorsAreEqual<MaskedInstrument>(instruments(),
+                                           other.instruments()) &&
+         VectorsAreEqual<Address>(addresses(), other.addresses()) &&
+         VectorsAreEqual<LegalDocument>(legal_documents(),
+                                        other.legal_documents());
 }
 
 bool WalletItems::operator!=(const WalletItems& other) const {
diff --git a/components/autofill/browser/wallet/wallet_items.h b/components/autofill/browser/wallet/wallet_items.h
index 72c8efb..f278aa3 100644
--- a/components/autofill/browser/wallet/wallet_items.h
+++ b/components/autofill/browser/wallet/wallet_items.h
@@ -16,8 +16,7 @@
 #include "base/string16.h"
 #include "components/autofill/browser/wallet/required_action.h"
 #include "components/autofill/browser/wallet/wallet_address.h"
-
-class GURL;
+#include "googleurl/src/gurl.h"
 
 namespace base {
 class DictionaryValue;
@@ -156,33 +155,39 @@ class WalletItems {
    public:
     ~LegalDocument();
 
-    // Returns null if input is invalid or a valid legal document. Caller owns
-    // returned pointer.
+    // Returns null if input is invalid or a valid legal document.
     static scoped_ptr<LegalDocument>
         CreateLegalDocument(const base::DictionaryValue& dictionary);
 
-    // Get the url where this legal document is hosted.
-    GURL GetUrl();
+    // Returns a document for the privacy policy (acceptance of which is not
+    // tracked by the server).
+    static scoped_ptr<LegalDocument> CreatePrivacyPolicyDocument();
 
     bool operator==(const LegalDocument& other) const;
     bool operator!=(const LegalDocument& other) const;
 
-    const std::string& document_id() const { return document_id_; }
-    const std::string& display_name() const { return display_name_; }
+    const std::string& id() { return id_; }
+    const GURL& url() const { return url_; }
+    const string16& display_name() const { return display_name_; }
 
    private:
     friend class WalletItemsTest;
     FRIEND_TEST_ALL_PREFIXES(WalletItemsTest, CreateLegalDocument);
     FRIEND_TEST_ALL_PREFIXES(WalletItemsTest, CreateWalletItems);
-    FRIEND_TEST_ALL_PREFIXES(WalletItemsTest, LegalDocumentGetUrl);
-    LegalDocument(const std::string& document_id,
-                  const std::string& display_name);
-
-    // Externalized Online Wallet id for the document.
-    std::string document_id_;
-
+    FRIEND_TEST_ALL_PREFIXES(WalletItemsTest, LegalDocumentUrl);
+    FRIEND_TEST_ALL_PREFIXES(WalletItemsTest, LegalDocumentEmptyId);
+    LegalDocument(const std::string& id,
+                  const string16& display_name);
+    LegalDocument(const GURL& url,
+                  const string16& display_name);
+
+    // Externalized Online Wallet id for the document, or an empty string for
+    // documents not tracked by the server (such as the privacy policy).
+    std::string id_;
+    // The human-visitable URL that displays the document.
+    GURL url_;
     // User displayable name for the document.
-    std::string display_name_;
+    string16 display_name_;
     DISALLOW_COPY_AND_ASSIGN(LegalDocument);
   };
 
diff --git a/components/autofill/browser/wallet/wallet_items_unittest.cc b/components/autofill/browser/wallet/wallet_items_unittest.cc
index 3b5612c..552a31b 100644
--- a/components/autofill/browser/wallet/wallet_items_unittest.cc
+++ b/components/autofill/browser/wallet/wallet_items_unittest.cc
@@ -262,8 +262,11 @@ const char kWalletItemsMissingGoogleTransactionId[] =
     "      \"postal_address\":"
     "      {"
     "        \"recipient_name\":\"recipient_name\","
-    "        \"address_line_1\":\"address_line_1\","
-    "        \"address_line_2\":\"address_line_2\","
+    "        \"address_line\":"
+    "        ["
+    "          \"address_line_1\","
+    "          \"address_line_2\""
+    "        ],"
     "        \"locality_name\":\"locality_name\","
     "        \"administrative_area_name\":\"administrative_area_name\","
     "        \"postal_code_number\":\"postal_code_number\","
@@ -324,8 +327,11 @@ const char kWalletItems[] =
     "      \"postal_address\":"
     "      {"
     "        \"recipient_name\":\"recipient_name\","
-    "        \"address_line_1\":\"address_line_1\","
-    "        \"address_line_2\":\"address_line_2\","
+    "        \"address_line\":"
+    "        ["
+    "          \"address_line_1\","
+    "          \"address_line_2\""
+    "        ],"
     "        \"locality_name\":\"locality_name\","
     "        \"administrative_area_name\":\"administrative_area_name\","
     "        \"postal_code_number\":\"postal_code_number\","
@@ -334,15 +340,19 @@ const char kWalletItems[] =
     "    }"
     "  ],"
     "  \"default_address_id\":\"default_address_id\","
-    "  \"obfuscated_gaia_id\":\"obfuscated_gaia_id\","
+    "  \"obfuscated_gaia_id\":\"obfuscated_gaia_id\"";
+
+const char kRequiredLegalDocument[] =
+    "  ,"
     "  \"required_legal_document\":"
     "  ["
     "    {"
     "      \"legal_document_id\":\"doc_id\","
     "      \"display_name\":\"display_name\""
     "    }"
-    "  ]"
-    "}";
+    "  ]";
+
+const char kCloseJson[] = "}";
 
 }  // anonymous namespace
 
@@ -437,15 +447,21 @@ TEST_F(WalletItemsTest, CreateLegalDocumentMissingDisplayName) {
 
 TEST_F(WalletItemsTest, CreateLegalDocument) {
   SetUpDictionary(kLegalDocument);
-  WalletItems::LegalDocument expected("doc_id", "display_name");
+  WalletItems::LegalDocument expected("doc_id", ASCIIToUTF16("display_name"));
   EXPECT_EQ(expected,
             *WalletItems::LegalDocument::CreateLegalDocument(*dict));
 }
 
-TEST_F(WalletItemsTest, LegalDocumentGetUrl) {
-  WalletItems::LegalDocument legal_doc("doc_id", "display_name");
+TEST_F(WalletItemsTest, LegalDocumentUrl) {
+  WalletItems::LegalDocument legal_doc("doc_id", ASCIIToUTF16("display_name"));
   EXPECT_EQ("https://wallet.google.com/legaldocument?docId=doc_id",
-            legal_doc.GetUrl().spec());
+            legal_doc.url().spec());
+}
+
+TEST_F(WalletItemsTest, LegalDocumentEmptyId) {
+  WalletItems::LegalDocument legal_doc(GURL("http://example.com"),
+                                       ASCIIToUTF16("display_name"));
+  EXPECT_TRUE(legal_doc.id().empty());
 }
 
 TEST_F(WalletItemsTest, CreateWalletItemsWithRequiredActions) {
@@ -488,7 +504,7 @@ TEST_F(WalletItemsTest, CreateWalletItemsMissingGoogleTransactionId) {
 }
 
 TEST_F(WalletItemsTest, CreateWalletItems) {
-  SetUpDictionary(kWalletItems);
+  SetUpDictionary(std::string(kWalletItems) + std::string(kCloseJson));
   std::vector<RequiredAction> required_actions;
   WalletItems expected(required_actions,
                        "google_transaction_id",
@@ -519,21 +535,29 @@ TEST_F(WalletItemsTest, CreateWalletItems) {
                                         "object_id"));
   expected.AddInstrument(masked_instrument.Pass());
 
-  scoped_ptr<Address> shipping_address(new Address("country_code",
-                                                   ASCIIToUTF16("name"),
-                                                   ASCIIToUTF16("address1"),
-                                                   ASCIIToUTF16("address2"),
-                                                   ASCIIToUTF16("city"),
-                                                   ASCIIToUTF16("state"),
-                                                   ASCIIToUTF16("postal_code"),
-                                                   ASCIIToUTF16("phone_number"),
-                                                   "id"));
+  scoped_ptr<Address> shipping_address(
+      new Address("country_name_code",
+                  ASCIIToUTF16("recipient_name"),
+                  ASCIIToUTF16("address_line_1"),
+                  ASCIIToUTF16("address_line_2"),
+                  ASCIIToUTF16("locality_name"),
+                  ASCIIToUTF16("administrative_area_name"),
+                  ASCIIToUTF16("postal_code_number"),
+                  ASCIIToUTF16("phone_number"),
+                  "id"));
   expected.AddAddress(shipping_address.Pass());
+  EXPECT_EQ(expected, *WalletItems::CreateWalletItems(*dict));
 
+  // Now try with a legal document as well.
+  SetUpDictionary(std::string(kWalletItems) +
+                  std::string(kRequiredLegalDocument) +
+                  std::string(kCloseJson));
   scoped_ptr<WalletItems::LegalDocument> legal_document(
       new WalletItems::LegalDocument("doc_id",
-                                     "display_name"));
+                                     ASCIIToUTF16("display_name")));
   expected.AddLegalDocument(legal_document.Pass());
+  expected.AddLegalDocument(
+      WalletItems::LegalDocument::CreatePrivacyPolicyDocument());
 
   EXPECT_EQ(expected, *WalletItems::CreateWalletItems(*dict));
 }