[webnfc] Add default implementation for nfc mojo service.

[webnfc] Add default implementation for nfc mojo service.

This patch adds default nfc mojo service implementation.
It is used by platforms that do not support NFC functionality
or whenever nfc service implementation is not yet available.

BUG=520391
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

[shalamov, 2016-04-23 20:33:44]

Description was changed from

==========
[webnfc] Add default implementation for nfc mojo service.

This patch adds default nfc mojo service implementation.
It is used by platforms that do not support NFC functionality
or whenever nfc service implementation is not yet available.

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

to

==========
[webnfc] Add default implementation for nfc mojo service.

This patch adds default nfc mojo service implementation.
It is used by platforms that do not support NFC functionality
or whenever nfc service implementation is not yet available.

BUG=520391
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[shalamov, 2016-04-23 20:58:46]

Patchset #1 (id:1) has been deleted

[shalamov, 2016-04-23 20:58:54]

Patchset #1 (id:20001) has been deleted

[shalamov, 2016-04-23 21:20:26]

Patchset #2 (id:60001) has been deleted

[shalamov, 2016-04-24 14:02:51]

Patchset #2 (id:80001) has been deleted

[shalamov, 2016-04-24 19:58:21]

alexander.shalamov@intel.com changed reviewers:
+ kenneth.christiansen@gmail.com, kenneth.r.christiansen@intel.com

[shalamov, 2016-04-24 19:58:22]

PTAL.

Following patches depend on default implementation:

https://crrev.com/1708543002/ - nfc.push / nfc.cancelPush impl.
https://crrev.com/1759373003/ - nfc.watch / nfc.cancelWathc impl.

Android implementation of nfc mojo service:

https://crrev.com/1486043002/ - nfc.push / nfc.cancelPush impl.
https://crrev.com/1765533004/ - nfc.watch / nfc.cancelWathc impl.

[kenneth.christiansen, 2016-04-25 08:16:55]

lgtm, but I am no expert on the mojo parts

[shalamov, 2016-04-25 14:28:36]

alexander.shalamov@intel.com changed reviewers:
+ kinuko@chromium.org, nick@chromium.org, reillyg@chromium.org,
rockot@chromium.org

[shalamov, 2016-04-25 14:28:37]

Added device/ owners to review. Could you please take a look?

[Reilly Grant (use Gerrit), 2016-04-25 21:48:35]

https://codereview.chromium.org/1912363002/diff/120001/content/browser/frame_...
File content/browser/frame_host/render_frame_host_impl.cc (right):

https://codereview.chromium.org/1912363002/diff/120001/content/browser/frame_...
content/browser/frame_host/render_frame_host_impl.cc:1938:
GetServiceRegistry()->AddService<device::NFC>(
The template parameter shouldn't be necessary here.

https://codereview.chromium.org/1912363002/diff/120001/device/nfc/BUILD.gn
File device/nfc/BUILD.gn (right):

https://codereview.chromium.org/1912363002/diff/120001/device/nfc/BUILD.gn#ne...
device/nfc/BUILD.gn:59: "//base/third_party/dynamic_annotations",
This dependency is unused.

https://codereview.chromium.org/1912363002/diff/120001/device/nfc/nfc.gyp
File device/nfc/nfc.gyp (right):

https://codereview.chromium.org/1912363002/diff/120001/device/nfc/nfc.gyp#new...
device/nfc/nfc.gyp:69:
'../../base/third_party/dynamic_annotations/dynamic_annotations.gyp:dynamic_annotations',
Also here.

https://codereview.chromium.org/1912363002/diff/120001/device/nfc/nfc_impl_de...
File device/nfc/nfc_impl_default.cc (right):

https://codereview.chromium.org/1912363002/diff/120001/device/nfc/nfc_impl_de...
device/nfc/nfc_impl_default.cc:64: };
Add DISALLOW_COPY_AND_ASSIGN(NFCDefaultImpl);

[shalamov, 2016-04-26 10:17:32]

https://codereview.chromium.org/1912363002/diff/120001/content/browser/frame_...
File content/browser/frame_host/render_frame_host_impl.cc (right):

https://codereview.chromium.org/1912363002/diff/120001/content/browser/frame_...
content/browser/frame_host/render_frame_host_impl.cc:1938:
GetServiceRegistry()->AddService<device::NFC>(
On 2016/04/25 21:48:35, Reilly Grant wrote:
> The template parameter shouldn't be necessary here.

Done.

https://codereview.chromium.org/1912363002/diff/120001/device/nfc/BUILD.gn
File device/nfc/BUILD.gn (right):

https://codereview.chromium.org/1912363002/diff/120001/device/nfc/BUILD.gn#ne...
device/nfc/BUILD.gn:59: "//base/third_party/dynamic_annotations",
On 2016/04/25 21:48:35, Reilly Grant wrote:
> This dependency is unused.

Done.

https://codereview.chromium.org/1912363002/diff/120001/device/nfc/nfc.gyp
File device/nfc/nfc.gyp (right):

https://codereview.chromium.org/1912363002/diff/120001/device/nfc/nfc.gyp#new...
device/nfc/nfc.gyp:69:
'../../base/third_party/dynamic_annotations/dynamic_annotations.gyp:dynamic_annotations',
On 2016/04/25 21:48:35, Reilly Grant wrote:
> Also here.

Done.

https://codereview.chromium.org/1912363002/diff/120001/device/nfc/nfc_impl_de...
File device/nfc/nfc_impl_default.cc (right):

https://codereview.chromium.org/1912363002/diff/120001/device/nfc/nfc_impl_de...
device/nfc/nfc_impl_default.cc:64: };
On 2016/04/25 21:48:35, Reilly Grant wrote:
> Add DISALLOW_COPY_AND_ASSIGN(NFCDefaultImpl);

Done.

[ncarter (slow), 2016-04-26 22:28:26]

https://codereview.chromium.org/1912363002/diff/140001/content/browser/frame_...
File content/browser/frame_host/render_frame_host_impl.cc (right):

https://codereview.chromium.org/1912363002/diff/140001/content/browser/frame_...
content/browser/frame_host/render_frame_host_impl.cc:1940:
GetServiceRegistry()->AddService(base::Bind(&device::NFCImpl::Create));
Do we plan to implement a c++, non-stub version of NFC on some platform?

If so, then this is fine.

If not, I'd prefer that it be clearer here that the purpose of this line is to
installs a stub implementation (either via comment, or by explicitly
instantiating a stub-looking object like NfcImplDefault()).

https://codereview.chromium.org/1912363002/diff/140001/device/nfc/nfc_impl.h
File device/nfc/nfc_impl.h (right):

https://codereview.chromium.org/1912363002/diff/140001/device/nfc/nfc_impl.h#...
device/nfc/nfc_impl.h:13: class NFCImpl {
Question for reilly: should we be calling this NfcImpl or NFCImpl?

There was a chromium-dev thread on this last fall, that suggested we should
prefer the lowercase-style acronyms, at least in new code.
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/k8-ztIrWaUY/hajvv...

[Reilly Grant (use Gerrit), 2016-04-26 22:35:33]

https://codereview.chromium.org/1912363002/diff/140001/content/browser/frame_...
File content/browser/frame_host/render_frame_host_impl.cc (right):

https://codereview.chromium.org/1912363002/diff/140001/content/browser/frame_...
content/browser/frame_host/render_frame_host_impl.cc:1940:
GetServiceRegistry()->AddService(base::Bind(&device::NFCImpl::Create));
On 2016/04/26 at 22:28:26, ncarter wrote:
> Do we plan to implement a c++, non-stub version of NFC on some platform?
> 
> If so, then this is fine.
> 
> If not, I'd prefer that it be clearer here that the purpose of this line is to
installs a stub implementation (either via comment, or by explicitly
instantiating a stub-looking object like NfcImplDefault()).

Actually, now that I think about it again I'd rather we not have a stub
implementation at all. If an implementation is not available the Mojo service
binding should simply fail (return from the service factory callback without
binding the interface request).

https://codereview.chromium.org/1912363002/diff/140001/device/nfc/nfc_impl.h
File device/nfc/nfc_impl.h (right):

https://codereview.chromium.org/1912363002/diff/140001/device/nfc/nfc_impl.h#...
device/nfc/nfc_impl.h:13: class NFCImpl {
On 2016/04/26 at 22:28:26, ncarter wrote:
> Question for reilly: should we be calling this NfcImpl or NFCImpl?
> 
> There was a chromium-dev thread on this last fall, that suggested we should
prefer the lowercase-style acronyms, at least in new code.
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/k8-ztIrWaUY/hajvv...

It should be NfcImpl.

[shalamov, 2016-04-27 10:01:59]

On 2016/04/26 22:35:33, Reilly Grant wrote:
>
https://codereview.chromium.org/1912363002/diff/140001/content/browser/frame_...
> File content/browser/frame_host/render_frame_host_impl.cc (right):
> 
>
https://codereview.chromium.org/1912363002/diff/140001/content/browser/frame_...
> content/browser/frame_host/render_frame_host_impl.cc:1940:
> GetServiceRegistry()->AddService(base::Bind(&device::NFCImpl::Create));
> On 2016/04/26 at 22:28:26, ncarter wrote:
> > Do we plan to implement a c++, non-stub version of NFC on some platform?
> > 
> > If so, then this is fine.
> > 
> > If not, I'd prefer that it be clearer here that the purpose of this line is
to
> installs a stub implementation (either via comment, or by explicitly
> instantiating a stub-looking object like NfcImplDefault()).
> 
> Actually, now that I think about it again I'd rather we not have a stub
> implementation at all. If an implementation is not available the Mojo service
> binding should simply fail (return from the service factory callback without
> binding the interface request).

I have prototype implementation for Chrome OS, which is also easy to adopt for
linux platform. I could introduce default implementation when I would start
upstreaming ChromeOS / Linux / Win implementations.

Reilly, I just tried removing dependency to this CL from
https://crrev.com/1708543002 and noticed that js bindings (used by LayoutTests)
are not generated anymore, since blink part only depends on _wtf variant. Do you
have any suggestions how I could force js bindings generation?

[Reilly Grant (use Gerrit), 2016-04-27 23:45:08]

On 2016/04/27 at 10:01:59, alexander.shalamov wrote:
> On 2016/04/26 22:35:33, Reilly Grant wrote:
> >
https://codereview.chromium.org/1912363002/diff/140001/content/browser/frame_...
> > File content/browser/frame_host/render_frame_host_impl.cc (right):
> > 
> >
https://codereview.chromium.org/1912363002/diff/140001/content/browser/frame_...
> > content/browser/frame_host/render_frame_host_impl.cc:1940:
> > GetServiceRegistry()->AddService(base::Bind(&device::NFCImpl::Create));
> > On 2016/04/26 at 22:28:26, ncarter wrote:
> > > Do we plan to implement a c++, non-stub version of NFC on some platform?
> > > 
> > > If so, then this is fine.
> > > 
> > > If not, I'd prefer that it be clearer here that the purpose of this line
is to
> > installs a stub implementation (either via comment, or by explicitly
> > instantiating a stub-looking object like NfcImplDefault()).
> > 
> > Actually, now that I think about it again I'd rather we not have a stub
> > implementation at all. If an implementation is not available the Mojo
service
> > binding should simply fail (return from the service factory callback without
> > binding the interface request).
> 
> I have prototype implementation for Chrome OS, which is also easy to adopt for
linux platform. I could introduce default implementation when I would start
upstreaming ChromeOS / Linux / Win implementations.
> 
> Reilly, I just tried removing dependency to this CL from
https://crrev.com/1708543002 and noticed that js bindings (used by LayoutTests)
are not generated anymore, since blink part only depends on _wtf variant. Do you
have any suggestions how I could force js bindings generation?

There's a better solution in the works but in the mean time you can add a
dependency on the non-_wtf variant to content/content_tests.gypi.

[shalamov, 2016-12-01 11:06:39]

Description was changed from

==========
[webnfc] Add default implementation for nfc mojo service.

This patch adds default nfc mojo service implementation.
It is used by platforms that do not support NFC functionality
or whenever nfc service implementation is not yet available.

BUG=520391
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
[webnfc] Add default implementation for nfc mojo service.

This patch adds default nfc mojo service implementation.
It is used by platforms that do not support NFC functionality
or whenever nfc service implementation is not yet available.

BUG=520391
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========