Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(57)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1444303002: [webnfc] Align NFC interface with latest specification. (Closed)

Created:
5 years, 1 month ago by shalamov
Modified:
5 years, 1 month ago
CC:
blink-reviews, chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[webnfc] Align NFC interface with latest specification. Based on review feedback, WebNFC specification was modified and requestAdapter() method was removed from NFC interface. Methods from NFCAdapter were moved to NFC interface. This patch adds new methods to NFC interface and aligns corresponding idl files with latest specification. https://w3c.github.io/web-nfc/#dom-nfc-push https://w3c.github.io/web-nfc/#dom-nfc-cancelpush https://w3c.github.io/web-nfc/#dom-nfc-watch https://w3c.github.io/web-nfc/#dom-nfc-cancelwatch BUG=520391 Committed: https://crrev.com/a59c14f4aededac3580756cec407d587f3d4351b Cr-Commit-Position: refs/heads/master@{#361085}

Patch Set 1 #

Patch Set 2 : Use default destructor for !ENABLE(OILPAN) builds #

Total comments: 12

Patch Set 3 : Fix review comments from Riju #

Patch Set 4 : Add virtual destructor to MessageCallback #

Patch Set 5 : Forward declare NFCMessage in MessageCallback.h #

Total comments: 8

Patch Set 6 : Pass NFCMessage by const ref to callback method #

Patch Set 7 : Fixes for review comments from Kenneth #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+131 lines, -32 lines) Patch
M third_party/WebKit/LayoutTests/nfc/idl-NFC.html View 1 chunk +9 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt View 1 2 3 4 5 1 chunk +4 lines, -1 line 0 comments Download
M third_party/WebKit/Source/modules/modules.gypi View 3 chunks +4 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/modules/nfc/MessageCallback.h View 1 2 3 4 5 1 chunk +23 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/modules/nfc/MessageCallback.idl View 1 chunk +9 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/modules/nfc/NFC.h View 1 2 3 4 5 6 2 chunks +23 lines, -7 lines 1 comment Download
M third_party/WebKit/Source/modules/nfc/NFC.cpp View 1 2 2 chunks +25 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/modules/nfc/NFC.idl View 1 chunk +7 lines, -2 lines 0 comments Download
A third_party/WebKit/Source/modules/nfc/NFCMessage.idl View 1 chunk +10 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/modules/nfc/NFCPushOptions.idl View 1 chunk +5 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/modules/nfc/NFCRecord.idl View 1 chunk +7 lines, -8 lines 0 comments Download
M third_party/WebKit/Source/modules/nfc/NFCWatchOptions.idl View 1 chunk +5 lines, -7 lines 0 comments Download

Messages

Total messages: 22 (7 generated)
shalamov
PTAL
5 years, 1 month ago (2015-11-16 14:30:06 UTC) #3
riju_
Thanks Alex, minor nits, else LGTM. https://codereview.chromium.org/1444303002/diff/20001/third_party/WebKit/Source/modules/nfc/MessageCallback.h File third_party/WebKit/Source/modules/nfc/MessageCallback.h (right): https://codereview.chromium.org/1444303002/diff/20001/third_party/WebKit/Source/modules/nfc/MessageCallback.h#newcode17 third_party/WebKit/Source/modules/nfc/MessageCallback.h:17: virtual void handleMessage(NFCMessage) ...
5 years, 1 month ago (2015-11-16 15:05:01 UTC) #4
shalamov
https://codereview.chromium.org/1444303002/diff/20001/third_party/WebKit/Source/modules/nfc/MessageCallback.h File third_party/WebKit/Source/modules/nfc/MessageCallback.h (right): https://codereview.chromium.org/1444303002/diff/20001/third_party/WebKit/Source/modules/nfc/MessageCallback.h#newcode17 third_party/WebKit/Source/modules/nfc/MessageCallback.h:17: virtual void handleMessage(NFCMessage) = 0; On 2015/11/16 15:05:01, riju_ ...
5 years, 1 month ago (2015-11-17 08:21:18 UTC) #5
riju_
https://codereview.chromium.org/1444303002/diff/20001/third_party/WebKit/Source/modules/nfc/MessageCallback.h File third_party/WebKit/Source/modules/nfc/MessageCallback.h (right): https://codereview.chromium.org/1444303002/diff/20001/third_party/WebKit/Source/modules/nfc/MessageCallback.h#newcode15 third_party/WebKit/Source/modules/nfc/MessageCallback.h:15: MessageCallback() { } If you inherit from GarbageCollectedFinalized, a ...
5 years, 1 month ago (2015-11-17 08:52:32 UTC) #6
shalamov
https://codereview.chromium.org/1444303002/diff/20001/third_party/WebKit/Source/modules/nfc/MessageCallback.h File third_party/WebKit/Source/modules/nfc/MessageCallback.h (right): https://codereview.chromium.org/1444303002/diff/20001/third_party/WebKit/Source/modules/nfc/MessageCallback.h#newcode15 third_party/WebKit/Source/modules/nfc/MessageCallback.h:15: MessageCallback() { } On 2015/11/17 08:52:32, riju_ wrote: > ...
5 years, 1 month ago (2015-11-17 09:04:55 UTC) #7
kenneth.r.christiansen
https://codereview.chromium.org/1444303002/diff/80001/third_party/WebKit/Source/modules/nfc/MessageCallback.h File third_party/WebKit/Source/modules/nfc/MessageCallback.h (right): https://codereview.chromium.org/1444303002/diff/80001/third_party/WebKit/Source/modules/nfc/MessageCallback.h#newcode14 third_party/WebKit/Source/modules/nfc/MessageCallback.h:14: class MessageCallback : public GarbageCollectedFinalized<MessageCallback> { You need to ...
5 years, 1 month ago (2015-11-18 16:37:08 UTC) #8
shalamov
https://codereview.chromium.org/1444303002/diff/80001/third_party/WebKit/Source/modules/nfc/MessageCallback.h File third_party/WebKit/Source/modules/nfc/MessageCallback.h (right): https://codereview.chromium.org/1444303002/diff/80001/third_party/WebKit/Source/modules/nfc/MessageCallback.h#newcode14 third_party/WebKit/Source/modules/nfc/MessageCallback.h:14: class MessageCallback : public GarbageCollectedFinalized<MessageCallback> { On 2015/11/18 16:37:08, ...
5 years, 1 month ago (2015-11-19 08:26:08 UTC) #9
shalamov
https://codereview.chromium.org/1444303002/diff/20001/third_party/WebKit/Source/modules/nfc/NFC.cpp File third_party/WebKit/Source/modules/nfc/NFC.cpp (right): https://codereview.chromium.org/1444303002/diff/20001/third_party/WebKit/Source/modules/nfc/NFC.cpp#newcode42 third_party/WebKit/Source/modules/nfc/NFC.cpp:42: // todo(shalamov): To be implemented On 2015/11/16 15:05:01, riju_ ...
5 years, 1 month ago (2015-11-19 12:59:06 UTC) #11
sof
https://codereview.chromium.org/1444303002/diff/120001/third_party/WebKit/Source/modules/nfc/NFC.h File third_party/WebKit/Source/modules/nfc/NFC.h (right): https://codereview.chromium.org/1444303002/diff/120001/third_party/WebKit/Source/modules/nfc/NFC.h#newcode31 third_party/WebKit/Source/modules/nfc/NFC.h:31: #if !ENABLE(OILPAN) looks fine this way wrt Oilpan.
5 years, 1 month ago (2015-11-19 13:13:13 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1444303002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1444303002/120001
5 years, 1 month ago (2015-11-20 08:10:15 UTC) #15
commit-bot: I haz the power
No L-G-T-M from a valid reviewer yet. Only full committers are accepted. Even if an ...
5 years, 1 month ago (2015-11-20 08:10:17 UTC) #17
kenneth.r.christiansen
lgtm
5 years, 1 month ago (2015-11-23 08:21:13 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1444303002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1444303002/120001
5 years, 1 month ago (2015-11-23 09:46:56 UTC) #20
commit-bot: I haz the power
Committed patchset #7 (id:120001)
5 years, 1 month ago (2015-11-23 11:05:32 UTC) #21
commit-bot: I haz the power
5 years, 1 month ago (2015-11-23 11:06:39 UTC) #22
Message was sent while issue was closed. 
Patchset 7 (id:??) landed as
https://crrev.com/a59c14f4aededac3580756cec407d587f3d4351b
Cr-Commit-Position: refs/heads/master@{#361085}

Powered by Google App Engine
This is Rietveld 408576698