nfc: Add native NFC API definitions.

nfc: Add native NFC API definitions.

Added the headers and non-platform-specific base class definitions for
the NFC API.

BUG=chromium:316471
TEST=device_unittests

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=238336

[armansito, 2013-11-20 04:11:16]

keybuk@, please review the API and let me know what you think. The CL is large
but the majority of content consists of documentation, as the CL mostly consists
of headers with little to no implementation and a good portion of code is in
nfc_ndef_record_unittest.cc.

Hopefully, this shouldn't be tiresome to review. The main things to look at are:
1. Whether the API overall makes sense, e.g. if anything could/should be done
differently; 2. The .gyp and DEPS files.

[keybuk, 2013-11-25 19:53:00]

https://codereview.chromium.org/77563002/diff/1/device/nfc/nfc_adapter.h
File device/nfc/nfc_adapter.h (right):

https://codereview.chromium.org/77563002/diff/1/device/nfc/nfc_adapter.h#newc...
device/nfc/nfc_adapter.h:104: static scoped_refptr<NfcAdapter> GetAdapter();
For future-proofing I think it's worth moving this to an NfcAdapterFactory
class.

You may also want to consider the callback approach, while it's a little
trickier to deal with at first, you may find that it's necessary when/if someone
comes to implement Windows or OS X support for NFC - and you save them the
effort of doing that.

ie.  NfcAdapterFactory::GetAdapter(callback)
  &  NfcAdapterFactory::MaybeGetAdapter() -> scoped_refptr<NfcAdapter>

https://codereview.chromium.org/77563002/diff/1/device/nfc/nfc_adapter.h#newc...
device/nfc/nfc_adapter.h:113: virtual std::string GetIdentifier() const = 0;
Do adapters need identifiers? would there be a situation in which you'd really
want "get the adapter with identifier X?" - I think that's something you
probably just want to hide from the API.

[armansito, 2013-11-25 21:35:35]

https://codereview.chromium.org/77563002/diff/1/device/nfc/nfc_adapter.h
File device/nfc/nfc_adapter.h (right):

https://codereview.chromium.org/77563002/diff/1/device/nfc/nfc_adapter.h#newc...
device/nfc/nfc_adapter.h:104: static scoped_refptr<NfcAdapter> GetAdapter();
On 2013/11/25 19:53:00, keybuk wrote:
> For future-proofing I think it's worth moving this to an NfcAdapterFactory
> class.
> 
> You may also want to consider the callback approach, while it's a little
> trickier to deal with at first, you may find that it's necessary when/if
someone
> comes to implement Windows or OS X support for NFC - and you save them the
> effort of doing that.
> 
> ie.  NfcAdapterFactory::GetAdapter(callback)
>   &  NfcAdapterFactory::MaybeGetAdapter() -> scoped_refptr<NfcAdapter>

Makes sense. I was debating whether I should follow the BluetoothAdapterFactory
example, as I wasn't exactly sure why it was made the way it is, but figured
we'd clear it up in the code review.

As an aside, why is "NfcAdapterFactory::MaybeGetAdapter" necessary exactly? Why
not just simply use NfcAdapterFactory::GetAdapter and
NfcAdapterFactory::IsNfcAvailable?

https://codereview.chromium.org/77563002/diff/1/device/nfc/nfc_adapter.h#newc...
device/nfc/nfc_adapter.h:113: virtual std::string GetIdentifier() const = 0;
On 2013/11/25 19:53:00, keybuk wrote:
> Do adapters need identifiers? would there be a situation in which you'd really
> want "get the adapter with identifier X?" - I think that's something you
> probably just want to hide from the API.

You're right. I guess, at the moment, we won't allow interacting with specific
adapters, so might as well remove it. Done.

[keybuk, 2013-11-25 21:43:17]

On 2013/11/25 21:35:35, armansito wrote:
> https://codereview.chromium.org/77563002/diff/1/device/nfc/nfc_adapter.h
> File device/nfc/nfc_adapter.h (right):
> 
>
https://codereview.chromium.org/77563002/diff/1/device/nfc/nfc_adapter.h#newc...
> device/nfc/nfc_adapter.h:104: static scoped_refptr<NfcAdapter> GetAdapter();
> On 2013/11/25 19:53:00, keybuk wrote:
> > For future-proofing I think it's worth moving this to an NfcAdapterFactory
> > class.
> > 
> > You may also want to consider the callback approach, while it's a little
> > trickier to deal with at first, you may find that it's necessary when/if
> someone
> > comes to implement Windows or OS X support for NFC - and you save them the
> > effort of doing that.
> > 
> > ie.  NfcAdapterFactory::GetAdapter(callback)
> >   &  NfcAdapterFactory::MaybeGetAdapter() -> scoped_refptr<NfcAdapter>
> 
> Makes sense. I was debating whether I should follow the
BluetoothAdapterFactory
> example, as I wasn't exactly sure why it was made the way it is, but figured
> we'd clear it up in the code review.
> 
> As an aside, why is "NfcAdapterFactory::MaybeGetAdapter" necessary exactly?
Why
> not just simply use NfcAdapterFactory::GetAdapter and
> NfcAdapterFactory::IsNfcAvailable?
> 

I don't think MaybeGetAdapter is used anymore actually, it was used in test
cases before we switched to using Fakes - which we create directly in the test
case rather than go through AdapterFactory

So no need to follow that example, just GetAdapter with the callback

[armansito, 2013-11-25 22:01:21]

On 2013/11/25 21:43:17, keybuk wrote:
> On 2013/11/25 21:35:35, armansito wrote:
> > https://codereview.chromium.org/77563002/diff/1/device/nfc/nfc_adapter.h
> > File device/nfc/nfc_adapter.h (right):
> > 
> >
>
https://codereview.chromium.org/77563002/diff/1/device/nfc/nfc_adapter.h#newc...
> > device/nfc/nfc_adapter.h:104: static scoped_refptr<NfcAdapter> GetAdapter();
> > On 2013/11/25 19:53:00, keybuk wrote:
> > > For future-proofing I think it's worth moving this to an NfcAdapterFactory
> > > class.
> > > 
> > > You may also want to consider the callback approach, while it's a little
> > > trickier to deal with at first, you may find that it's necessary when/if
> > someone
> > > comes to implement Windows or OS X support for NFC - and you save them the
> > > effort of doing that.
> > > 
> > > ie.  NfcAdapterFactory::GetAdapter(callback)
> > >   &  NfcAdapterFactory::MaybeGetAdapter() -> scoped_refptr<NfcAdapter>
> > 
> > Makes sense. I was debating whether I should follow the
> BluetoothAdapterFactory
> > example, as I wasn't exactly sure why it was made the way it is, but figured
> > we'd clear it up in the code review.
> > 
> > As an aside, why is "NfcAdapterFactory::MaybeGetAdapter" necessary exactly?
> Why
> > not just simply use NfcAdapterFactory::GetAdapter and
> > NfcAdapterFactory::IsNfcAvailable?
> > 
> 
> I don't think MaybeGetAdapter is used anymore actually, it was used in test
> cases before we switched to using Fakes - which we create directly in the test
> case rather than go through AdapterFactory
> 
> So no need to follow that example, just GetAdapter with the callback

Done.

[armansito, 2013-11-25 22:01:58]

https://codereview.chromium.org/77563002/diff/1/device/nfc/nfc_adapter.h
File device/nfc/nfc_adapter.h (right):

https://codereview.chromium.org/77563002/diff/1/device/nfc/nfc_adapter.h#newc...
device/nfc/nfc_adapter.h:104: static scoped_refptr<NfcAdapter> GetAdapter();
On 2013/11/25 21:35:35, armansito wrote:
> On 2013/11/25 19:53:00, keybuk wrote:
> > For future-proofing I think it's worth moving this to an NfcAdapterFactory
> > class.
> > 
> > You may also want to consider the callback approach, while it's a little
> > trickier to deal with at first, you may find that it's necessary when/if
> someone
> > comes to implement Windows or OS X support for NFC - and you save them the
> > effort of doing that.
> > 
> > ie.  NfcAdapterFactory::GetAdapter(callback)
> >   &  NfcAdapterFactory::MaybeGetAdapter() -> scoped_refptr<NfcAdapter>
> 
> Makes sense. I was debating whether I should follow the
BluetoothAdapterFactory
> example, as I wasn't exactly sure why it was made the way it is, but figured
> we'd clear it up in the code review.
> 
> As an aside, why is "NfcAdapterFactory::MaybeGetAdapter" necessary exactly?
Why
> not just simply use NfcAdapterFactory::GetAdapter and
> NfcAdapterFactory::IsNfcAvailable?

Done.

[armansito, 2013-12-02 22:26:35]

ping

[keybuk, 2013-12-03 01:35:49]

https://codereview.chromium.org/77563002/diff/80001/device/nfc/nfc_adapter.h
File device/nfc/nfc_adapter.h (right):

https://codereview.chromium.org/77563002/diff/80001/device/nfc/nfc_adapter.h#...
device/nfc/nfc_adapter.h:72: virtual void TagLost(NfcAdapter*adapter, NfcTag*
tag) {}
nit: space here after *

https://codereview.chromium.org/77563002/diff/80001/device/nfc/nfc_adapter_fa...
File device/nfc/nfc_adapter_factory.cc (right):

https://codereview.chromium.org/77563002/diff/80001/device/nfc/nfc_adapter_fa...
device/nfc/nfc_adapter_factory.cc:22: }
nit: extra newline after this

[keybuk, 2013-12-03 01:35:59]

LGTM after fixing nits

[armansito, 2013-12-03 02:43:06]

https://codereview.chromium.org/77563002/diff/80001/device/nfc/nfc_adapter.h
File device/nfc/nfc_adapter.h (right):

https://codereview.chromium.org/77563002/diff/80001/device/nfc/nfc_adapter.h#...
device/nfc/nfc_adapter.h:72: virtual void TagLost(NfcAdapter*adapter, NfcTag*
tag) {}
On 2013/12/03 01:35:49, keybuk wrote:
> nit: space here after *

Done.

https://codereview.chromium.org/77563002/diff/80001/device/nfc/nfc_adapter_fa...
File device/nfc/nfc_adapter_factory.cc (right):

https://codereview.chromium.org/77563002/diff/80001/device/nfc/nfc_adapter_fa...
device/nfc/nfc_adapter_factory.cc:22: }
On 2013/12/03 01:35:49, keybuk wrote:
> nit: extra newline after this

Done.

[commit-bot: I haz the power, 2013-12-03 02:47:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/armansito@chromium.org/77563002/120001

[commit-bot: I haz the power, 2013-12-03 03:30:08]

Retried try job too often on mac_rel for step(s) app_list_unittests,
base_unittests, browser_tests, cacheinvalidation_unittests, cc_unittests,
check_deps, chromedriver2_unittests, components_unittests, content_browsertests,
content_unittests, crypto_unittests, google_apis_unittests, gpu_unittests,
interactive_ui_tests, ipc_tests, jingle_unittests, media_unittests,
message_center_unittests, nacl_integration, net_unittests, ppapi_unittests,
printing_unittests, remoting_unittests, sql_unittests, sync_integration_tests,
sync_unit_tests, telemetry_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2013-12-03 05:10:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/armansito@chromium.org/77563002/140001

[commit-bot: I haz the power, 2013-12-03 10:37:16]

Change committed as 238336