Ports VRService to be hosted in the device service.

Ports VRService to be hosted in the device service.

Ports the VRService to be hosted by te Device Service instead of by
//content/browser.

BUG=612342
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

[ke.he, 2016-12-23 08:38:33]

Description was changed from

==========
Ports VRService to be hosted in the device service.

BUG=
==========

to

==========
Ports VRService to be hosted in the device service.

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

[ke.he, 2016-12-23 08:38:59]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-12-23 08:39:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-23 08:42:35]

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

[commit-bot: I haz the power, 2016-12-23 08:42:35]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[ke.he, 2016-12-23 09:01:31]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-12-23 09:01:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-23 09:04:52]

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

[commit-bot: I haz the power, 2016-12-23 09:04:53]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[ke.he, 2016-12-23 09:18:44]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-12-23 09:18:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-23 09:27:22]

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

[commit-bot: I haz the power, 2016-12-23 09:27:23]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[ke.he, 2016-12-23 09:36:36]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-12-23 09:36:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-23 10:00:20]

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

[commit-bot: I haz the power, 2016-12-23 10:00:21]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[ke.he, 2016-12-24 02:52:36]

Patchset #2 (id:20001) has been deleted

[ke.he, 2016-12-24 02:52:44]

Patchset #3 (id:60001) has been deleted

[ke.he, 2016-12-24 02:52:52]

Patchset #2 (id:40001) has been deleted

[ke.he, 2016-12-24 02:53:52]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-12-24 02:54:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-24 03:10:07]

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

[commit-bot: I haz the power, 2016-12-24 03:10:07]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[ke.he, 2016-12-24 04:21:39]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-12-24 04:21:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-24 06:03:12]

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

[commit-bot: I haz the power, 2016-12-24 06:03:13]

Dry run: This issue passed the CQ dry run.

[ke.he, 2016-12-26 03:29:28]

Description was changed from

==========
Ports VRService to be hosted in the device service.

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

to

==========
Ports VRService to be hosted in the device service.

Ports the VRService to be hosted by te Device Service instead of by
//content/browser.

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

[ke.he, 2016-12-26 03:39:51]

ke.he@intel.com changed reviewers:
+ blundell@chromium.org

[ke.he, 2016-12-26 03:39:52]

Hi,Colin, This patch is very similar as the porting of time_zone_monitor.  PTAL.
Thanks.

By the way, what's the BUG ID?

[blundell, 2016-12-29 16:44:04]

This looks great, but it also re-raises the question in my mind of whether there
should be one Device Service or many; when I see it laid out in code, it seems
odd to have VR buried underneath the "Device Service." I'm going to re-raise
this issue on chromium-mojo@. Let's hold off on this CL until resolving that
question.

[bajones, 2016-12-31 05:27:06]

On 2016/12/29 16:44:04, blundell wrote:
> This looks great, but it also re-raises the question in my mind of whether
there
> should be one Device Service or many; when I see it laid out in code, it seems
> odd to have VR buried underneath the "Device Service." I'm going to re-raise
> this issue on chromium-mojo@. Let's hold off on this CL until resolving that
> question.

Could you clarify something for me? Where does the Device Service run? Is it
still in the browser process, or is it it's own process (or could it become it's
own process some day?) Currently the VR service must run in the browser process
on Android. I think we'll have more flexibility with desktop implementations.

[blundell, 2017-01-03 15:34:40]

On 2016/12/31 05:27:06, bajones (OOO till January) wrote:
> On 2016/12/29 16:44:04, blundell wrote:
> > This looks great, but it also re-raises the question in my mind of whether
> there
> > should be one Device Service or many; when I see it laid out in code, it
seems
> > odd to have VR buried underneath the "Device Service." I'm going to re-raise
> > this issue on chromium-mojo@. Let's hold off on this CL until resolving that
> > question.
> 
> Could you clarify something for me? Where does the Device Service run? Is it
> still in the browser process, or is it it's own process (or could it become
it's
> own process some day?) Currently the VR service must run in the browser
process
> on Android. I think we'll have more flexibility with desktop implementations.

Hi Brandon,

This is one of the things we're feeling out. The vision is that the Device
Service would be able to run in its own process, but we're hitting limitations
on other device features as well on Android. What causes the coupling of the
VRService to the browser process on Android?

[bajones, 2017-01-04 19:16:07]

On 2017/01/03 15:34:40, blundell wrote:
> Hi Brandon,
> 
> This is one of the things we're feeling out. The vision is that the Device
> Service would be able to run in its own process, but we're hitting limitations
> on other device features as well on Android. What causes the coupling of the
> VRService to the browser process on Android?

I imagine it's similar to the other limitations you're encountering. The
Daydream API requires that we create and manage the main API object from a Java
object (GvrLayout:
https://developers.google.com/vr/android/reference/com/google/vr/ndk/base/Gvr...)
that must exist in the view hierarchy. This keeps us tied to the browser
process, since that's the only process that has an android view hierarchy (and
seems to be the only process that uses Java at all.) We've asked the Daydream
team about how hard of a dependency this is, and it seems like it's here to stay
for a variety of reasons.

[mthiesse, 2017-01-04 19:40:20]

On 2017/01/04 19:16:07, bajones (OOO till January) wrote:
> On 2017/01/03 15:34:40, blundell wrote:
> > Hi Brandon,
> > 
> > This is one of the things we're feeling out. The vision is that the Device
> > Service would be able to run in its own process, but we're hitting
limitations
> > on other device features as well on Android. What causes the coupling of the
> > VRService to the browser process on Android?
> 
> I imagine it's similar to the other limitations you're encountering. The
> Daydream API requires that we create and manage the main API object from a
Java
> object (GvrLayout:
>
https://developers.google.com/vr/android/reference/com/google/vr/ndk/base/Gvr...)
> that must exist in the view hierarchy. This keeps us tied to the browser
> process, since that's the only process that has an android view hierarchy (and
> seems to be the only process that uses Java at all.) We've asked the Daydream
> team about how hard of a dependency this is, and it seems like it's here to
stay
> for a variety of reasons.

I'm not sure what you're referring to Brandon, but only some parts of the GVR
API can only be used from the browser process, like initial API context
creation. After that the GVR API can be initialized and used from any single
thread, but it's not threadsafe.

crbug.com/674594 tracks the issue of using the GVR API from multiple threads
simultaneously. VR Shell uses the GVR API from its GL Thread, while using the
GvrLayout only from the UI (browser) thread, as is the intended usage pattern.
However, the gamepad API and VRService use the API from the browser process
while VR Shell uses it from its GL Thread.

Ideally the VRService should either only asynchronously use GVR APIs, or should
be hosted in VR Shell's GL Thread. This is complicated by the fact that the GL
Thread doesn't exist currently while in magic window mode, so we would have to
move (rebind?) the service between the browser process and GL Thread when
entering/exiting presentation.

[blundell, 2017-01-05 16:36:10]

On 2017/01/04 19:40:20, mthiesse wrote:
> On 2017/01/04 19:16:07, bajones (OOO till January) wrote:
> > On 2017/01/03 15:34:40, blundell wrote:
> > > Hi Brandon,
> > > 
> > > This is one of the things we're feeling out. The vision is that the Device
> > > Service would be able to run in its own process, but we're hitting
> limitations
> > > on other device features as well on Android. What causes the coupling of
the
> > > VRService to the browser process on Android?
> > 
> > I imagine it's similar to the other limitations you're encountering. The
> > Daydream API requires that we create and manage the main API object from a
> Java
> > object (GvrLayout:
> >
>
https://developers.google.com/vr/android/reference/com/google/vr/ndk/base/Gvr...)
> > that must exist in the view hierarchy. This keeps us tied to the browser
> > process, since that's the only process that has an android view hierarchy
(and
> > seems to be the only process that uses Java at all.) We've asked the
Daydream
> > team about how hard of a dependency this is, and it seems like it's here to
> stay
> > for a variety of reasons.
> 
> I'm not sure what you're referring to Brandon, but only some parts of the GVR
> API can only be used from the browser process, like initial API context
> creation. After that the GVR API can be initialized and used from any single
> thread, but it's not threadsafe.
> 
> crbug.com/674594 tracks the issue of using the GVR API from multiple threads
> simultaneously. VR Shell uses the GVR API from its GL Thread, while using the
> GvrLayout only from the UI (browser) thread, as is the intended usage pattern.
> However, the gamepad API and VRService use the API from the browser process
> while VR Shell uses it from its GL Thread.
> 
> Ideally the VRService should either only asynchronously use GVR APIs, or
should
> be hosted in VR Shell's GL Thread. This is complicated by the fact that the GL
> Thread doesn't exist currently while in magic window mode, so we would have to
> move (rebind?) the service between the browser process and GL Thread when
> entering/exiting presentation.

I'm having trouble finding the Java side of the VR impl in Chromium. Could
someone give me a pointer? Thanks!

[mthiesse, 2017-01-05 16:44:45]

On 2017/01/05 16:36:10, blundell wrote:
> On 2017/01/04 19:40:20, mthiesse wrote:
> > On 2017/01/04 19:16:07, bajones (OOO till January) wrote:
> > > On 2017/01/03 15:34:40, blundell wrote:
> > > > Hi Brandon,
> > > > 
> > > > This is one of the things we're feeling out. The vision is that the
Device
> > > > Service would be able to run in its own process, but we're hitting
> > limitations
> > > > on other device features as well on Android. What causes the coupling of
> the
> > > > VRService to the browser process on Android?
> > > 
> > > I imagine it's similar to the other limitations you're encountering. The
> > > Daydream API requires that we create and manage the main API object from a
> > Java
> > > object (GvrLayout:
> > >
> >
>
https://developers.google.com/vr/android/reference/com/google/vr/ndk/base/Gvr...)
> > > that must exist in the view hierarchy. This keeps us tied to the browser
> > > process, since that's the only process that has an android view hierarchy
> (and
> > > seems to be the only process that uses Java at all.) We've asked the
> Daydream
> > > team about how hard of a dependency this is, and it seems like it's here
to
> > stay
> > > for a variety of reasons.
> > 
> > I'm not sure what you're referring to Brandon, but only some parts of the
GVR
> > API can only be used from the browser process, like initial API context
> > creation. After that the GVR API can be initialized and used from any single
> > thread, but it's not threadsafe.
> > 
> > crbug.com/674594 tracks the issue of using the GVR API from multiple threads
> > simultaneously. VR Shell uses the GVR API from its GL Thread, while using
the
> > GvrLayout only from the UI (browser) thread, as is the intended usage
pattern.
> > However, the gamepad API and VRService use the API from the browser process
> > while VR Shell uses it from its GL Thread.
> > 
> > Ideally the VRService should either only asynchronously use GVR APIs, or
> should
> > be hosted in VR Shell's GL Thread. This is complicated by the fact that the
GL
> > Thread doesn't exist currently while in magic window mode, so we would have
to
> > move (rebind?) the service between the browser process and GL Thread when
> > entering/exiting presentation.
> 
> I'm having trouble finding the Java side of the VR impl in Chromium. Could
> someone give me a pointer? Thanks!

It's at
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java

[blundell, 2017-01-12 09:59:55]

blundell@chromium.org changed reviewers:
+ bajones@chromium.org, mthiesse@chromium.org

[blundell, 2017-01-12 09:59:56]

Apologies, it took a little while for me to analyze this code and process the
implications here.

(1) It's a non-goal to have the Device Service OOP on Android at the current
time, so that's not a blocker.

(2) //device/vr has code coupling to //chrome. In the long term, we should
analyze that as well, but that shouldn't block us from further modularization of
//content at this time.

Ke He, I suggest that you add a README to //services/device noting that the
Device Service is currently coupled to the browser process on certain platforms
and giving a little context about VR in particular (//device/vr is used directly
by //chrome/browser/android/vr_shell).

I'll also file an umbrella bug for analyzing interdependencies between the
Device Service and higher layers of Chromium (i.e., above //content), and then
will file a bug under that for analyzing the VR coupling.

I also have a technical question for Brandon below.

https://codereview.chromium.org/2603553002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/vr/VRController.cpp (left):

https://codereview.chromium.org/2603553002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/vr/VRController.cpp:24:
navigatorVR->document()->frame()->interfaceProvider()->getInterface(
Brandon, do you know this interface was exposed at the per-frame level rather
than the per-process level? I don't see that VRServiceImpl has any per-frame
context.

[ke.he, 2017-01-13 09:03:07]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-13 09:03:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-13 10:51:57]

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

[commit-bot: I haz the power, 2017-01-13 10:51:57]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[ke.he, 2017-01-14 02:11:52]

Patchset #4 (id:120001) has been deleted

[ke.he, 2017-01-17 13:23:14]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-17 13:23:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-17 13:27:48]

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

[commit-bot: I haz the power, 2017-01-17 13:27:49]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[bajones, 2017-01-17 21:33:27]

On 2017/01/12 09:59:56, blundell wrote:
>
https://codereview.chromium.org/2603553002/diff/100001/third_party/WebKit/Sou...
> third_party/WebKit/Source/modules/vr/VRController.cpp:24:
> navigatorVR->document()->frame()->interfaceProvider()->getInterface(
> Brandon, do you know this interface was exposed at the per-frame level rather
> than the per-process level? I don't see that VRServiceImpl has any per-frame
> context.

We should be differentiating these connections at a per-frame level because only
one frame at a time is allowed to present to a VRDisplay. 
There are multiple checks for this in
https://cs.chromium.org/chromium/src/device/vr/vr_display_impl.cc (look for
"IsAccessAllowed"). If we were handling this connection on a per-process level
it may be possible for one frame to preempt another's VR presentation
unexpectedly.

[ke.he, 2017-01-18 06:38:21]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-18 06:38:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-18 08:53:57]

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

[commit-bot: I haz the power, 2017-01-18 08:53:58]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[ke.he, 2017-01-18 09:53:45]

Hi, Colin and Brandon, 
My understanding is Per-frame level or Per-process level has no difference
because it uses the mojo connection. Each frame will create its own
connection(by blink::ServiceConnector), we didn't change the "IsAccessAllowed"
logic so there is no preempt, right?

The real blocker is we broke the layout test after we replaced the original
"frame-level mojo connection" by "device-service mojo connection".

I found the VR layout test create a "mock-vr-service.js" which implements the VR
mojo interface in javascript. The Javascript-mojo is enabled in
"interface_registry_js_wrapper.cc", "interface_provider_js_wrapper.cc" and
"mojo-helpers.js". The logic is to pass the interface-registry and
interface-provider of frame-level or process-level into *js_wrapper, so
Javascript code can also use mojo.

But now the device-service hides the "interface_registry" and
"interface_provider". I cannot find a way to enable the mojo of javascript.
Sorry I'm not quite sure my understanding is correct:(

One choice is to remove all the VR layouttest html files temporarily, and add
them back after javascript mojo is ready to device service.
Another choice is to pending this CL till javascript mojo is ready.

Could you please give some comments, Thanks.

[blundell, 2017-01-18 18:43:35]

On 2017/01/18 09:53:45, ke.he wrote:
> Hi, Colin and Brandon, 
> My understanding is Per-frame level or Per-process level has no difference
> because it uses the mojo connection. Each frame will create its own
> connection(by blink::ServiceConnector), we didn't change the "IsAccessAllowed"
> logic so there is no preempt, right?

Agreed. Thanks for the info, Brandon! This should still work fine.

> 
> The real blocker is we broke the layout test after we replaced the original
> "frame-level mojo connection" by "device-service mojo connection".
> 
> I found the VR layout test create a "mock-vr-service.js" which implements the
VR
> mojo interface in javascript. The Javascript-mojo is enabled in
> "interface_registry_js_wrapper.cc", "interface_provider_js_wrapper.cc" and
> "mojo-helpers.js". The logic is to pass the interface-registry and
> interface-provider of frame-level or process-level into *js_wrapper, so
> Javascript code can also use mojo.
> 
> But now the device-service hides the "interface_registry" and
> "interface_provider". I cannot find a way to enable the mojo of javascript.
> Sorry I'm not quite sure my understanding is correct:(
> 
> One choice is to remove all the VR layouttest html files temporarily, and add
> them back after javascript mojo is ready to device service.
> Another choice is to pending this CL till javascript mojo is ready.
> 
> Could you please give some comments, Thanks.

Funny, I was examining this problem yesterday because I hit it with
BatteryMonitor. Your analysis of the problem is correct. I just sent an email to
{chromium-mojo, platform-architecture-dev}@ presenting two approaches for
solving it. Once I determine which approach people prefer, I anticipate landing
the solution in a short period. So I think we could just wait to land this CL
until I solve that issue.

[ke.he, 2017-04-17 02:55:51]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-17 02:56:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-17 03:04:12]

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

[commit-bot: I haz the power, 2017-04-17 03:04:13]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[ke.he, 2017-04-17 03:45:08]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-17 03:45:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-17 03:53:32]

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

[commit-bot: I haz the power, 2017-04-17 03:53:33]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[ke.he, 2017-04-17 04:00:32]

Patchset #10 (id:260001) has been deleted

[ke.he, 2017-04-17 04:00:47]

Patchset #10 (id:280001) has been deleted

[ke.he, 2017-04-17 04:00:59]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-17 04:01:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-17 05:37:48]

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

[commit-bot: I haz the power, 2017-04-17 05:37:49]

Dry run: This issue passed the CQ dry run.

[ke.he, 2017-04-17 05:38:44]

Hi, Colin, PTAL, thanks!

https://codereview.chromium.org/2603553002/diff/220001/services/device/manife...
File services/device/manifest.json (right):

https://codereview.chromium.org/2603553002/diff/220001/services/device/manife...
services/device/manifest.json:11: "device::mojom::VRDisplay"
only VRService is enough.

https://codereview.chromium.org/2603553002/diff/220001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/vr/VRController.cpp (left):

https://codereview.chromium.org/2603553002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/vr/VRController.cpp:26:
m_service.set_connection_error_handler(convertToBaseCallback(
del this by mistake.

[ke.he, 2017-04-17 05:48:42]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-17 05:48:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ke.he, 2017-04-17 05:53:49]

Description was changed from

==========
Ports VRService to be hosted in the device service.

Ports the VRService to be hosted by te Device Service instead of by
//content/browser.

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

to

==========
Ports VRService to be hosted in the device service.

Ports the VRService to be hosted by te Device Service instead of by
//content/browser.

BUG=612342
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[commit-bot: I haz the power, 2017-04-17 07:28:13]

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

[commit-bot: I haz the power, 2017-04-17 07:28:14]

Dry run: This issue passed the CQ dry run.

[ke.he, 2017-04-17 09:21:49]

On 2017/04/17 05:38:44, ke.he wrote:
> Hi, Colin, PTAL, thanks!
> 
>
https://codereview.chromium.org/2603553002/diff/220001/services/device/manife...
> File services/device/manifest.json (right):
> 
>
https://codereview.chromium.org/2603553002/diff/220001/services/device/manife...
> services/device/manifest.json:11: "device::mojom::VRDisplay"
> only VRService is enough.
> 
>
https://codereview.chromium.org/2603553002/diff/220001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/modules/vr/VRController.cpp (left):
> 
>
https://codereview.chromium.org/2603553002/diff/220001/third_party/WebKit/Sou...
> third_party/WebKit/Source/modules/vr/VRController.cpp:26:
> m_service.set_connection_error_handler(convertToBaseCallback(
> del this by mistake.

Oh, Sorry, please ignore the draft comments.  Apologize for my careless :(

[mthiesse, 2017-04-17 14:41:44]

What process/thread does the device service run in? The VRService still has to
run in the browser UI process currently.

[mthiesse, 2017-04-17 14:41:45]

What process/thread does the device service run in? The VRService still has to
run in the browser UI process currently.

[ke.he, 2017-04-18 02:40:34]

On 2017/04/17 14:41:45, mthiesse wrote:
> What process/thread does the device service run in? The VRService still has to
> run in the browser UI process currently.

The Device Service run in Browser process, VRService still runs in UI thread
after this porting.

[blundell, 2017-04-18 13:53:41]

Hi,

Ke He, thanks for following up here! Michael, I, and others are in the middle of
a discussion about servicification of VR, and it looks like there are more
wide-ranging design issues here than I had been aware of. Let's hold off on this
for now pending some followup design discussions. I apologize for the churn
here.

[ke.he, 2017-04-19 05:23:50]

On 2017/04/18 13:53:41, blundell wrote:
> Hi,
> 
> Ke He, thanks for following up here! Michael, I, and others are in the middle
of
> a discussion about servicification of VR, and it looks like there are more
> wide-ranging design issues here than I had been aware of. Let's hold off on
this
> for now pending some followup design discussions. I apologize for the churn
> here.

There is nothing to apologize for, totally understand:)

[ke.he, 2017-05-19 05:07:09]

Description was changed from

==========
Ports VRService to be hosted in the device service.

Ports the VRService to be hosted by te Device Service instead of by
//content/browser.

BUG=612342
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Ports VRService to be hosted in the device service.

Ports the VRService to be hosted by te Device Service instead of by
//content/browser.

BUG=612342
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========