New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 612319 link

Starred by 6 users

Issue metadata

Status: Assigned
Owner:
Last visit > 30 days ago
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Task

Blocked on:
issue 508771

Blocking:
issue 598069
issue 612328



Sign in to add a comment

Content Modularization Project: Bluetooth

Project Member Reported by jam@chromium.org, May 16 2016

Issue description

Tracking bug for the bluetooth part of the Content Modularization Project. The end goal should be to have a self-contained bluetooth service. As part of getting rid (or greatly reducing) content/, this code should move out of content/ Most likely to device/bluetooth with the rest of the bluetooth code.
 

Comment 1 by jam@chromium.org, May 16 2016

Blocking: 612328

Comment 2 by jam@chromium.org, May 16 2016

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 to device/bluetooth

Comment 3 by jam@chromium.org, May 17 2016

btw above is for the content/browser code. The code in content/renderer should move to blink per onion soup project.

Comment 4 by jam@chromium.org, May 17 2016

Blocking: -598073 598069
We have a distinction between the platform abstraction layer in device/bluetooth, which several Bluetooth-using systems in Chrome use, and the Web Bluetooth adaptation layer in content/browser. I'd like to maintain that distinction, but we can move the adaptation layer to device/bluetooth/something or to components/something. 

Comment 6 by scheib@chromium.org, May 17 2016

Components: Blink>Bluetooth
Labels: -Type-Bug OS-All Type-Feature
I'd vote for components/webbluetooth, as the service is higher level than the device/bluetooth interfaces.

Comment 7 by jam@chromium.org, May 17 2016

Thanks for the explanation about the layering (I'm not familiar with this code).

I have one question: what purpose does the code in content/browser/bluetooth serve? Is it the glue between IPCs from the renderer to device/bluetooth? If so, if device/bluetooth has mojo interfaces that are accessible from the renderer, can the logic in content/browser/bluetooth just move to the renderer?

Comment 8 by scheib@chromium.org, May 17 2016

content/browser/bluetooth will move completely to WebBluetoothService.

Layer cake:

renderer: bindings to JS and basics of object managment. Does this by using WebBluetoothService.

browser: Implement WebBluetoothService with security/privacy features, e.g. blacklist services/characteristics, generate / associate unique IDs per origin, uses device/bluetooth, use chooser to select devices. 

device/bluetooth: Lower level and more functionality than is exposed in WebBluetoothService, including all classic Bluetooth and ChromeOS system level Bluetooth capabilities.
Cc: st...@chromium.org
+steel for Chrome OS bluetooth

For mustash it would be nice if the functionality we need for the ash system tray menu could exist in one of these layers

In my current understanding, there are actually 4 projects here:

1. Wrapping the code in //device/bluetooth in a Mojo interface.
2. Changing browser process clients of //device/bluetooth to consume it via Mojo, thus removing a blocker to moving the Device Service out of the browser process.
3. Decoupling //content/browser/bluetooth from //content into e.g. //components/webbluetooth.
4. Migrating //content/renderer/bluetooth into Blink.

The work that is blocking the Device Service (i.e., crbug.com/612328) is projects 1 and 2. Projects 3 and 4 are unrelated to the Device Service.
The relationship between the Device Service and //content/browser/bluetooth is just that //content/browser/bluetooth is one of the "browser process clients" mentioned in project 2, so it would have to be migrated to consuming //device/bluetooth via Mojo instead of C++ as part of project 2.

scheib@, does that match your understanding?
fwiw that is my understanding as well. Ideally the mojo service would be designed in a way that it could be directly consumed by blink and we wouldn't need 3. The security requirements of the Web Bluetooth API might make this hard though.
Re: comment #11, am I correct that the challenge (or at least one of the challenges) is //content/browser/bluetooth's maintenance of which devices a frame's origin is allowed to access? This seems to be both (a) specific to the Web and (b) requiring maintenance in a trusted process. (a) means it shouldn't be part of the Device Service, while (b) means that it can't be moved to the renderer.
re: comment #9, my assumption is that as part of project 2, the ash system tray menu would be ported to use the Mojo interface created in project 1.

Comment 14 by st...@chromium.org, Sep 23 2016

Your understand does seem correct. Just a small annotation.

"2. Changing browser process clients of //device/bluetooth to consume it via Mojo, thus removing a blocker to moving the Device Service out of the browser process."

Moving the Bluetooth service into its own process is not particularly a priority. Decoupling the clients of //device/bluetooth will be the most time consuming and painful part of this work. We will get a lot more wins, if we just leave the Bluetooth service in whichever process that is the most tightly coupled with //device/bluetooth; de-coupling only the clients that absolutely need to live in separate processes (ash system tray for example).

Re #12: Yes, exactly. In the end game, I think we'll have separate Mojo services for both //device/bluetooth and //content/browser/bluetooth running in one or two trusted processes, with Blink calling into //content/browser/bluetooth calling into //device/bluetooth. We can rename the directories however people want as long as it's compatible with that service structure.
+1 to nearly all of 10-15.

#12: Yes, we need something in browser process to make security decisions, and also to maintain per-origin ID mapping translations so that we don't leak private data to the web.

My intuition is that layering Web Bluetooth over device/bluetooth will be easiest. Many of the device/bluetooth interfaces can be 'passed through'/reused, as security is mainly at the device layer.

An alternative would be to have device/bluetooth optionaly delegate to another service for some security checks and ID mutation. Web Bluetooth would have a delegate implementation in the browser, create an instance of device/bluetooth with it, and provide it to the web.

I don't like how that forces complexity into device/bluetooth, instead of layering/wrapping. But if folk's have differing intuitions we should do a side by side comparison.
Thanks for the comments, everyone!

Re: comment#14, I am currently operating under the assumption that the decoupling the Device Service from the browser process is a key near-term priority for crbug.com/612328. I am going to resolve with ben@ and jam@ this week whether that assumption is correct.

Could you elaborate on what the win is from just decoupling //device/bluetooth from e.g. the ash system tray? As long as //device/bluetooth lives in the browser process, then the browser process will need to be running for anything to talk to //device/bluetooth, whether it does so via Mojo or via C++.
re: comment #16, I'll keep this information in mind for when we get to looking concretely at what the relationship between //content/browser/bluetooth and //device/bluetooth should be.
Components: IO>Bluetooth
Testing & a note in implementation steps:

I believe mbrunson should implement interfaces as closely matching the C++ interfaces as possible in this current first stage, and while used only by the internals page not add additional testing for the mojom interfaces.

I discussed with jam and mbrunson, and spent time thinking out various options for the complete refactor. Some scenarios include:

C++ refactor first:
. Refactor C++ interfaces first to new desired interface form. 
  [1] has had some design discussion on what we'd like mojom interface to be.
. Then start to migrate all users, including tests, to new mojom interfaces.

This option is too time consuming for the internals page, which needs
bindings to JS now.


Both simultaneous:
. Build out new mojom interfaces in the form we'd like in the long run.
. As we go, bring up a new test fixture that parallels BluetoothTest,
  and uses the new mojom interface. Duplicate tests as we go.
. Include additional tests there for the new mojom interface style.
. After moving C++ clients to mojom, remove BluetoothTest fixture & tests.

This option adds too much to the internals project, in both
opening design uncertainty for the mojo interfaces and in bringing
up all tests & test fixture implementation on all platforms.

Alternately, another team member could bring these tests up. I believe
it would be the incorrect priority for Web Bluetooth team now. 


Mojom thin wrapper first:
. Write mojoms to handle the language bindings for the bluetooth internals
  project, but keep them as thin a wrapper around the C++ interfaces
  as possible.
. Do not add new tests for them, as they are too minimal.
. Decide later if it's OK to move other systems other than bluetooth 
  internals to the mojo interfaces before they are refactored or not.
. Transition tests to mojom interfaces.
. Refactor mojom interfaces.

My current preferred solution. I acknowledge that the full 
servicification of bluetooth will require several passes 
(e.g. moving clients to mojom, and refactoring mojom & tests to 
desired long term interface style). Sometimes refactors
require multiple steps.


I'm open to thoughts.


[1] https://docs.google.com/document/d/1G5sKA2UDuiQl0w84GHRhJ2lUu0tymDxJBbdncgk-oXc

Comment 20 by st...@chromium.org, Sep 26 2016

#17: Decoupling the device service from ash is a near-term priority, not the browser process. As part of the AppShell design, I did some analysis on this. In order of the complexity of the coupling, this is where the major components start wrt to //device/bluetooth.

1.) Apps API - This is the most tightly coupled to //device/ bluetooth.
2.) Chrome - There are several places with Chrome which directly call into //device/bluetooth.
3.) Ash - This coupling is mostly for the system tray UI

There are other components in the picture like WebBluetooth and ARC++ but I am not listing them since they don't relate to this one particular point.

Putting a //device/bluetooth service in its own process will require extensive re-factoring of all the three components, which the only component that is "not" going to be running in the same process in the near-term is ash.

So if the system tray needs to continue talking to //device/bluetooth, we *need* an interface that it can use to go over processes.

Chrome and the apps API do not need this anytime in the 'near' future. However, that being said, of course, that will change too and at one point the Apps API code will no longer run in the same process as Chrome. When this happens, we will need to deal with de-coupling //device/bluetooth either from the Apps API, the Chrome code, or both.

I've talked about a lot of this in my AppShell design doc (go/chrome-appshell), just with all system services, not just Bluetooth.

Comment 21 by st...@chromium.org, Sep 26 2016

#19:  Your preferred solution does sound reasonable but my concern is that I am not sure if we'll be able to ensure that the mojo interface is actually a thin enough wrapper to not need its own testing.

If we can, then sure. We definitely will have many, many stages in this refactor, during each of which we'll learn a lot more. Adding tests once we're further along that learning is an additional incentive.

Cc: mbrunson@chromium.org
#19: So after speaking with mbrunson@ a bit about this and thinking a bit more - I think we will be in good shape, if we can just provide basic unit tests that verify that only the mojo interface does what is supposed to. i.e., unit tests for type converters and basic tests that verify expectations of any data manipulation by the interface.

The //device/bluetooth code is already very well tested. As long the mojo interface is a thin wrapper *and* we can verify that it does its job correctly, we can have a high level of confidence that things won't break.

Comment 23 by st...@chromium.org, Nov 29 2016

Owner: st...@chromium.org
Status: Assigned (was: Available)
Labels: DeviceService
Owner: r...@chromium.org
Cc: r...@chromium.org
Cc: -st...@chromium.org
Blocking: 769631
Components: Internals>Services>Device
Labels: Type-Task
Migrating S13N meta bugs to Type=Task, so that they can be distinguished from technical work.
Components: Internals>Services
Cc: -scheib@chromium.org
Blocking: -769631
Owner: ortuno@chromium.org

Sign in to add a comment