[Web Payment] Correctly handle empty modifiers.

Before this patch, specifying an empty list of modifiers would crash
Android PaymentRequest, because it would clear its own list of modifiers
before initializing it.

This patch checks that the list of modifiers is initialized before
clearing it. The new test EmptyParametersTest.NoCrash crashes without
the patch and passes with the patch.

After this patch, specifying an empty list of modifiers does not crash
Android PaymentRequest.

The android_browsertests were chosen to make sure both Android and
desktop behave correctly. Although WPTs could also find the bug, they
currently don't run on the waterfall (neither desktop nor Android).

Bug: 1022810
Change-Id: I661aa80889822a73939c5f96662dc3531689a85e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1906806
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: Danyao Wang <danyao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713909}

chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java

@@ -1396,7 +1396,7 @@ public class PaymentRequestImpl
         }
 
         if (details.modifiers != null) {
-            if (details.modifiers.length == 0) mModifiers.clear();
+            if (details.modifiers.length == 0 && mModifiers != null) mModifiers.clear();
 
             for (int i = 0; i < details.modifiers.length; i++) {
                 PaymentDetailsModifier modifier = details.modifiers[i];

chrome/browser/payments/empty_parameters_browsertest.cc

+// Copyright 2019 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 "base/macros.h"
+#include "build/build_config.h"
+#include "chrome/test/base/chrome_test_utils.h"
+#include "content/public/test/browser_test_utils.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+#if defined(OS_ANDROID)
+#include "chrome/test/base/android/android_browser_test.h"
+#else
+#include "chrome/test/base/in_process_browser_test.h"
+#endif
+
+namespace content {
+class WebContents;
+}  // namespace content
+
+namespace payments {
+namespace {
+
+class EmptyParametersTest : public PlatformBrowserTest {
+ public:
+  EmptyParametersTest() : https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {}
+  ~EmptyParametersTest() override {}
+
+  void SetUpOnMainThread() override {
+    https_server_.ServeFilesFromSourceDirectory(
+        "components/test/data/payments");
+    ASSERT_TRUE(https_server_.Start());
+
+    ASSERT_TRUE(content::NavigateToURL(
+        GetActiveWebContents(),
+        https_server_.GetURL("/empty_parameters_test.html")));
+
+    PlatformBrowserTest::SetUpOnMainThread();
+  }
+
+  content::WebContents* GetActiveWebContents() {
+    return chrome_test_utils::GetActiveWebContents(this);
+  }
+
+ private:
+  net::EmbeddedTestServer https_server_;
+
+  DISALLOW_COPY_AND_ASSIGN(EmptyParametersTest);
+};
+
+IN_PROC_BROWSER_TEST_F(EmptyParametersTest, NoCrash) {
+  EXPECT_EQ(true, content::EvalJs(GetActiveWebContents(), "runTest()"));
+}
+
+}  // namespace
+}  // namespace payments

chrome/test/BUILD.gn

@@ -525,6 +525,7 @@ if (is_android) {
 
     sources = [
       "../browser/engagement/important_sites_util_browsertest.cc",
+      "../browser/payments/empty_parameters_browsertest.cc",
       "../browser/payments/has_enrolled_instrument_browsertest.cc",
       "../browser/payments/has_enrolled_instrument_query_quota_browsertest.cc",
       "../browser/payments/hybrid_request_skip_ui_browsertest.cc",
@@ -1841,6 +1842,7 @@ if (!is_android) {
     }
     if (toolkit_views) {
       sources += [
+        "../browser/payments/empty_parameters_browsertest.cc",
         "../browser/payments/has_enrolled_instrument_browsertest.cc",
         "../browser/payments/has_enrolled_instrument_query_quota_browsertest.cc",
         "../browser/payments/journey_logger_browsertest.cc",

components/test/data/payments/empty_parameters.js

+/*
+ * Copyright 2019 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.
+ */
+
+/**
+ * Queries Payment Request with some empty parameters.
+ * Regression test for: https://crbug.com/1022810
+ * @return {Promise<boolean>} - Whether a payment can be made.
+ */
+async function runTest() { // eslint-disable-line no-unused-vars
+  return new PaymentRequest([{supportedMethods: 'basic-card'}], {
+    displayItems: [],
+    id: '',
+    modifiers: [],
+    shippingOptions: [],
+    total: {
+      label: 'Subscription',
+      amount: {
+        value: '1.00',
+        currency: 'USD',
+      },
+    },
+  }).canMakePayment();
+}

components/test/data/payments/empty_parameters_test.html

+<!DOCTYPE html>
+<!--
+Copyright 2019 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.
+-->
+<html>
+<head>
+<title>Empty Parameters Test</title>
+<meta charset="utf-8">
+<meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1, minimum-scale=1">
+<link rel="stylesheet" type="text/css" href="style.css">
+</head>
+<body>
+<p>Regression test for <a href="https://crbug.com/1022810">Issue 1022810</a>.</p>
+<div><button onclick="runTest()" id="runTest">Run Test</button></div>
+<script src="empty_parameters.js"></script>
+</body>
+</html>