Content Modularization Project: Bluetooth |
|||||||||||||||||
Issue descriptionTracking 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.
,
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
,
May 17 2016
btw above is for the content/browser code. The code in content/renderer should move to blink per onion soup project.
,
May 17 2016
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.
,
May 17 2016
I'd vote for components/webbluetooth, as the service is higher level than the device/bluetooth interfaces.
,
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?
,
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.
,
Sep 14 2016
+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
,
Sep 23 2016
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?
,
Sep 23 2016
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.
,
Sep 23 2016
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.
,
Sep 23 2016
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.
,
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).
,
Sep 23 2016
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.
,
Sep 23 2016
+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.
,
Sep 26 2016
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++.
,
Sep 26 2016
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.
,
Sep 26 2016
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
,
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.
,
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.
,
Oct 6 2016
#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.
,
Nov 29 2016
,
Jan 26 2017
,
Mar 3 2017
,
Mar 3 2017
,
Mar 3 2017
,
Oct 4 2017
,
Nov 7 2017
,
Nov 7 2017
Migrating S13N meta bugs to Type=Task, so that they can be distinguished from technical work.
,
Nov 8 2017
,
Jan 9 2018
,
Mar 5 2018
,
Aug 21
|
|||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||
Comment 1 by jam@chromium.org
, May 16 2016