[webnfc] Add public interfaces.

[webnfc] Add public interfaces.

Add public interfaces for embedder.
WebNFCClient.h
WebNFCError.h
WebNFCMessage.h
WebNFCObserver.h
WebNFCPushOptions.h
WebNFCPushTarget.h
WebNFCWatchOptions.h

BUG=520391
Committed: https://crrev.com/7fe6f67a165a2b5a5c1a574add1af61f4a70b25f
Cr-Commit-Position: refs/heads/master@{#363465}

[shalamov, 2015-11-30 15:20:25]

alexander.shalamov@intel.com changed reviewers:
+ jyasskin@chromium.org, kenneth.christiansen@gmail.com,
kenneth.r.christiansen@intel.com, rijubrata.bhaumik@intel.com

[shalamov, 2015-11-30 15:20:25]

This patch adds public interfaces that are required for interfacing between
embedder <-> blink. Follow-up patch uses new interfaces crrev.com/1484673003

PTAL

[kenneth.christiansen, 2015-11-30 15:35:51]

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
File third_party/WebKit/public/platform/modules/nfc/WebNFCClient.h (right):

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/modules/nfc/WebNFCClient.h:30: // Sets
observer to the client so that the client can notify observer about
s/the client/it/
s/observer/the observer/
s/watch/watch events/

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/modules/nfc/WebNFCClient.h:34: // NFC
interface method.
Why not just group them and write

// NFC interface methods:

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/modules/nfc/WebNFCClient.h:36: //
WebNFCCallbacks ownership is transfered to client.
the client

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
File third_party/WebKit/public/platform/modules/nfc/WebNFCError.h (right):

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/modules/nfc/WebNFCError.h:10: // Errors that
may occur during WebNFC execution.
...and what they map to.

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
File third_party/WebKit/public/platform/modules/nfc/WebNFCMessage.h (right):

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/modules/nfc/WebNFCMessage.h:35: : data(data)
shouldn't the : not be indented 4 chars more?

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
File third_party/WebKit/public/platform/modules/nfc/WebNFCPushOptions.h (right):

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/modules/nfc/WebNFCPushOptions.h:20: double
timeout; // in milliseconds
Why not call it timeoutMillis or timeoutMS - I think that is used elsewhere -
you can check

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
File third_party/WebKit/public/platform/modules/nfc/WebNFCWatchOptions.h
(right):

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/modules/nfc/WebNFCWatchOptions.h:9: #include
"public/platform/modules/nfc/WebNFCMessage.h"
do you need the full include path here?

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/modules/nfc/WebNFCWatchOptions.h:23:
WebNFCRecordTypeFilter() : m_matchAnyRecord(true) {}
not sure this is changed in blink, but in WebKit we used { } (with one space)

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/modules/nfc/WebNFCWatchOptions.h:39: :
url(url), recordFilter(recordFilter), mediaType(mediaType), mode(mode)
would be nicer with these on separate lines

: ...
, ...
, ...

[shalamov, 2015-12-01 07:57:53]

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
File third_party/WebKit/public/platform/modules/nfc/WebNFCClient.h (right):

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/modules/nfc/WebNFCClient.h:30: // Sets
observer to the client so that the client can notify observer about
On 2015/11/30 15:35:50, kenneth.christiansen wrote:
> s/the client/it/
> s/observer/the observer/
> s/watch/watch events/

Done.

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/modules/nfc/WebNFCClient.h:34: // NFC
interface method.
On 2015/11/30 15:35:50, kenneth.christiansen wrote:
> Why not just group them and write
> 
> // NFC interface methods:

Done.

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/modules/nfc/WebNFCClient.h:36: //
WebNFCCallbacks ownership is transfered to client.
On 2015/11/30 15:35:50, kenneth.christiansen wrote:
> the client

Done.

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
File third_party/WebKit/public/platform/modules/nfc/WebNFCError.h (right):

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/modules/nfc/WebNFCError.h:10: // Errors that
may occur during WebNFC execution.
On 2015/11/30 15:35:50, kenneth.christiansen wrote:
> ...and what they map to.

To the specification.

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
File third_party/WebKit/public/platform/modules/nfc/WebNFCMessage.h (right):

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/modules/nfc/WebNFCMessage.h:35: : data(data)
On 2015/11/30 15:35:50, kenneth.christiansen wrote:
> shouldn't the : not be indented 4 chars more?

Style checker didn't complain, looks fine to me.

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
File third_party/WebKit/public/platform/modules/nfc/WebNFCPushOptions.h (right):

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/modules/nfc/WebNFCPushOptions.h:20: double
timeout; // in milliseconds
On 2015/11/30 15:35:50, kenneth.christiansen wrote:
> Why not call it timeoutMillis or timeoutMS - I think that is used elsewhere -
> you can check

I can fix that if you want, but I prefer to have members named in same way as in
the spec, easier to follow during implementation.

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
File third_party/WebKit/public/platform/modules/nfc/WebNFCWatchOptions.h
(right):

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/modules/nfc/WebNFCWatchOptions.h:9: #include
"public/platform/modules/nfc/WebNFCMessage.h"
On 2015/11/30 15:35:51, kenneth.christiansen wrote:
> do you need the full include path here?

Done.

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/modules/nfc/WebNFCWatchOptions.h:23:
WebNFCRecordTypeFilter() : m_matchAnyRecord(true) {}
On 2015/11/30 15:35:51, kenneth.christiansen wrote:
> not sure this is changed in blink, but in WebKit we used { } (with one space)

Done.

https://codereview.chromium.org/1481763005/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/modules/nfc/WebNFCWatchOptions.h:39: :
url(url), recordFilter(recordFilter), mediaType(mediaType), mode(mode)
On 2015/11/30 15:35:50, kenneth.christiansen wrote:
> would be nicer with these on separate lines
> 
> : ...
> , ...
> , ...

Done.

[kenneth.christiansen, 2015-12-01 09:21:18]

lgtm

[kenneth.r.christiansen, 2015-12-01 09:24:12]

lgtm

[riju_, 2015-12-01 09:52:19]

https://codereview.chromium.org/1481763005/diff/20001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/modules/nfc/WebNFCError.h (right):

https://codereview.chromium.org/1481763005/diff/20001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/nfc/WebNFCError.h:26: CannotCancel,
Just a comment : I understand that you will map these in NFCError.cpp, but apart
from DeviceDisabled which is clubbed with NotSupported, there is no other error
which is "grouped". But I am guessing you need to differentiate between
NfcDisabled and NotSupported(in terms of error message), hence this MAP_ERROR
way, instead of switch-case i guess?

[shalamov, 2015-12-01 12:00:49]

On 2015/12/01 09:52:19, riju_ wrote:
>
https://codereview.chromium.org/1481763005/diff/20001/third_party/WebKit/publ...
> File third_party/WebKit/public/platform/modules/nfc/WebNFCError.h (right):
> 
>
https://codereview.chromium.org/1481763005/diff/20001/third_party/WebKit/publ...
> third_party/WebKit/public/platform/modules/nfc/WebNFCError.h:26: CannotCancel,
> Just a comment : I understand that you will map these in NFCError.cpp, but
apart
> from DeviceDisabled which is clubbed with NotSupported, there is no other
error
> which is "grouped". But I am guessing you need to differentiate between
> NfcDisabled and NotSupported(in terms of error message), hence this MAP_ERROR
> way, instead of switch-case i guess?

As we've agreed with Kenneth and Zoltan, at the moment DeviceDisabled and
NotSupported would be mapped to NotSupportedError DOM exception to avoid
fingerprinting.
Error messages would be the same for DeviceDisabled and NotSupported NFC errors.

[riju_, 2015-12-01 12:52:56]

lgtm :)

[riju_, 2015-12-01 12:55:33]

On 2015/12/01 12:52:56, riju_ wrote:
> lgtm :)

public/platform/modules/nfc/ needs an OWNERS file.

[shalamov, 2015-12-02 07:34:35]

The CQ bit was checked by alexander.shalamov@intel.com

[shalamov, 2015-12-02 07:34:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
rijubrata.bhaumik@intel.com, kenneth.r.christiansen@intel.com,
kenneth.christiansen@gmail.com
Link to the patchset: https://codereview.chromium.org/1481763005/#ps40001
(title: "Added OWNERS file")

[commit-bot: I haz the power, 2015-12-02 07:34:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1481763005/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1481763005/40001

[commit-bot: I haz the power, 2015-12-02 07:42:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-02 07:42:31]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[shalamov, 2015-12-03 08:20:03]

Description was changed from

==========
[webnfc] Add public interfaces.

Add public interfaces for embedder.
WebNFCClient.h
WebNFCError.h
WebNFCMessage.h
WebNFCObserver.h
WebNFCPushOptions.h
WebNFCPushTarget.h
WebNFCWatchOptions.h

BUG=520391
==========

to

==========
[webnfc] Add public interfaces.

Add public interfaces for embedder.
WebNFCClient.h
WebNFCError.h
WebNFCMessage.h
WebNFCObserver.h
WebNFCPushOptions.h
WebNFCPushTarget.h
WebNFCWatchOptions.h

BUG=520391
==========

[shalamov, 2015-12-03 08:20:03]

alexander.shalamov@intel.com changed reviewers:
+ esprehn@chromium.org

[shalamov, 2015-12-07 09:06:45]

alexander.shalamov@intel.com changed reviewers:
+ jochen@chromium.org, tkent@chromium.org

[shalamov, 2015-12-07 09:07:31]

alexander.shalamov@intel.com changed reviewers:
+ mkwst@chromium.org

[Mike West, 2015-12-07 09:49:07]

On 2015/12/02 at 07:42:31, commit-bot wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

Can I assume these will be locked behind a suitable runtime flag until such time
as you're ready to ship? If so, could you add the flag and the IDL files here as
well?

[shalamov, 2015-12-07 10:53:58]

On 2015/12/07 09:49:07, Mike West wrote:
> On 2015/12/02 at 07:42:31, commit-bot wrote:
> > Try jobs failed on following builders:
> >   chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
> 
> Can I assume these will be locked behind a suitable runtime flag until such
time
> as you're ready to ship? If so, could you add the flag and the IDL files here
as
> well?

Feature is already under WebNFC runtime flag.
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

Idl interfaces also guarded by RuntimeEnabled=WebNFC
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

Is that enough?

[Mike West, 2015-12-07 10:56:21]

On 2015/12/07 at 10:53:58, alexander.shalamov wrote:
> Feature is already under WebNFC runtime flag.
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> 
> Idl interfaces also guarded by RuntimeEnabled=WebNFC
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> 
> Is that enough?

Yup. I misread the CL as introducing the module, not the public interface for
the module. Adding that seems totally reasonable. LGTM.

[shalamov, 2015-12-07 11:11:02]

The CQ bit was checked by alexander.shalamov@intel.com

[commit-bot: I haz the power, 2015-12-07 11:11:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1481763005/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1481763005/40001

[commit-bot: I haz the power, 2015-12-07 13:22:19]

Description was changed from

==========
[webnfc] Add public interfaces.

Add public interfaces for embedder.
WebNFCClient.h
WebNFCError.h
WebNFCMessage.h
WebNFCObserver.h
WebNFCPushOptions.h
WebNFCPushTarget.h
WebNFCWatchOptions.h

BUG=520391
==========

to

==========
[webnfc] Add public interfaces.

Add public interfaces for embedder.
WebNFCClient.h
WebNFCError.h
WebNFCMessage.h
WebNFCObserver.h
WebNFCPushOptions.h
WebNFCPushTarget.h
WebNFCWatchOptions.h

BUG=520391
==========

[commit-bot: I haz the power, 2015-12-07 13:22:20]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-12-07 13:23:01]

Description was changed from

==========
[webnfc] Add public interfaces.

Add public interfaces for embedder.
WebNFCClient.h
WebNFCError.h
WebNFCMessage.h
WebNFCObserver.h
WebNFCPushOptions.h
WebNFCPushTarget.h
WebNFCWatchOptions.h

BUG=520391
==========

to

==========
[webnfc] Add public interfaces.

Add public interfaces for embedder.
WebNFCClient.h
WebNFCError.h
WebNFCMessage.h
WebNFCObserver.h
WebNFCPushOptions.h
WebNFCPushTarget.h
WebNFCWatchOptions.h

BUG=520391

Committed: https://crrev.com/7fe6f67a165a2b5a5c1a574add1af61f4a70b25f
Cr-Commit-Position: refs/heads/master@{#363465}
==========

[commit-bot: I haz the power, 2015-12-07 13:23:02]

Patchset 3 (id:??) landed as
https://crrev.com/7fe6f67a165a2b5a5c1a574add1af61f4a70b25f
Cr-Commit-Position: refs/heads/master@{#363465}