diff --git a/DEPS b/DEPS index b1dc93c1..b2fb2af 100644 --- a/DEPS +++ b/DEPS
@@ -40,7 +40,7 @@ # Three lines of non-changing comments so that # the commit queue can handle CLs rolling Skia # and whatever else without interference from each other. - 'skia_revision': 'ad61192eda55a71de9253b6204ed5490867b4f36', + 'skia_revision': '14b748ddd2a8f0eba9a1b3061980d1174dfc279d', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling V8 # and whatever else without interference from each other.
diff --git a/ios/web/payments/payment_request.cc b/ios/web/payments/payment_request.cc index ca77381e..820b6b5 100644 --- a/ios/web/payments/payment_request.cc +++ b/ios/web/payments/payment_request.cc
@@ -23,13 +23,16 @@ static const char kAddressRecipient[] = "recipient"; static const char kAddressPhone[] = "phone"; static const char kMethodData[] = "methodData"; +static const char kMethodDataData[] = "data"; +static const char kPaymentCurrencyAmountCurrency[] = "currency"; +static const char kPaymentCurrencyAmountValue[] = "value"; +static const char kPaymentItemLabel[] = "label"; +static const char kPaymentItemAmount[] = "amount"; +static const char kPaymentItemPending[] = "pending"; static const char kSupportedMethods[] = "supportedMethods"; -static const char kData[] = "data"; static const char kPaymentDetails[] = "details"; static const char kPaymentDetailsTotal[] = "total"; -static const char kPaymentDetailsTotalAmount[] = "amount"; -static const char kPaymentDetailsTotalAmountCurrency[] = "currency"; -static const char kPaymentDetailsTotalAmountValue[] = "value"; +static const char kPaymentDetailsDisplayItems[] = "displayItems"; static const char kMethodName[] = "methodName"; static const char kCardCardholderName[] = "cardholderName"; static const char kCardCardNumber[] = "cardNumber"; @@ -113,6 +116,30 @@ return !(*this == other); } +bool PaymentMethodData::FromDictionaryValue( + const base::DictionaryValue& value) { + this->supported_methods.clear(); + + const base::ListValue* supported_methods_list = nullptr; + // At least one supported method is required. + if (!value.GetList(kSupportedMethods, &supported_methods_list) || + supported_methods_list->GetSize() == 0) { + return false; + } + for (size_t i = 0; i < supported_methods_list->GetSize(); ++i) { + base::string16 supported_method; + if (!supported_methods_list->GetString(i, &supported_method)) { + return false; + } + this->supported_methods.push_back(supported_method); + } + + // Data is optional. + value.GetString(kMethodDataData, &this->data); + + return true; +} + PaymentCurrencyAmount::PaymentCurrencyAmount() {} PaymentCurrencyAmount::~PaymentCurrencyAmount() = default; @@ -126,17 +153,43 @@ return !(*this == other); } -PaymentItem::PaymentItem() {} +bool PaymentCurrencyAmount::FromDictionaryValue( + const base::DictionaryValue& value) { + return value.GetString(kPaymentCurrencyAmountCurrency, &this->currency) && + value.GetString(kPaymentCurrencyAmountValue, &this->value); +} + +PaymentItem::PaymentItem() : pending(false) {} PaymentItem::~PaymentItem() = default; bool PaymentItem::operator==(const PaymentItem& other) const { - return this->label == other.label && this->amount == other.amount; + return this->label == other.label && this->amount == other.amount && + this->pending == other.pending; } bool PaymentItem::operator!=(const PaymentItem& other) const { return !(*this == other); } +bool PaymentItem::FromDictionaryValue(const base::DictionaryValue& value) { + if (!value.GetString(kPaymentItemLabel, &this->label)) { + return false; + } + + const base::DictionaryValue* amount_dict = nullptr; + if (!value.GetDictionary(kPaymentItemAmount, &amount_dict)) { + return false; + } + if (!this->amount.FromDictionaryValue(*amount_dict)) { + return false; + } + + // Pending is optional. + value.GetBoolean(kPaymentItemPending, &this->pending); + + return true; +} + PaymentShippingOption::PaymentShippingOption() : selected(false) {} PaymentShippingOption::PaymentShippingOption( const PaymentShippingOption& other) = default; @@ -185,6 +238,37 @@ return !(*this == other); } +bool PaymentDetails::FromDictionaryValue(const base::DictionaryValue& value) { + this->display_items.clear(); + this->shipping_options.clear(); + this->modifiers.clear(); + + const base::DictionaryValue* total_dict = nullptr; + if (!value.GetDictionary(kPaymentDetailsTotal, &total_dict)) { + return false; + } + if (!this->total.FromDictionaryValue(*total_dict)) { + return false; + } + + const base::ListValue* display_items_list = nullptr; + if (value.GetList(kPaymentDetailsDisplayItems, &display_items_list)) { + for (size_t i = 0; i < display_items_list->GetSize(); ++i) { + const base::DictionaryValue* payment_item_dict; + if (!display_items_list->GetDictionary(i, &payment_item_dict)) { + return false; + } + PaymentItem payment_item; + if (!payment_item.FromDictionaryValue(*payment_item_dict)) { + return false; + } + this->display_items.push_back(payment_item); + } + } + + return true; +} + PaymentOptions::PaymentOptions() : request_payer_email(false), request_payer_phone(false), @@ -228,43 +312,20 @@ } for (size_t i = 0; i < method_data_list->GetSize(); ++i) { const base::DictionaryValue* method_data_dict; - // Method data is required. if (!method_data_list->GetDictionary(i, &method_data_dict)) return false; PaymentMethodData method_data; - const base::ListValue* supported_methods_list = nullptr; - // At least one supported method is required. - if (!method_data_dict->GetList(kSupportedMethods, - &supported_methods_list) || - supported_methods_list->GetSize() == 0) { + if (!method_data.FromDictionaryValue(*method_data_dict)) return false; - } - for (size_t i = 0; i < supported_methods_list->GetSize(); ++i) { - base::string16 supported_method; - supported_methods_list->GetString(i, &supported_method); - method_data.supported_methods.push_back(supported_method); - } - method_data_dict->GetString(kData, &method_data.data); - this->method_data.push_back(method_data); } // Parse the payment details. const base::DictionaryValue* payment_details_dict = nullptr; - if (value.GetDictionary(kPaymentDetails, &payment_details_dict)) { - const base::DictionaryValue* total_dict = nullptr; - if (payment_details_dict->GetDictionary(kPaymentDetailsTotal, - &total_dict)) { - const base::DictionaryValue* amount_dict = nullptr; - if (total_dict->GetDictionary(kPaymentDetailsTotalAmount, &amount_dict)) { - amount_dict->GetString(kPaymentDetailsTotalAmountCurrency, - &this->details.total.amount.currency); - amount_dict->GetString(kPaymentDetailsTotalAmountValue, - &this->details.total.amount.value); - } - } - } + if (value.GetDictionary(kPaymentDetails, &payment_details_dict)) + if (!this->details.FromDictionaryValue(*payment_details_dict)) + return false; // TODO(crbug.com/602666): Parse the remaining elements.
diff --git a/ios/web/payments/payment_request_unittest.cc b/ios/web/payments/payment_request_unittest.cc index 46901c3b..add9076 100644 --- a/ios/web/payments/payment_request_unittest.cc +++ b/ios/web/payments/payment_request_unittest.cc
@@ -14,14 +14,125 @@ // PaymentRequest parsing tests. -// Tests that parsing an empty value fails. +// Tests the success case when populating a PaymentMethodData from a dictionary. +TEST(PaymentRequestTest, PaymentMethodDataFromDictionaryValueSuccess) { + PaymentMethodData expected; + std::vector<base::string16> supported_methods; + supported_methods.push_back(base::ASCIIToUTF16("Visa")); + supported_methods.push_back(base::ASCIIToUTF16("Bitcoin")); + expected.supported_methods = supported_methods; + expected.data = base::ASCIIToUTF16("{merchantId: 'af22fke9'}"); + + base::DictionaryValue method_data_dict; + std::unique_ptr<base::ListValue> supported_methods_list(new base::ListValue); + supported_methods_list->AppendString("Visa"); + supported_methods_list->AppendString("Bitcoin"); + method_data_dict.Set("supportedMethods", std::move(supported_methods_list)); + method_data_dict.SetString("data", "{merchantId: 'af22fke9'}"); + + PaymentMethodData actual; + EXPECT_TRUE(actual.FromDictionaryValue(method_data_dict)); + + EXPECT_EQ(expected, actual); +} + +// Tests the failure case when populating a PaymentMethodData from a dictionary. +TEST(PaymentRequestTest, PaymentMethodDataFromDictionaryValueFailure) { + // At least one supported method is required. + PaymentMethodData actual; + base::DictionaryValue method_data_dict; + EXPECT_FALSE(actual.FromDictionaryValue(method_data_dict)); + + // The value in the supported methods list must be a string. + std::unique_ptr<base::ListValue> supported_methods_list(new base::ListValue); + supported_methods_list->AppendInteger(13); + method_data_dict.Set("supportedMethods", std::move(supported_methods_list)); + EXPECT_FALSE(actual.FromDictionaryValue(method_data_dict)); +} + +// Tests the success case when populating a PaymentCurrencyAmount from a +// dictionary. +TEST(PaymentRequestTest, PaymentCurrencyAmountFromDictionaryValueSuccess) { + PaymentCurrencyAmount expected; + expected.currency = base::ASCIIToUTF16("AUD"); + expected.value = base::ASCIIToUTF16("-438.23"); + + base::DictionaryValue amount_dict; + amount_dict.SetString("currency", "AUD"); + amount_dict.SetString("value", "-438.23"); + + PaymentCurrencyAmount actual; + EXPECT_TRUE(actual.FromDictionaryValue(amount_dict)); + + EXPECT_EQ(expected, actual); +} + +// Tests the failure case when populating a PaymentCurrencyAmount from a +// dictionary. +TEST(PaymentRequestTest, PaymentCurrencyAmountFromDictionaryValueFailure) { + // Both a currency and a value are required. + PaymentCurrencyAmount actual; + base::DictionaryValue amount_dict; + EXPECT_FALSE(actual.FromDictionaryValue(amount_dict)); + + // Both values must be strings. + amount_dict.SetInteger("currency", 842); + amount_dict.SetString("value", "-438.23"); + EXPECT_FALSE(actual.FromDictionaryValue(amount_dict)); + + amount_dict.SetString("currency", "NZD"); + amount_dict.SetDouble("value", -438.23); + EXPECT_FALSE(actual.FromDictionaryValue(amount_dict)); +} + +// Tests the success case when populating a PaymentItem from a dictionary. +TEST(PaymentRequestTest, PaymentItemFromDictionaryValueSuccess) { + PaymentItem expected; + expected.label = base::ASCIIToUTF16("Payment Total"); + expected.amount.currency = base::ASCIIToUTF16("NZD"); + expected.amount.value = base::ASCIIToUTF16("2,242,093.00"); + + base::DictionaryValue item_dict; + item_dict.SetString("label", "Payment Total"); + std::unique_ptr<base::DictionaryValue> amount_dict(new base::DictionaryValue); + amount_dict->SetString("currency", "NZD"); + amount_dict->SetString("value", "2,242,093.00"); + item_dict.Set("amount", std::move(amount_dict)); + + PaymentItem actual; + EXPECT_TRUE(actual.FromDictionaryValue(item_dict)); + + EXPECT_EQ(expected, actual); +} + +// Tests the failure case when populating a PaymentItem from a dictionary. +TEST(PaymentRequestTest, PaymentItemFromDictionaryValueFailure) { + // Both a label and an amount are required. + PaymentItem actual; + base::DictionaryValue item_dict; + EXPECT_FALSE(actual.FromDictionaryValue(item_dict)); + + item_dict.SetString("label", "Payment Total"); + EXPECT_FALSE(actual.FromDictionaryValue(item_dict)); + + // Even with both present, the label must be a string. + std::unique_ptr<base::DictionaryValue> amount_dict(new base::DictionaryValue); + amount_dict->SetString("currency", "NZD"); + amount_dict->SetString("value", "2,242,093.00"); + item_dict.Set("amount", std::move(amount_dict)); + item_dict.SetInteger("label", 42); + EXPECT_FALSE(actual.FromDictionaryValue(item_dict)); +} + +// Tests that populating a PaymentRequest from an empty dictionary fails. TEST(PaymentRequestTest, ParsingEmptyRequestDictionaryFails) { PaymentRequest output_request; base::DictionaryValue request_dict; EXPECT_FALSE(output_request.FromDictionaryValue(request_dict)); } -// Tests that parsing a dictionary without all requirement values fails. +// Tests that populating a PaymentRequest from a dictionary without all +// required values fails. TEST(PaymentRequestTest, ParsingPartiallyPopulatedRequestDictionaryFails) { PaymentRequest expected_request; PaymentRequest output_request; @@ -53,8 +164,8 @@ EXPECT_EQ(expected_request, output_request); } -// Tests that parsing a dictionary with all required elements succeeds and -// produces the expected result. +// Tests that populating a PaymentRequest from a dictionary with all required +// elements succeeds and produces the expected result. TEST(PaymentRequestTest, ParsingFullyPopulatedRequestDictionarySucceeds) { PaymentRequest expected_request; PaymentRequest output_request; @@ -82,12 +193,14 @@ EXPECT_EQ(expected_request, output_request); // If payment details are present, parse those as well. + expected_request.details.total.label = base::ASCIIToUTF16("TOTAL"); expected_request.details.total.amount.currency = base::ASCIIToUTF16("GBP"); expected_request.details.total.amount.value = base::ASCIIToUTF16("6.66"); std::unique_ptr<base::DictionaryValue> details_dict( new base::DictionaryValue); std::unique_ptr<base::DictionaryValue> total_dict(new base::DictionaryValue); + total_dict->SetString("label", "TOTAL"); std::unique_ptr<base::DictionaryValue> amount_dict(new base::DictionaryValue); amount_dict->SetString("currency", "GBP"); amount_dict->SetString("value", "6.66"); @@ -311,6 +424,11 @@ EXPECT_NE(item1, item2); item2.amount.value = base::ASCIIToUTF16("104.34"); EXPECT_EQ(item1, item2); + + item1.pending = true; + EXPECT_NE(item1, item2); + item2.pending = true; + EXPECT_EQ(item1, item2); } // Tests that two shipping option objects are not equal if their property values
diff --git a/ios/web/public/payments/payment_request.h b/ios/web/public/payments/payment_request.h index 177cb39d..d295c2f 100644 --- a/ios/web/public/payments/payment_request.h +++ b/ios/web/public/payments/payment_request.h
@@ -88,6 +88,10 @@ bool operator==(const PaymentMethodData& other) const; bool operator!=(const PaymentMethodData& other) const; + // Populates the properties of this PaymentMethodData from |value|. Returns + // true if the required values are present. + bool FromDictionaryValue(const base::DictionaryValue& value); + // Payment method identifiers for payment methods that the merchant web site // accepts. std::vector<base::string16> supported_methods; @@ -106,6 +110,10 @@ bool operator==(const PaymentCurrencyAmount& other) const; bool operator!=(const PaymentCurrencyAmount& other) const; + // Populates the properties of this PaymentCurrencyAmount from |value|. + // Returns true if the required values are present. + bool FromDictionaryValue(const base::DictionaryValue& value); + // A currency identifier. The most common identifiers are three-letter // alphabetic codes as defined by ISO 4217 (for example, "USD" for US Dollars) // however any string is considered valid. @@ -125,11 +133,20 @@ bool operator==(const PaymentItem& other) const; bool operator!=(const PaymentItem& other) const; + // Populates the properties of this PaymentItem from |value|. Returns true if + // the required values are present. + bool FromDictionaryValue(const base::DictionaryValue& value); + // A human-readable description of the item. base::string16 label; // The monetary amount for the item. PaymentCurrencyAmount amount; + + // When set to true this flag means that the amount field is not final. This + // is commonly used to show items such as shipping or tax amounts that depend + // upon selection of shipping address or shipping option. + bool pending; }; // Information describing a shipping option. @@ -198,6 +215,10 @@ bool operator==(const PaymentDetails& other) const; bool operator!=(const PaymentDetails& other) const; + // Populates the properties of this PaymentDetails from |value|. Returns true + // if the required values are present. + bool FromDictionaryValue(const base::DictionaryValue& value); + // The total amount of the payment request. PaymentItem total; @@ -252,7 +273,8 @@ bool operator==(const PaymentRequest& other) const; bool operator!=(const PaymentRequest& other) const; - // Populates the properties of this PaymentRequest from |value|. + // Populates the properties of this PaymentRequest from |value|. Returns true + // if the required values are present. bool FromDictionaryValue(const base::DictionaryValue& value); // Properties set in order to communicate user choices back to the page.
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/common/system/executive_mock.py b/third_party/WebKit/Tools/Scripts/webkitpy/common/system/executive_mock.py index 24afeac..82ea5018 100644 --- a/third_party/WebKit/Tools/Scripts/webkitpy/common/system/executive_mock.py +++ b/third_party/WebKit/Tools/Scripts/webkitpy/common/system/executive_mock.py
@@ -53,9 +53,11 @@ return None return self.returncode -# FIXME: This should be unified with MockExecutive2 + def communicate(self, *_): + return (self.stdout.getvalue(), self.stderr.getvalue()) +# FIXME: This should be unified with MockExecutive2 (http://crbug.com/626115). class MockExecutive(object): PIPE = "MOCK PIPE" STDOUT = "MOCK STDOUT" @@ -72,8 +74,9 @@ self._should_return_zero_when_run = should_return_zero_when_run or set() # FIXME: Once executive wraps os.getpid() we can just use a static pid for "this" process. self._running_pids = {'test-webkitpy': os.getpid()} - self._proc = None self.calls = [] + self._output = "MOCK output of child process" + self._proc = None def check_running_pid(self, pid): return pid in self._running_pids.values() @@ -116,18 +119,17 @@ if input: input_string = ", input=%s" % input _log.info("MOCK run_command: %s, cwd=%s%s%s", args, cwd, env_string, input_string) - output = "MOCK output of child process" if self._should_throw_when_run.intersection(args): raise ScriptError("Exception for %s" % args, output="MOCK command output") if self._should_throw: - raise ScriptError("MOCK ScriptError", output=output) + raise ScriptError("MOCK ScriptError", output=self._output) if return_exit_code and self._should_return_zero_when_run.intersection(args): return 0 - return output + return self._output def cpu_count(self): return 2 @@ -150,7 +152,7 @@ env_string = ", env=%s" % env _log.info("MOCK popen: %s%s%s", args, cwd_string, env_string) if not self._proc: - self._proc = MockProcess() + self._proc = MockProcess(self._output) return self._proc def call(self, args, **kwargs): @@ -183,12 +185,12 @@ """MockExecutive2 is like MockExecutive except it doesn't log anything.""" def __init__(self, output='', exit_code=0, exception=None, run_command_fn=None, stderr=''): + super(MockExecutive2, self).__init__() self._output = output self._stderr = stderr self._exit_code = exit_code self._exception = exception self._run_command_fn = run_command_fn - self.calls = [] def run_command(self, args,
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py index be88f0b..f20de4f3 100644 --- a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py +++ b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py
@@ -326,16 +326,32 @@ def _upload_cl(self): _log.info('Uploading change list.') cc_list = self.get_directory_owners_to_cc() - last_commit_message = self.check_run(['git', 'log', '-1', '--format=%B']) - commit_message = last_commit_message + 'TBR=qyearsley@chromium.org' + description = self._cl_description() self.git_cl.run([ 'upload', '-f', '--rietveld', '-m', - commit_message, + description, ] + ['--cc=' + email for email in cc_list]) + def _cl_description(self): + description = self.check_run(['git', 'log', '-1', '--format=%B']) + build_link = self._build_link() + if build_link: + description += 'Build: %s\n\n' % build_link + description += 'TBR=qyearsley@chromium.org' + return description + + def _build_link(self): + """Returns a link to a job, if running on buildbot.""" + master_name = self.host.environ.get('BUILDBOT_MASTERNAME') + builder_name = self.host.environ.get('BUILDBOT_BUILDERNAME') + build_number = self.host.environ.get('BUILDBOT_BUILDNUMBER') + if not (master_name and builder_name and build_number): + return None + return 'https://build.chromium.org/p/%s/builders/%s/builds/%s' % (master_name, builder_name, build_number) + def get_directory_owners_to_cc(self): """Returns a list of email addresses to CC for the current import.""" _log.info('Gathering directory owners emails to CC.')
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater_unittest.py b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater_unittest.py index 1752a90..5ebfb63 100644 --- a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater_unittest.py +++ b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater_unittest.py
@@ -6,7 +6,7 @@ from webkitpy.w3c.deps_updater import DepsUpdater from webkitpy.common.host_mock import MockHost -from webkitpy.common.system.filesystem_mock import MockFileSystem +from webkitpy.common.system.executive_mock import MockExecutive2 class DepsUpdaterTest(unittest.TestCase): @@ -59,3 +59,31 @@ host.filesystem.read_text_file('/mock-checkout/third_party/WebKit/LayoutTests/TestExpectations'), ('Bug(test) new/a.html [ Failure ]\n' 'Bug(test) new/c.html [ Failure ]\n')) + + # Tests for protected methods - pylint: disable=protected-access + + def test_cl_description_with_empty_environ(self): + host = MockHost() + host.executive = MockExecutive2(output='Last commit message\n') + updater = DepsUpdater(host) + description = updater._cl_description() + self.assertEqual( + description, + ('Last commit message\n' + 'TBR=qyearsley@chromium.org')) + self.assertEqual(host.executive.calls, [['git', 'log', '-1', '--format=%B']]) + + def test_cl_description_with_environ_variables(self): + host = MockHost() + host.executive = MockExecutive2(output='Last commit message\n') + updater = DepsUpdater(host) + updater.host.environ['BUILDBOT_MASTERNAME'] = 'my.master' + updater.host.environ['BUILDBOT_BUILDERNAME'] = 'b' + updater.host.environ['BUILDBOT_BUILDNUMBER'] = '123' + description = updater._cl_description() + self.assertEqual( + description, + ('Last commit message\n' + 'Build: https://build.chromium.org/p/my.master/builders/b/builds/123\n\n' + 'TBR=qyearsley@chromium.org')) + self.assertEqual(host.executive.calls, [['git', 'log', '-1', '--format=%B']])