PaymentRequest: Provide shippingAddress in PaymentResponse.

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

Index: third_party/WebKit/Source/modules/payments/PaymentResponseTest.cpp
diff --git a/third_party/WebKit/Source/modules/payments/PaymentResponseTest.cpp b/third_party/WebKit/Source/modules/payments/PaymentResponseTest.cpp
index 5677cb40a10bd0a6b2cfb1f8577eceeab730e328..32bebb3b6e1e989f82ac4c4e7821b24b4e078983 100644
--- a/third_party/WebKit/Source/modules/payments/PaymentResponseTest.cpp
+++ b/third_party/WebKit/Source/modules/payments/PaymentResponseTest.cpp
@@ -7,8 +7,12 @@
 #include "bindings/core/v8/ExceptionState.h"
 #include "bindings/core/v8/ScriptState.h"
 #include "bindings/core/v8/ScriptValue.h"
+#include "bindings/modules/v8/V8PaymentResponse.h"
 #include "core/testing/DummyPageHolder.h"
+#include "modules/payments/PaymentAddress.h"
 #include "modules/payments/PaymentCompleter.h"
+#include "modules/payments/PaymentDetailsTestHelper.h"
+#include "modules/payments/PaymentRequest.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "wtf/OwnPtr.h"
@@ -43,6 +47,7 @@ public:
     PaymentResponseTest()
         : m_page(DummyPageHolder::create())
     {
+        setSecurityOrigin("https://www.example.com");

[please use gerrit instead, 2016/06/06 01:09:35]

You're calling this function only once. So it's simpler to copy the contents of
the function directly into here.

[zino, 2016/06/06 10:02:13]

Done.

     }
 
     ~PaymentResponseTest() override {}
@@ -50,11 +55,63 @@ public:
     ScriptState* getScriptState() { return ScriptState::forMainWorld(m_page->document().frame()); }
     ExceptionState& getExceptionState() { return m_exceptionState; }
 
+    void setSecurityOrigin(const String& securityOrigin)
+    {
+        m_page->document().setSecurityOrigin(SecurityOrigin::create(KURL(KURL(), securityOrigin)));
+    }
+
 private:
     OwnPtr<DummyPageHolder> m_page;
     NonThrowableExceptionState m_exceptionState;
 };
 
+class PaymentResponseFunction : public ScriptFunction {
+public:
+    static v8::Local<v8::Function> create(ScriptState* scriptState, ScriptValue* outValue)
+    {
+        PaymentResponseFunction* self = new PaymentResponseFunction(scriptState, outValue);
+        return self->bindToV8Function();
+    }
+
+    ScriptValue call(ScriptValue value) override
+    {
+        DCHECK(!value.isEmpty());
+        *m_value = value;
+        return value;
+    }
+
+private:
+    explicit PaymentResponseFunction(ScriptState* scriptState, ScriptValue* outValue)

[please use gerrit instead, 2016/06/06 01:09:35]

No need for "explicit" keyword with more than one parameter.

[zino, 2016/06/06 10:02:13]

Done.

Sorry, this code was created based on MockFunction in PaymentRequest. (Copy &
paste, and then modify)
So.. I had to remove the keyword but I forgot it. :P

+        : ScriptFunction(scriptState)
+        , m_value(outValue)
+    {

[please use gerrit instead, 2016/06/06 01:09:34]

DCHECK(m_value);

[zino, 2016/06/06 10:02:13]

Done.

+    }
+
+    ScriptValue* m_value;
+};
+
+class UnreachableFunction : public ScriptFunction {
+public:
+    static v8::Local<v8::Function> create(ScriptState* scriptState)
+    {
+        UnreachableFunction* self = new UnreachableFunction(scriptState);
+        return self->bindToV8Function();
+    }
+
+    ScriptValue call(ScriptValue value) override
+    {
+        ADD_FAILURE() << "Unexpected call to a null ScriptFunction.";

[please use gerrit instead, 2016/06/06 01:09:35]

"Unexpected call"

[zino, 2016/06/06 10:02:13]

Done.

+        return value;
+    }
+
+private:
+    UnreachableFunction(ScriptState* scriptState)

[please use gerrit instead, 2016/06/06 01:09:34]

Need "explicit" keyword here to avoid implicit conversion from ScriptState to
UnreachableFunction.

[zino, 2016/06/06 10:02:13]

Done.

+        : ScriptFunction(scriptState)
+    {
+    }
+};
+
+
 TEST_F(PaymentResponseTest, DataCopiedOver)
 {
     ScriptState::Scope scope(getScriptState());
@@ -104,5 +161,50 @@ TEST_F(PaymentResponseTest, CompleteCalledWithFailure)
     output.complete(getScriptState(), false);
 }
 
+TEST_F(PaymentResponseTest, ShippingAddressAttributeWithRequestShippingTrue)

[please use gerrit instead, 2016/06/06 01:09:35]

Plus one more test: options.setRequestShipping(true) and the
mojom::blink::PaymentResponse does not have an address. This should reject the
promise.

[zino, 2016/06/06 10:02:13]

Done.

+{
+    ScriptState::Scope scope(getScriptState());
+    PaymentOptions options;
+    options.setRequestShipping(true);
+    PaymentRequest* request = PaymentRequest::create(getScriptState(), Vector<String>(1, "foo"), buildPaymentDetailsForTest(), options, getExceptionState());
+    EXPECT_FALSE(getExceptionState().hadException());
+    mojom::blink::PaymentResponsePtr response = mojom::blink::PaymentResponse::New();
+    response->shipping_address = mojom::blink::PaymentAddress::New();
+    response->shipping_address->region_code = "US";
+    response->shipping_address->language_code = "en";
+    response->shipping_address->script_code = "Latn";
+
+    ScriptValue outValue;
+    request->show(getScriptState()).then(PaymentResponseFunction::create(getScriptState(), &outValue), UnreachableFunction::create(getScriptState()));
+    static_cast<mojom::blink::PaymentRequestClient*>(request)->OnPaymentResponse(std::move(response));
+    v8::MicrotasksScope::PerformCheckpoint(getScriptState()->isolate());
+    PaymentResponse* pr = V8PaymentResponse::toImplWithTypeCheck(getScriptState()->isolate(), outValue.v8Value());
+
+    EXPECT_EQ("US", pr->shippingAddress()->regionCode());
+    EXPECT_EQ("en-Latn", pr->shippingAddress()->languageCode());
+}
+
+TEST_F(PaymentResponseTest, ShippingAddressAttributeWithRequestShippingFalse)
+{
+    ScriptState::Scope scope(getScriptState());
+    PaymentOptions options;
+    options.setRequestShipping(false);
+    PaymentRequest* request = PaymentRequest::create(getScriptState(), Vector<String>(1, "foo"), buildPaymentDetailsForTest(), options, getExceptionState());
+    EXPECT_FALSE(getExceptionState().hadException());
+    mojom::blink::PaymentResponsePtr response = mojom::blink::PaymentResponse::New();
+    response->shipping_address = mojom::blink::PaymentAddress::New();
+    response->shipping_address->region_code = "US";
+    response->shipping_address->language_code = "en";
+    response->shipping_address->script_code = "Latn";
+
+    ScriptValue outValue;
+    request->show(getScriptState()).then(PaymentResponseFunction::create(getScriptState(), &outValue), UnreachableFunction::create(getScriptState()));

[please use gerrit instead, 2016/06/06 01:09:35]

My suggestion in PaymentRequest.cpp is to reject the show() promise if
options.setRequestShipping(false) and mojom::blink::PaymentResponse has a
shipping address. Therefore, let's ensure that this promise is rejected:

request->show(getScriptState()).then(MockFunction::expectNoCall(getScriptState()),
MockFunction::expectCall(getScriptState()));

Example:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/paymen...

[zino, 2016/06/06 10:02:13]

Done.

+    static_cast<mojom::blink::PaymentRequestClient*>(request)->OnPaymentResponse(std::move(response));
+    v8::MicrotasksScope::PerformCheckpoint(getScriptState()->isolate());
+    PaymentResponse* pr = V8PaymentResponse::toImplWithTypeCheck(getScriptState()->isolate(), outValue.v8Value());
+
+    EXPECT_EQ(nullptr, pr->shippingAddress());
+}
+
 } // namespace
 } // namespace blink