Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2602213002:
[PaymentApp] Fix a crash when enabledMethods called. (Closed)
|
Created:
3 years, 11 months ago by Hwanseung Lee Modified:
3 years, 11 months ago CC:
chromium-reviews, blink-reviews, haraken, sebsg+paymentswatch_chromium.org, gogerald+paymentswatch_chromium.org, rouslan+payments_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[PaymentApp] Fix a crash when enabledMethods called.
The spec says enabled_methods in PaymentOptions is not required,
but if we skip the field, a crash occurs in the current implementation.
So, add a check logic whether the field is omitted or not.
Spec:
https://w3c.github.io/webpayments-payment-apps-api/#idl-def-paymentappoption
BUG=661608
Committed: https://crrev.com/42a9f508444a30bde35c51a4d3854cde070febb3
Cr-Commit-Position: refs/heads/master@{#441556}
Patch Set 1 #Patch Set 2 : [Payment] fix a crash when enabledMethods called. #
Messages
Total messages: 27 (19 generated)
The CQ bit was checked by hs1217.lee@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
hs1217.lee@samsung.com changed reviewers: + jinho.bang@samsung.com
@zino PTAL. thank you.
Please add a test to reproduce the issue.
Description was changed from ========== [PaymentApp] fix a crash when enabledMethods called. enabledMethods is not required field. when enabledMethods is not set and it called, crash was ocurred. so check whether enabledMethods was set or not to prevent crash. BUG=661608 ========== to ========== [PaymentApp] fix a crash when enabledMethods called. enabledMethods is not required member. when enabledMethods is not set and it called, crash was ocurred. so check whether enabledMethods was set or not to prevent crash. BUG=661608 ==========
The CQ bit was checked by hs1217.lee@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2017/01/02 12:21:14, zino wrote: > Please add a test to reproduce the issue. @zino PTAL i added a test. thank you.
jinho.bang@samsung.com changed reviewers: + rouslan@chromium.org
lgtm but the CL description and test description looks confusing to me. Could you please update the descriptions clearer? For example, The spec says enabled_methods in PaymentOptions is not required, but if we skip the field, a crash occurs in the current implementation. So, add a check logic whether the field is omitted or not.
Description was changed from ========== [PaymentApp] fix a crash when enabledMethods called. enabledMethods is not required member. when enabledMethods is not set and it called, crash was ocurred. so check whether enabledMethods was set or not to prevent crash. BUG=661608 ========== to ========== [PaymentApp] fix a crash when enabledMethods called. enabledMethods is not required member. When enabledMethods is not set and it called, crash was ocurred. So check whether enabledMethods was set or not to prevent crash. BUG=661608 ==========
Description was changed from ========== [PaymentApp] fix a crash when enabledMethods called. enabledMethods is not required member. When enabledMethods is not set and it called, crash was ocurred. So check whether enabledMethods was set or not to prevent crash. BUG=661608 ========== to ========== [PaymentApp] Fix a crash when enabledMethods called. enabledMethods is not required member. When enabledMethods is not set and it called, crash was ocurred. So check whether enabledMethods was set or not to prevent crash. BUG=661608 ==========
Description was changed from ========== [PaymentApp] Fix a crash when enabledMethods called. enabledMethods is not required member. When enabledMethods is not set and it called, crash was ocurred. So check whether enabledMethods was set or not to prevent crash. BUG=661608 ========== to ========== [PaymentApp] Fix a crash when enabledMethods called. enabledMethods is not required member. When enabledMethods is not set and it called, crash was ocurred. So check whether enabledMethods was set or not to prevent crash. Spec: https://w3c.github.io/webpayments-payment-apps-api/#idl-def-paymentappoption BUG=661608 ==========
lgtm
Description was changed from ========== [PaymentApp] Fix a crash when enabledMethods called. enabledMethods is not required member. When enabledMethods is not set and it called, crash was ocurred. So check whether enabledMethods was set or not to prevent crash. Spec: https://w3c.github.io/webpayments-payment-apps-api/#idl-def-paymentappoption BUG=661608 ========== to ========== [PaymentApp] Fix a crash when enabledMethods called. The spec says enabled_methods in PaymentOptions is not required, but if we skip the field, a crash occurs in the current implementation. So, add a check logic whether the field is omitted or not. Spec: https://w3c.github.io/webpayments-payment-apps-api/#idl-def-paymentappoption BUG=661608 ==========
The CQ bit was checked by hs1217.lee@samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1483573395982630,
"parent_rev": "0d4bb556a5e714606e3382b1c58dbbbb8f288e41", "commit_rev":
"cebb69ed04aff08f7bcc3c8535c2039f03c74c2b"}
Message was sent while issue was closed.
Description was changed from ========== [PaymentApp] Fix a crash when enabledMethods called. The spec says enabled_methods in PaymentOptions is not required, but if we skip the field, a crash occurs in the current implementation. So, add a check logic whether the field is omitted or not. Spec: https://w3c.github.io/webpayments-payment-apps-api/#idl-def-paymentappoption BUG=661608 ========== to ========== [PaymentApp] Fix a crash when enabledMethods called. The spec says enabled_methods in PaymentOptions is not required, but if we skip the field, a crash occurs in the current implementation. So, add a check logic whether the field is omitted or not. Spec: https://w3c.github.io/webpayments-payment-apps-api/#idl-def-paymentappoption BUG=661608 Review-Url: https://codereview.chromium.org/2602213002 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== [PaymentApp] Fix a crash when enabledMethods called. The spec says enabled_methods in PaymentOptions is not required, but if we skip the field, a crash occurs in the current implementation. So, add a check logic whether the field is omitted or not. Spec: https://w3c.github.io/webpayments-payment-apps-api/#idl-def-paymentappoption BUG=661608 Review-Url: https://codereview.chromium.org/2602213002 ========== to ========== [PaymentApp] Fix a crash when enabledMethods called. The spec says enabled_methods in PaymentOptions is not required, but if we skip the field, a crash occurs in the current implementation. So, add a check logic whether the field is omitted or not. Spec: https://w3c.github.io/webpayments-payment-apps-api/#idl-def-paymentappoption BUG=661608 Committed: https://crrev.com/42a9f508444a30bde35c51a4d3854cde070febb3 Cr-Commit-Position: refs/heads/master@{#441556} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/42a9f508444a30bde35c51a4d3854cde070febb3 Cr-Commit-Position: refs/heads/master@{#441556} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
