PaymentRequest Mojo bindings.

third_party/WebKit/Source/modules/payments/ShippingAddress.cpp

Index: third_party/WebKit/Source/modules/payments/ShippingAddress.cpp
diff --git a/third_party/WebKit/Source/modules/payments/ShippingAddress.cpp b/third_party/WebKit/Source/modules/payments/ShippingAddress.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..1708eda019cad25311257bfa6bb6f8f3532cdbb6
--- /dev/null
+++ b/third_party/WebKit/Source/modules/payments/ShippingAddress.cpp
@@ -0,0 +1,83 @@
+// Copyright 2016 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 "modules/payments/ShippingAddress.h"
+
+namespace blink {
+namespace {
+
+bool isValidRegionCodeString(const String& code)

[groby-ooo-7-16, 2016/03/18 00:47:26]

Please add comments on what _kind_ of region codes those are. This looks like
ISO 3166-1, except you're not checking for lowers.

[rwlbuis, 2016/03/18 14:37:59]

Why not make all three helpers static?

[please use gerrit instead, 2016/03/23 00:14:52]

Done.

[please use gerrit instead, 2016/03/23 00:14:52]

This comes from region_data_constants.cc in Chrome (for now). These are all
upper case in Chrome.

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/libadd...

+{
+    if (code.length() != 2)
+        return false;
+
+    for (size_t i = 0; i < code.length(); ++i) {

[groby-ooo-7-16, 2016/03/18 00:47:25]

I take it String doesn't have iterators, so no range loops?

[please use gerrit instead, 2016/03/23 00:14:52]

That's right.

+        if (code[i] < 'A' || code[i] > 'Z')

[groby-ooo-7-16, 2016/03/18 00:47:26]

Probably isalpha() instead, unless there's a reason to stay with upper case, in
which case isupper

Which simplifies the whole thing to
  return (code.length()==2) && isupper(code[0]) && isupper(code[1]);

[Marijn Kruisselbrink, 2016/03/18 01:39:48]

isASCIIUpper(), rather than isupper(). std::isupper() takes the current locale
into account, which is definitely not something we want here.

[please use gerrit instead, 2016/03/23 00:14:52]

Using ScriptRegexp instead.

[please use gerrit instead, 2016/03/23 00:14:52]

Using ScriptRegexp instead.

+            return false;
+    }
+
+    return true;
+}
+
+bool isValidLanguageCodeString(const String& code)
+{
+    if (code.isEmpty())
+        return true;
+
+    if (code.length() != 2 && code.length() != 3)

[groby-ooo-7-16, 2016/03/18 00:47:26]

ISO639, I take it? (Although, again, that would allow alpha, not only lower)

[please use gerrit instead, 2016/03/23 00:14:52]

This comes from region_data_constants.cc in Chrome (for now), which has
https://tools.ietf.org/html/bcp47 language tags. The first part of the BCP 47
language tag is the shortest ISO 639 language code. These are all lower case in
region_data_constants.cc.

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/libadd...

+        return false;
+
+    for (size_t i = 0; i < code.length(); ++i) {
+        if (code[i] < 'a' || code[i] > 'z')

[groby-ooo-7-16, 2016/03/18 00:47:26]

islower

[Marijn Kruisselbrink, 2016/03/18 01:39:48]

isASCIILower()

[please use gerrit instead, 2016/03/23 00:14:52]

Using ScriptRegexp instead.

[please use gerrit instead, 2016/03/23 00:14:52]

Using ScriptRegexp instead.

+            return false;
+    }
+
+    return true;
+}
+
+bool isValidScriptCodeString(const String& code)

[groby-ooo-7-16, 2016/03/18 00:47:26]

ISO 15924 ?

[please use gerrit instead, 2016/03/23 00:14:52]

Added comments and error message strings that specify the ISO.

+{
+    if (code.isEmpty())
+        return true;
+
+    if (code.length() != 4)
+        return false;
+
+    if (code[0] < 'A' || code[0] > 'A')

[groby-ooo-7-16, 2016/03/18 00:47:25]

Technically, that's again isalpha for all 4. 

[Marijn Kruisselbrink, 2016/03/18 01:39:48]

I assume you didn't mean to write "if (code[0] != 'A')" ?

[please use gerrit instead, 2016/03/23 00:14:52]

Using ScriptRegexp instead.

[please use gerrit instead, 2016/03/23 00:14:52]

Fixed, tested in PaymentsValidatorsTest.cpp, and switched to ScriptRegexp in the
end.

+        return false;
+
+    for (size_t i = 1; i < code.length(); ++i) {
+        if (code[i] < 'a' || code[i] > 'z')
+            return false;
+    }
+
+    return true;
+}
+
+} // namespace
+
+ShippingAddress::ShippingAddress(mojom::blink::ShippingAddressPtr address)
+    : m_regionCode(address->region_code)
+    , m_addressLine(address->address_line.PassStorage())
+    , m_administrativeArea(address->administrative_area)
+    , m_locality(address->locality)
+    , m_dependentLocality(address->dependent_locality)
+    , m_postalCode(address->postal_code)
+    , m_sortingCode(address->sorting_code)
+    , m_languageCode(address->language_code)
+    , m_organization(address->organization)
+    , m_recipient(address->recipient)
+    , m_validRegionCode(isValidRegionCodeString(address->region_code))

[groby-ooo-7-16, 2016/03/18 00:47:26]

Assuming we're doing the validation for security reasons, shouldn't we never
copy those strings if they fail validation?

Also, would this be better off as a mojo message validator?

[please use gerrit instead, 2016/03/23 00:14:52]

Moved validation to PaymentRequest::onShippingAddressChanged(), before copying
the strings.
Going to consider mojo validators in follow up patches. This patch has grown to
~1.5k lines already.

+    , m_validLanguageCode(isValidLanguageCodeString(address->language_code))
+    , m_validScriptCode(isValidScriptCodeString(address->script_code))
+{
+    if (!m_languageCode.isEmpty() && !address->script_code.isEmpty()) {
+        m_languageCode.append("-");
+        m_languageCode.append(address->script_code);
+    }
+}
+
+ShippingAddress::~ShippingAddress() {}
+
+} // namespace blink