WIP WebVR mojo refactor

WIP WebVR mojo refactor

Based on the CL by shaobo.yan@intel.com https://codereview.chromium.org/2420743003/

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

[bajones, 2016-10-26 23:10:48]

Description was changed from

==========
WIP WebVR mojo refactor

BUG=652080
==========

to

==========
WIP WebVR mojo refactor

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

[bajones, 2016-10-27 06:10:42]

Description was changed from

==========
WIP WebVR mojo refactor

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

to

==========
WIP WebVR mojo refactor

Based on the CL by shaobo.yan@intel.com
https://codereview.chromium.org/2420743003/

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

[bajones, 2016-10-27 06:57:09]

For the record, the Mojo connection flow here is:

* Blink requests a VRService
* VRServiceImpl created and sent back in response
* First VRServiceImpl creates VRDeviceManager singleton
* When Blink receives the VRService instance, sets the VRController as it's
VRServiceClient
* Setting the client implicitly queries the list of VRDevices
* A VRDisplayImpl is created for each VRDevice, and sent through VRServiceImpl's
OnDisplayConnected along with the initial VRDisplayInfo
* When any OnDisplayConnected is received a VRDisplay is created on the Blink
end and given the mojom::VRDisplayPtr
* The blink VRDisplay implements mojom::VRDisplayClient, and is send to the
provided VRDisplayPtr through SetClient which establishes the two-way
communication.
* Once all OnDisplayConnected calls have been sent the VRService.SetClient
call's callback is fired to indicate that all active displays have been
received.
* This is used to resolved the getDisplays promise.

[shaobo.yan, 2016-10-27 07:58:01]

On 2016/10/27 06:57:09, bajones wrote:
> For the record, the Mojo connection flow here is:
> 
> * Blink requests a VRService
> * VRServiceImpl created and sent back in response
> * First VRServiceImpl creates VRDeviceManager singleton
> * When Blink receives the VRService instance, sets the VRController as it's
> VRServiceClient
> * Setting the client implicitly queries the list of VRDevices
> * A VRDisplayImpl is created for each VRDevice, and sent through
VRServiceImpl's
> OnDisplayConnected along with the initial VRDisplayInfo
> * When any OnDisplayConnected is received a VRDisplay is created on the Blink
> end and given the mojom::VRDisplayPtr
> * The blink VRDisplay implements mojom::VRDisplayClient, and is send to the
> provided VRDisplayPtr through SetClient which establishes the two-way
> communication.
> * Once all OnDisplayConnected calls have been sent the VRService.SetClient
> call's callback is fired to indicate that all active displays have been
> received.
> * This is used to resolved the getDisplays promise.

Sorry for blocking again and I think I understand the whole process now, even I
solved sync problem, it is still better than my patch for it will reduce lots of
map used which I don't like....
And I also notice it seems that it remains some pieces which will block the
building and running process. 
If you don't mind, I'd like to grab the changes and help with solving these
glitches.

BR,
Yan,Shaobo

[bajones, 2016-10-27 19:58:16]

On 2016/10/27 07:58:01, shaobo.yan wrote:
> On 2016/10/27 06:57:09, bajones wrote:
> > For the record, the Mojo connection flow here is:
> > 
> > * Blink requests a VRService
> > * VRServiceImpl created and sent back in response
> > * First VRServiceImpl creates VRDeviceManager singleton
> > * When Blink receives the VRService instance, sets the VRController as it's
> > VRServiceClient
> > * Setting the client implicitly queries the list of VRDevices
> > * A VRDisplayImpl is created for each VRDevice, and sent through
> VRServiceImpl's
> > OnDisplayConnected along with the initial VRDisplayInfo
> > * When any OnDisplayConnected is received a VRDisplay is created on the
Blink
> > end and given the mojom::VRDisplayPtr
> > * The blink VRDisplay implements mojom::VRDisplayClient, and is send to the
> > provided VRDisplayPtr through SetClient which establishes the two-way
> > communication.
> > * Once all OnDisplayConnected calls have been sent the VRService.SetClient
> > call's callback is fired to indicate that all active displays have been
> > received.
> > * This is used to resolved the getDisplays promise.
> 
> Sorry for blocking again and I think I understand the whole process now, even
I
> solved sync problem, it is still better than my patch for it will reduce lots
of
> map used which I don't like....
> And I also notice it seems that it remains some pieces which will block the
> building and running process. 
> If you don't mind, I'd like to grab the changes and help with solving these
> glitches.
> 
> BR,
> Yan,Shaobo

Of course! my intent isn't to take over the patch, just to provide feedback.
Sometimes I find the easiest way to do that is with code.

I've updated the CL to fix some of the compile bugs, though it's still not fully
tested.

[bajones, 2016-10-28 00:22:55]

FYI Klaus

[shaobo.yan, 2016-10-28 03:04:15]

On 2016/10/27 19:58:16, bajones wrote:
> On 2016/10/27 07:58:01, shaobo.yan wrote:
> > On 2016/10/27 06:57:09, bajones wrote:
> > > For the record, the Mojo connection flow here is:
> > > 
> > > * Blink requests a VRService
> > > * VRServiceImpl created and sent back in response
> > > * First VRServiceImpl creates VRDeviceManager singleton
> > > * When Blink receives the VRService instance, sets the VRController as
it's
> > > VRServiceClient
> > > * Setting the client implicitly queries the list of VRDevices
> > > * A VRDisplayImpl is created for each VRDevice, and sent through
> > VRServiceImpl's
> > > OnDisplayConnected along with the initial VRDisplayInfo
> > > * When any OnDisplayConnected is received a VRDisplay is created on the
> Blink
> > > end and given the mojom::VRDisplayPtr
> > > * The blink VRDisplay implements mojom::VRDisplayClient, and is send to
the
> > > provided VRDisplayPtr through SetClient which establishes the two-way
> > > communication.
> > > * Once all OnDisplayConnected calls have been sent the VRService.SetClient
> > > call's callback is fired to indicate that all active displays have been
> > > received.
> > > * This is used to resolved the getDisplays promise.
> > 
> > Sorry for blocking again and I think I understand the whole process now,
even
> I
> > solved sync problem, it is still better than my patch for it will reduce
lots
> of
> > map used which I don't like....
> > And I also notice it seems that it remains some pieces which will block the
> > building and running process. 
> > If you don't mind, I'd like to grab the changes and help with solving these
> > glitches.
> > 
> > BR,
> > Yan,Shaobo
> 
> Of course! my intent isn't to take over the patch, just to provide feedback.
> Sometimes I find the easiest way to do that is with code.
> 
> I've updated the CL to fix some of the compile bugs, though it's still not
fully
> tested.

Sorry for late due to time zone reason... 
I've finished rebase work and built out a browser.
And it indeed has problem on pop up correct device info when call getVRDisplay
the first time.
I'll investigate this and test other function and fix it based on the old issue
if you don't mind :)

BR,
Yan,Shaobo

[bajones, 2016-10-28 05:46:00]

On 2016/10/28 03:04:15, shaobo.yan wrote:
> On 2016/10/27 19:58:16, bajones wrote:
> > On 2016/10/27 07:58:01, shaobo.yan wrote:
> > > On 2016/10/27 06:57:09, bajones wrote:
> > > > For the record, the Mojo connection flow here is:
> > > > 
> > > > * Blink requests a VRService
> > > > * VRServiceImpl created and sent back in response
> > > > * First VRServiceImpl creates VRDeviceManager singleton
> > > > * When Blink receives the VRService instance, sets the VRController as
> it's
> > > > VRServiceClient
> > > > * Setting the client implicitly queries the list of VRDevices
> > > > * A VRDisplayImpl is created for each VRDevice, and sent through
> > > VRServiceImpl's
> > > > OnDisplayConnected along with the initial VRDisplayInfo
> > > > * When any OnDisplayConnected is received a VRDisplay is created on the
> > Blink
> > > > end and given the mojom::VRDisplayPtr
> > > > * The blink VRDisplay implements mojom::VRDisplayClient, and is send to
> the
> > > > provided VRDisplayPtr through SetClient which establishes the two-way
> > > > communication.
> > > > * Once all OnDisplayConnected calls have been sent the
VRService.SetClient
> > > > call's callback is fired to indicate that all active displays have been
> > > > received.
> > > > * This is used to resolved the getDisplays promise.
> > > 
> > > Sorry for blocking again and I think I understand the whole process now,
> even
> > I
> > > solved sync problem, it is still better than my patch for it will reduce
> lots
> > of
> > > map used which I don't like....
> > > And I also notice it seems that it remains some pieces which will block
the
> > > building and running process. 
> > > If you don't mind, I'd like to grab the changes and help with solving
these
> > > glitches.
> > > 
> > > BR,
> > > Yan,Shaobo
> > 
> > Of course! my intent isn't to take over the patch, just to provide feedback.
> > Sometimes I find the easiest way to do that is with code.
> > 
> > I've updated the CL to fix some of the compile bugs, though it's still not
> fully
> > tested.
> 
> Sorry for late due to time zone reason... 
> I've finished rebase work and built out a browser.
> And it indeed has problem on pop up correct device info when call getVRDisplay
> the first time.
> I'll investigate this and test other function and fix it based on the old
issue
> if you don't mind :)
> 
> BR,
> Yan,Shaobo

Sounds good to me! Thanks, and hope it's not to tough to track down the problem.

[shaobo.yan, 2016-10-28 08:20:50]

shaobo.yan@intel.com changed reviewers:
+ shaobo.yan@intel.com

[shaobo.yan, 2016-10-28 08:20:51]

Hi, brandon
I've fixed getVRDisplay return null issue and updated a patch set here
https://codereview.chromium.org/2420743003/;
The reason is that onDisplaySynced callback may invoked before
onDisplayConnected callback and will update m_displaySynced to true.
So getVRDisplay will return m_displays, before any onDisplayConnected, which is
null.
My solution is to add a new attribute to get how many devices are there in
browser side. it is not so elegant and if you have better idea, pls update it.

I also try to run WebVR Samples here(https://toji.github.io/webvr-samples/). And
it seems most of the sample could not run successfully. Further reason are
investigated.

BR,
Yan,Shaobo

[shaobo.yan, 2016-10-31 02:31:49]

Hi, Brandon,
Happly Hallowee first !
After some fix, the mojo simpify patch seems to work now. I try to run WebVR
Sample VR Presenting, and it gives me correct rendering now. I think it means
the patch could function now.

However, I find that the sample will in low fps when entering vr with my debug
build, even if without the refactory patch. Is it a known issue or something
else ? 

Thx for your help with the refactory again !

BR,
Yan,Shaobo