Tracking bug for the gamepad part of the Content Modularization Project. The end goal should be to have a self-contained service. As part of getting rid (or greatly reducing) content/, this code should move out of content/. Most likely to device/gamepad to be consistent with what's being done for other device related APIs.
Strawman proposal, following other services that we're extracting from content:
-convert IPCs to mojo
-eliminate the dependencies in this code on other code in content/
-move remaining code in content/browser/gamepad to device/gamepad
One possible wrinkle with this one is the unusual use of shared memory to have the browser only write once and have all the renderers read from the same one simultaneously. (I'm not saying it's a blocker/problem, just that it might require a bit of extra planning.)
For multicast, Mojo doesn't have such a concept. We could implement it manually, just like we could with chrome IPC. The drawback would be that we'd be waking up all the renderers each time the gamepad values change, which seems like something we'd want to avoid?
I presume it's not a layering violation to have code within devices/ depend on other code within devices/? I've got some speculative code that has gamepad interfaces implemented within what will soon be devices/vr/.
Also, when you say "For multicast, Mojo doesn't have such a concept" you mean Mojo doesn't support multicast? If so the manual implementation seems to be having each renderer create a GamepadServiceClient (presentation.mojom does this), have the GamepadService maintain a list of all active clients, and for each multicast message sending a message to each client separately, correct?
Labels: Merge-Request-53 Status: Started (was: Available)
I'll request the merge, though there's a couple of follow up patches that will need to go along with it to fix a couple of dependencies I missed on the first pass.
Before we approve merge to M53, Could you please confirm whether this change is baked/verified in Canary and safe to merge?
Also is this change applicable to all OS or any specific OS?
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!
If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.
Thanks for your time! To disable nags, add the Disable-Nags label.
For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
The main thing left to do is convert the IPC calls to Mojo and make sure that all the shared memory code can migrate cleanly. (I recall that was a sticking point for the move that's already been done, but can't recall the exact reason why off the top of my head.)
I have every intention of doing this, but I've also got other pressing deadlines to attend to so I may not be able to address it for a while.
Hi bajones@, I'm highly interested in having design docs for all the remaining Device Service work ASAP to estimate the amount of work involved. Would you be able to do the design doc for gamepad in the next week or so, or should I take an initial crack at it (i.e., just the design doc part, it's OK if the technical work lags behind)?
Hi blundell,
I'm an intern on Mojo. I discussed with rockot and we decided this would be a good starting servicification bug for me to help with. I'll be moving the GamepadMonitor interface into Device Service.
Hi,
Sounds good! Ke He has been doing work in this area. He has an outstanding CL (https://codereview.chromium.org/2580693003/). Ke He, can you remind me what is blocking that CL from landing? Did you have any other work in progress here? It should work fine to transition Gamepad work to Jimmy, as there's more than enough Device Service work to go around :).
I also recently did analysis on the project of servicifying Gamepad. I'll flesh out the bugtree with my findings by the end of this week, and we can touch base next week to go through the project in detail.
Hi, Colin, Hi, Jimmy, welcome:)
My pending CL is "Decouple and move Gamepad impl in //content/renderer/ into Blink".
I met two blocking issues in that CL.
1): "GamepadSharedMemoryReader uses Chromium synchronization primitives, which are not allowed in //third_party/WebKit/Source/modules.", See crbug 676328.
2): Layout-test is breaked. Because in my CL, the RendererBlinkPlatformImpl::platform_event_observers_ are not used by Gamepad anymore, but it is still useful for layout-tests.
I think my CL is about Renderer side, it is parallel with "moving the gamepadMonitor interface into Device Service", so Jimmy can move on.
Thanks, Ke He! Agreed that that work is for Blink Onion Soup and orthogonal to servicification.
Jimmy, I filled in the bugtree here based on the recent analysis that I did.
Update: We need to figure out the relationship between Gamepad and VR. VR is not going to go into the Device Service, but has a tight dependency on Gamepad. Does this mean that Gamepad should also not go into the Device Service? See https://bugs.chromium.org/p/chromium/issues/detail?id=689437#c7 for more context.
Re: c#43, note that it wouldn't be problematic technically to have Gamepad live outside of the Device Service, as Gamepad doesn't have dependencies on any other device features (besides //device/base/synchronization, which is fine).
FWIW we're refactoring the VR API and as part of it we plan or breaking the dependency with the gamepad API in favor of a more purpose-built and action-based API.
Hey Brandon,
This is really good information to have, thanks! Am I understanding correctly that by "breaking the dependency with the gamepad API" you mean that the VR implementation will no longer depend on //device/gamepad at all once your refactoring is complete? Do you have a design doc/tracking bug/etc? Thanks!
Hi Brandon, I'd like to have someone start on crbug.com/689437 (moving VR entirely out of //device). However, before doing so I wanted to confirm: Is the definitive plan that VR will no longer depend on //device/gamepad in the future? If so, do you have an idea of a rough timeframe for dropping the dependence? Thanks!
oksamyt@: I'm glad that you're interested! As you can see from the above discussion, s13n of Gamepad and VR is in kind of a messy and ambiguous state :P. Do you have concrete technical plans here? Thanks!
Hello Colin,
I would like to provide some context:
Initially I was going to attempt moving files from content/renderer/device_sensors/ to third_party/blink/renderer/modules/device_orientation/, which looked like a fairly focused task (that would allow me to get familiar with mojo and layers). Then I discovered dependencies between GamepadSharedMemoryReader, PlatformEventObserver and DeviceSensorEventPump, so looks like GamepadSharedMemoryReader needs to be moved to blink first. The patch that does that move (https://crrev.com/2580693003) was blocked on layout tests. Reilly and Matt have been helping me to understand how to resolve the issue with tests, but I am still in the process of learning what the different parts are and how they are related. I will share some notes describing the approaches and options soon. No plans to change anything VR-related.
Thanks!
Thanks, that makes sense! So at this time, you're looking at completing the renderer-side work for Gamepad (i.e., the Blink Onion Soup work that Ke He had started and described in c#39 above). As you've noted, that's orthogonal to the relationship between Gamepad and VR.
Comment 1 by jam@chromium.org
, May 16 2016