Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(3)

Issue 2885813002: [webnfc] Align nfc.push operation with the specification (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 months ago by shalamov
Modified:
4 months, 4 weeks ago
CC:
chromium-reviews, blink-reviews, haraken
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[webnfc] Align nfc.push operation with the specification This CL aligns error handling for nfc.push operation, with the latest changes in the specification. Whenever invlid type is provided, resulting promise is rejected with 'TypeError' exception. When error happens due to the invalid sytnax or tokens, resulting promise is rejected with 'SyntaxError' exception. Layout test updated accordingly. BUG=718349 Review-Url: https://codereview.chromium.org/2885813002 Cr-Commit-Position: refs/heads/master@{#474265} Committed: https://chromium.googlesource.com/chromium/src/+/f33e1d034eabce26a32bf9b05f3ca29a924fd237

Patch Set 1 #

Total comments: 9

Patch Set 2 : Fixes for comments + new tests. #

Total comments: 4

Patch Set 3 : Remove WTF:: namespace where not needed. Don't use MaybeLocal. #

Total comments: 2

Patch Set 4 : Removed 'the' articles #

Patch Set 5 : Rebased to master #

Messages

Total messages: 47 (33 generated)
shalamov
Could you please take a look?
5 months ago (2017-05-16 14:10:34 UTC) #4
Reilly Grant (use Gerrit)
https://codereview.chromium.org/2885813002/diff/1/third_party/WebKit/LayoutTests/nfc/push.html File third_party/WebKit/LayoutTests/nfc/push.html (right): https://codereview.chromium.org/2885813002/diff/1/third_party/WebKit/LayoutTests/nfc/push.html#newcode150 third_party/WebKit/LayoutTests/nfc/push.html:150: }, 'nfc.push with "empty" record should succeed.'); Can an ...
5 months ago (2017-05-16 15:21:04 UTC) #5
Alexander Shalamov
https://codereview.chromium.org/2885813002/diff/1/third_party/WebKit/LayoutTests/nfc/push.html File third_party/WebKit/LayoutTests/nfc/push.html (right): https://codereview.chromium.org/2885813002/diff/1/third_party/WebKit/LayoutTests/nfc/push.html#newcode150 third_party/WebKit/LayoutTests/nfc/push.html:150: }, 'nfc.push with "empty" record should succeed.'); On 2017/05/16 ...
5 months ago (2017-05-16 17:49:10 UTC) #8
shalamov
https://codereview.chromium.org/2885813002/diff/1/third_party/WebKit/Source/modules/nfc/NFC.cpp File third_party/WebKit/Source/modules/nfc/NFC.cpp (right): https://codereview.chromium.org/2885813002/diff/1/third_party/WebKit/Source/modules/nfc/NFC.cpp#newcode706 third_party/WebKit/Source/modules/nfc/NFC.cpp:706: "Cannot set WebNFC Id."); On 2017/05/16 17:49:10, Alexander Shalamov ...
5 months ago (2017-05-17 13:12:26 UTC) #9
haraken
https://codereview.chromium.org/2885813002/diff/20001/third_party/WebKit/Source/modules/nfc/NFC.cpp File third_party/WebKit/Source/modules/nfc/NFC.cpp (right): https://codereview.chromium.org/2885813002/diff/20001/third_party/WebKit/Source/modules/nfc/NFC.cpp#newcode201 third_party/WebKit/Source/modules/nfc/NFC.cpp:201: if (!stringify_result.ToLocal(&jsonString) || try_catch.HasCaught()) { I prefer avoiding using ...
5 months ago (2017-05-17 15:11:22 UTC) #14
shalamov
Thanks for looking. https://codereview.chromium.org/2885813002/diff/20001/third_party/WebKit/Source/modules/nfc/NFC.cpp File third_party/WebKit/Source/modules/nfc/NFC.cpp (right): https://codereview.chromium.org/2885813002/diff/20001/third_party/WebKit/Source/modules/nfc/NFC.cpp#newcode201 third_party/WebKit/Source/modules/nfc/NFC.cpp:201: if (!stringify_result.ToLocal(&jsonString) || try_catch.HasCaught()) { On ...
5 months ago (2017-05-18 08:34:04 UTC) #17
Reilly Grant (use Gerrit)
lgtm https://codereview.chromium.org/2885813002/diff/40001/third_party/WebKit/Source/modules/nfc/NFC.cpp File third_party/WebKit/Source/modules/nfc/NFC.cpp (right): https://codereview.chromium.org/2885813002/diff/40001/third_party/WebKit/Source/modules/nfc/NFC.cpp#newcode384 third_party/WebKit/Source/modules/nfc/NFC.cpp:384: "The data for the 'text' NFCRecords must be ...
5 months ago (2017-05-18 21:49:37 UTC) #20
shalamov
https://codereview.chromium.org/2885813002/diff/40001/third_party/WebKit/Source/modules/nfc/NFC.cpp File third_party/WebKit/Source/modules/nfc/NFC.cpp (right): https://codereview.chromium.org/2885813002/diff/40001/third_party/WebKit/Source/modules/nfc/NFC.cpp#newcode384 third_party/WebKit/Source/modules/nfc/NFC.cpp:384: "The data for the 'text' NFCRecords must be of ...
5 months ago (2017-05-19 07:03:03 UTC) #23
shalamov
On 2017/05/19 07:03:03, shalamov wrote: > https://codereview.chromium.org/2885813002/diff/40001/third_party/WebKit/Source/modules/nfc/NFC.cpp > File third_party/WebKit/Source/modules/nfc/NFC.cpp (right): > > https://codereview.chromium.org/2885813002/diff/40001/third_party/WebKit/Source/modules/nfc/NFC.cpp#newcode384 > ...
4 months, 4 weeks ago (2017-05-23 11:17:56 UTC) #26
haraken
LGTM
4 months, 4 weeks ago (2017-05-23 15:16:18 UTC) #27
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2885813002/60001
4 months, 4 weeks ago (2017-05-24 06:55:27 UTC) #30
commit-bot: I haz the power
Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds/217706) ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, ...
4 months, 4 weeks ago (2017-05-24 06:58:28 UTC) #32
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2885813002/100001
4 months, 4 weeks ago (2017-05-24 12:48:17 UTC) #44
commit-bot: I haz the power
4 months, 4 weeks ago (2017-05-24 12:52:37 UTC) #47
Message was sent while issue was closed.
Committed patchset #5 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/f33e1d034eabce26a32bf9b05f3c...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 81bcdb8aa