Create ChildProcessId mojom [chromium/src : main]
Christopher Staite (Gerrit)
Message from Christopher Staite![]( "Open in Gerrit")
Set Ready For Review
Related details
- Antonio Rivera
- Antonio Sartori
- Avi Drissman
- Dmitry Gozman
- James Cook
- Matt Reynolds
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Christopher Staite (Gerrit)
Christopher Staite added 1 comment![]( "Open in Gerrit")
I think I've resolved the dependency issues now with a move to common. If I could get a CQ+1 to validate that will save me a lot of build time. Thanks.
Christopher Staite (Gerrit)
Christopher Staite added 1 comment![]( "Open in Gerrit")
#include "content/public/browser/child_process_id.h"Christopher StaiteCode in //extensions/renderer is renderer code and cannot depend on browser code. It looks like this file and the test only rely on kInvalidChildProcessUniqueId. Perhaps that could move back to content_constants.h, since it's used in more than just browser code?
Antonio SartoriThe issue is that depending on that causes a circular dependency that I struggled to unpick. Very welcome to suggestions here. Perhaps we should move child_process_id.h to common?
Christopher StaiteCan't you put this in `content/common` since it needs to be used by renderer code? What is the circular dependency issue?
Antonio Sartori`child_process_id.h` depended on `content_constants.h` for the `kInvalidChildProcessUniqueId`. There was a circular dependency that `child_process_id.h` was depended on by `content/common:common_sources` which then depended on `content/common:child_process_id`.
Christopher StaiteI am looking again at this. If you are sending ChildProcessId via mojo it likely means that you'll need to use it from other processes than the browser process. Hence ChildProcessId and its mojo related code should not be in //content/public/browser, but in //content/public/common, no?
Before investigating dependency problems more deeply, I think you should fix that.
Christopher StaiteA very good point, and makes a lot of sense. To avoid conflating issues, I've raised a separate CL for this and then I'll rebase this one over it which should null out a lot of the changes.
Done
Christopher Staite (Gerrit)
Christopher Staite added 1 comment![]( "Open in Gerrit")
Looks like the tests at least are sending an invalid value (-1 or 0):
```
[3364:7468:0119/070053.551:INFO:CONSOLE:114] "Requesting immersive VR session", source: https://127.0.0.1:50618/chrome/test/data/xr/e2e_test_files/resources/webxr_boilerplate.js (114)
[3364:7468:0119/070053.551:INFO:chrome\browser\vr\test\xr_browser_test.cc:312] Run JavaScript: sessionInfos[sessionTypes.IMMERSIVE].currentSession != null
[8992:9448:0119/070053.553:ERROR:mojo\public\cpp\bindings\lib\validation_errors.cc:112] Invalid message: VALIDATION_ERROR_DESERIALIZATION_FAILED
[8992:9448:0119/070053.553:ERROR:mojo\public\cpp\bindings\lib\interface_endpoint_client.cc:748] Message 0 rejected by interface device.mojom.XRRuntime
[3364:7468:0119/070053.556:INFO:CONSOLE:0] "Could not create a session because: An unknown error occurred", source: https://127.0.0.1:50618/chrome/test/data/xr/e2e_test_files/html/generic_webxr_page.html (0)
```
I'm not really sure why as of yet, have you got any clues @mattre...@chromium.org?
Related details
- Matt Reynolds
Christopher Staite (Gerrit)
Christopher Staite added 1 comment![]( "Open in Gerrit")
Looks like the tests at least are sending an invalid value (-1 or 0):
```
[3364:7468:0119/070053.551:INFO:CONSOLE:114] "Requesting immersive VR session", source: https://127.0.0.1:50618/chrome/test/data/xr/e2e_test_files/resources/webxr_boilerplate.js (114)
[3364:7468:0119/070053.551:INFO:chrome\browser\vr\test\xr_browser_test.cc:312] Run JavaScript: sessionInfos[sessionTypes.IMMERSIVE].currentSession != null
[8992:9448:0119/070053.553:ERROR:mojo\public\cpp\bindings\lib\validation_errors.cc:112] Invalid message: VALIDATION_ERROR_DESERIALIZATION_FAILED
[8992:9448:0119/070053.553:ERROR:mojo\public\cpp\bindings\lib\interface_endpoint_client.cc:748] Message 0 rejected by interface device.mojom.XRRuntime
[3364:7468:0119/070053.556:INFO:CONSOLE:0] "Could not create a session because: An unknown error occurred", source: https://127.0.0.1:50618/chrome/test/data/xr/e2e_test_files/html/generic_webxr_page.html (0)
```
I'm not really sure why as of yet, have you got any clues @mattre...@chromium.org?
I think I found it. The values are optional and only sent on Android, hence the Windows tests failing. I've moved them to an optional struct and made the code fail if they aren't populated on Android. I set them to invalid values on Windows. Perhaps it would be better to have another optional in `OpenXrCreateInfo`?