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

Issue 1291663004: [webnfc]: Step 1: Add NavigatorNfc, Nfc and Layout tests. (Closed)

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

Description

[webnfc]: Step 1: Add NavigatorNfc, Nfc and Layout tests. Add IDL for Navigator.Nfc and Nfc. Add the supporting .h|cpp files. Add simple Layout tests. Add nfc in webexposed layout test. Intent to Implement: https://mail.google.com/mail/u/1/#label/blink-dev/14f261c80935592e BUG=520391 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=200966

Patch Set 1 #

Total comments: 2

Patch Set 2 : Nfc->NFC, add owners file, fix haraken's comment to use create() #

Total comments: 4

Patch Set 3 : name runtime-enabled-features flag as WebNFC; add kenneth as owner #

Total comments: 9

Patch Set 4 : fix jeffrey's comments #

Patch Set 5 : d'stor ~NFC() override #

Unified diffs Side-by-side diffs Delta from patch set Stats (+228 lines, -0 lines) Patch
A LayoutTests/nfc/idl-NFC.html View 1 1 chunk +11 lines, -0 lines 0 comments Download
A LayoutTests/nfc/idl-NavigatorNFC.html View 1 1 chunk +9 lines, -0 lines 0 comments Download
M LayoutTests/webexposed/global-interface-listing-expected.txt View 1 2 3 4 2 chunks +4 lines, -0 lines 0 comments Download
M Source/modules/modules.gypi View 1 2 3 4 3 chunks +6 lines, -0 lines 0 comments Download
A Source/modules/nfc/NFC.h View 1 2 3 4 1 chunk +39 lines, -0 lines 0 comments Download
A Source/modules/nfc/NFC.cpp View 1 1 chunk +43 lines, -0 lines 0 comments Download
A Source/modules/nfc/NFC.idl View 1 2 1 chunk +13 lines, -0 lines 0 comments Download
A Source/modules/nfc/NavigatorNFC.h View 1 2 3 1 chunk +39 lines, -0 lines 0 comments Download
A Source/modules/nfc/NavigatorNFC.cpp View 1 1 chunk +49 lines, -0 lines 0 comments Download
A Source/modules/nfc/NavigatorNFC.idl View 1 2 1 chunk +11 lines, -0 lines 0 comments Download
A Source/modules/nfc/OWNERS View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
M Source/platform/RuntimeEnabledFeatures.in View 1 2 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 38 (15 generated)
riju_
This is the first step towards implement WebNFC. I am adding the people who commented ...
5 years, 4 months ago (2015-08-17 13:46:08 UTC) #3
riju_
This is the first step towards implement WebNFC. I am adding the people who commented ...
5 years, 4 months ago (2015-08-17 13:46:08 UTC) #4
riju_
5 years, 4 months ago (2015-08-17 13:53:31 UTC) #6
kenneth.r.christiansen
On 2015/08/17 at 13:53:31, rijubrata.bhaumik wrote: > The spec changed the name Nfc to NFC ...
5 years, 4 months ago (2015-08-17 14:03:50 UTC) #9
Mike West
+oilpan folks, as I always get it wrong. :) I'll defer to Kenneth, et al. ...
5 years, 4 months ago (2015-08-17 14:57:37 UTC) #12
haraken
Oilpan part LGTM https://codereview.chromium.org/1291663004/diff/1/Source/modules/nfc/NavigatorNfc.cpp File Source/modules/nfc/NavigatorNfc.cpp (right): https://codereview.chromium.org/1291663004/diff/1/Source/modules/nfc/NavigatorNfc.cpp#newcode39 Source/modules/nfc/NavigatorNfc.cpp:39: self.m_nfc->suspendIfNeeded(); It would be better to ...
5 years, 4 months ago (2015-08-17 16:25:34 UTC) #14
riju_
Thanks haraken, using Nfc::create() now. Others, PTAL. https://codereview.chromium.org/1291663004/diff/1/Source/modules/nfc/NavigatorNfc.cpp File Source/modules/nfc/NavigatorNfc.cpp (right): https://codereview.chromium.org/1291663004/diff/1/Source/modules/nfc/NavigatorNfc.cpp#newcode39 Source/modules/nfc/NavigatorNfc.cpp:39: self.m_nfc->suspendIfNeeded(); On ...
5 years, 4 months ago (2015-08-18 11:36:15 UTC) #15
kenneth.r.christiansen
lgtm https://codereview.chromium.org/1291663004/diff/20001/Source/modules/nfc/NFC.idl File Source/modules/nfc/NFC.idl (right): https://codereview.chromium.org/1291663004/diff/20001/Source/modules/nfc/NFC.idl#newcode10 Source/modules/nfc/NFC.idl:10: RuntimeEnabled=Nfc, NFC no?
5 years, 4 months ago (2015-08-18 11:44:13 UTC) #16
riju_
https://codereview.chromium.org/1291663004/diff/20001/Source/modules/nfc/NFC.idl File Source/modules/nfc/NFC.idl (right): https://codereview.chromium.org/1291663004/diff/20001/Source/modules/nfc/NFC.idl#newcode10 Source/modules/nfc/NFC.idl:10: RuntimeEnabled=Nfc, On 2015/08/18 11:44:13, kenneth.r.christiansen wrote: > NFC no? ...
5 years, 4 months ago (2015-08-18 11:56:59 UTC) #17
kenneth.r.christiansen
On 2015/08/18 at 11:56:59, rijubrata.bhaumik wrote: > https://codereview.chromium.org/1291663004/diff/20001/Source/modules/nfc/NFC.idl > File Source/modules/nfc/NFC.idl (right): > > https://codereview.chromium.org/1291663004/diff/20001/Source/modules/nfc/NFC.idl#newcode10 ...
5 years, 4 months ago (2015-08-18 12:06:33 UTC) #18
riju_
On 2015/08/18 12:06:33, kenneth.r.christiansen wrote: > On 2015/08/18 at 11:56:59, rijubrata.bhaumik wrote: > > > ...
5 years, 4 months ago (2015-08-18 12:33:57 UTC) #19
Mike West
LGTM. https://codereview.chromium.org/1291663004/diff/20001/Source/modules/nfc/NFC.idl File Source/modules/nfc/NFC.idl (right): https://codereview.chromium.org/1291663004/diff/20001/Source/modules/nfc/NFC.idl#newcode10 Source/modules/nfc/NFC.idl:10: RuntimeEnabled=Nfc, On 2015/08/18 at 11:56:59, riju_ wrote: > ...
5 years, 4 months ago (2015-08-18 12:47:21 UTC) #20
riju_
https://codereview.chromium.org/1291663004/diff/20001/Source/modules/nfc/NFC.idl File Source/modules/nfc/NFC.idl (right): https://codereview.chromium.org/1291663004/diff/20001/Source/modules/nfc/NFC.idl#newcode10 Source/modules/nfc/NFC.idl:10: RuntimeEnabled=Nfc, On 2015/08/18 12:47:21, Mike West (traveling) wrote: > ...
5 years, 4 months ago (2015-08-18 15:22:12 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1291663004/30013 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1291663004/30013
5 years, 4 months ago (2015-08-18 15:22:34 UTC) #24
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/99822)
5 years, 4 months ago (2015-08-18 17:31:49 UTC) #26
Jeffrey Yasskin
https://codereview.chromium.org/1291663004/diff/30013/Source/modules/nfc/NFC.h File Source/modules/nfc/NFC.h (right): https://codereview.chromium.org/1291663004/diff/30013/Source/modules/nfc/NFC.h#newcode25 Source/modules/nfc/NFC.h:25: NFC(ExecutionContext*, LocalFrame*); If you only intend to create NFC ...
5 years, 4 months ago (2015-08-18 17:48:59 UTC) #27
riju_
Thanks jeffrey for having a look. https://codereview.chromium.org/1291663004/diff/30013/Source/modules/nfc/NFC.h File Source/modules/nfc/NFC.h (right): https://codereview.chromium.org/1291663004/diff/30013/Source/modules/nfc/NFC.h#newcode25 Source/modules/nfc/NFC.h:25: NFC(ExecutionContext*, LocalFrame*); On ...
5 years, 4 months ago (2015-08-19 08:41:22 UTC) #28
Jeffrey Yasskin
LGTM. Oilpan folks might want to take another look because of the change from GarbageCollectedFinalized ...
5 years, 4 months ago (2015-08-19 16:40:05 UTC) #29
riju_
5 years, 4 months ago (2015-08-20 10:12:53 UTC) #30
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1291663004/70001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1291663004/70001
5 years, 4 months ago (2015-08-20 11:48:35 UTC) #32
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 4 months ago (2015-08-20 14:22:25 UTC) #34
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1291663004/70001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1291663004/70001
5 years, 4 months ago (2015-08-21 05:55:08 UTC) #37
commit-bot: I haz the power
5 years, 4 months ago (2015-08-21 05:59:26 UTC) #38
Message was sent while issue was closed.
Committed patchset #5 (id:70001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=200966

Powered by Google App Engine
This is Rietveld 408576698