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

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

Created:
1 year, 1 month ago by shalamov
Modified:
1 year, 1 month 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 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+219 lines, -122 lines) Patch
M third_party/WebKit/LayoutTests/nfc/push.html View 1 9 chunks +53 lines, -13 lines 0 comments Download
M third_party/WebKit/Source/modules/nfc/NFC.h View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/modules/nfc/NFC.cpp View 1 2 3 4 14 chunks +165 lines, -108 lines 0 comments Download

Messages

Total messages: 47 (33 generated)
shalamov
Could you please take a look?
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 > ...
1 year, 1 month ago (2017-05-23 11:17:56 UTC) #26
haraken
LGTM
1 year, 1 month 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
1 year, 1 month 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, ...
1 year, 1 month 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
1 year, 1 month ago (2017-05-24 12:48:17 UTC) #44
commit-bot: I haz the power
1 year, 1 month 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...

Powered by Google App Engine
This is Rietveld 408576698