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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 days, 1 hour ago by shalamov
Modified:
3 days, 4 hours ago
Reviewers:
haraken, Reilly Grant
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

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 #

Messages

Total messages: 25 (17 generated)
shalamov
Could you please take a look?
6 days, 1 hour ago (2017-05-16 14:10:34 UTC) #4
Reilly Grant
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 ...
6 days 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 days, 21 hours 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 days, 2 hours 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 days 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 ...
4 days, 6 hours ago (2017-05-18 08:34:04 UTC) #17
Reilly Grant
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 ...
3 days, 17 hours ago (2017-05-18 21:49:37 UTC) #20
shalamov
3 days, 8 hours ago (2017-05-19 07:03:03 UTC) #23
https://codereview.chromium.org/2885813002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/nfc/NFC.cpp (right):

https://codereview.chromium.org/2885813002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/nfc/NFC.cpp:384: "The data for the 'text'
NFCRecords must be of "
On 2017/05/18 21:49:37, Reilly Grant wrote:
> "for the 'text' NFCRecords" -> "for 'text' NFCRecords"
> 
> (And below, remove the word "the".)

Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 650457f06