Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2749703007:
Add menu mode plumbing for WebVR mode. (Closed)
DescriptionAdd menu mode plumbing for WebVR mode.
- Allow the UI to suppress rendering of WebVR content.
- Allow the UI to enable/disable the cursor.
- Move API-related Javascript to the API module.
- Other related cleanup.
This change does not see menu mode work yet. crbug.com/703177 needs to
be resolved first.
BUG=687960
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2749703007
Cr-Commit-Position: refs/heads/master@{#458175}
Committed: https://chromium.googlesource.com/chromium/src/+/314b385358094f67c6afaaf5dabe3cbe1955dbd4
Patch Set 1 #
Total comments: 26
Patch Set 2 : Address comments. #
Total comments: 5
Patch Set 3 : Remove invalid static_assert TODOs. Thanks Michael! #Patch Set 4 : Rename JS WebVR rendering functions for clarity. #
Depends on Patchset: Messages
Total messages: 25 (9 generated)
Description was changed from ========== Enable menu mode while in WebVR. - Allow the UI to suppress rendering of WebVR content. - Allow the UI to enable/disable the cursor. - Move API-related Javascript to the API module. - Other related cleanup. BUG=687960 ========== to ========== Enable menu mode while in WebVR. - Allow the UI to suppress rendering of WebVR content. - Allow the UI to enable/disable the cursor. - Move API-related Javascript to the API module. - Other related cleanup. BUG=687960 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
cjgrant@chromium.org changed reviewers: + mthiesse@chromium.org, tiborg@chromium.org
Folks, a couple of notes: - This CL continues a CL started by Tibor (credit is due). - This change unearthed several things I want to clean up. Most will come in a separate CL, but some found its way in here. - Please see the few initial comments I'll post...
Folks, a couple of notes: - This CL continues a CL started by Tibor (credit is due). - This change unearthed several things I want to clean up. Most will come in a separate CL, but some found its way in here. - Please see the few initial comments I'll post...
https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... File chrome/browser/android/vr_shell/ui_scene.cc (right): https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... chrome/browser/android/vr_shell/ui_scene.cc:325: data->GetBoolean("drawWebVr", &webvr_rendering_enabled_); We could merge this with the action "pause content", or keep them separate. I'd prefer to merge them, unless there's disagreement. https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... File chrome/browser/android/vr_shell/vr_shell_gl.cc (right): https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... chrome/browser/android/vr_shell/vr_shell_gl.cc:702: if (ShouldDrawWebVr()) { Seeing the number of spots we call this, a couple things come to mind: - Do we set web_vr_mode_ on this thread, such that it can't change mid-render-loop. - Will skipping WebVR rendering while in the menu cause any pose queues (or related) to get out of sync? https://codereview.chromium.org/2749703007/diff/1/chrome/browser/resources/vr... File chrome/browser/resources/vr_shell/vr_shell_ui_scene.js (left): https://codereview.chromium.org/2749703007/diff/1/chrome/browser/resources/vr... chrome/browser/resources/vr_shell/vr_shell_ui_scene.js:133: * Set the background color of the scene. Dropping comments as these are direct wrappers on the documented API.
https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... File chrome/browser/android/vr_shell/ui_scene.cc (right): https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... chrome/browser/android/vr_shell/ui_scene.cc:325: data->GetBoolean("drawWebVr", &webvr_rendering_enabled_); On 2017/03/16 15:44:36, cjgrant wrote: > We could merge this with the action "pause content", or keep them separate. I'd > prefer to merge them, unless there's disagreement. +1 for merging. https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... chrome/browser/android/vr_shell/ui_scene.cc:356: const Colorf& UiScene::GetBackgroundColor() const { +1 for const! https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... File chrome/browser/android/vr_shell/vr_shell_gl.cc (right): https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... chrome/browser/android/vr_shell/vr_shell_gl.cc:702: if (ShouldDrawWebVr()) { On 2017/03/16 15:44:36, cjgrant wrote: > Seeing the number of spots we call this, a couple things come to mind: > - Do we set web_vr_mode_ on this thread, such that it can't change > mid-render-loop. > - Will skipping WebVR rendering while in the menu cause any pose queues (or > related) to get out of sync? Yes, web_vr_mode_ should only be set by the GL thread. So, it can't change in the middle of the render loop. The second question I don't know. https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... chrome/browser/android/vr_shell/vr_shell_gl.cc:705: // TODO(cjgrant): Do not compare the constants on every frame. Make a bug for this? https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... chrome/browser/android/vr_shell/vr_shell_gl.cc:777: // TODO(cjgrant): Do not compare the constants on every frame. Here, too. https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... chrome/browser/android/vr_shell/vr_shell_gl.cc:913: if (draw_cursor) Isn't it better, i.e. less error prone, to use curly braces even though there is only one line in the if? https://codereview.chromium.org/2749703007/diff/1/chrome/browser/resources/vr... File chrome/browser/resources/vr_shell/vr_shell_ui_api.js (right): https://codereview.chromium.org/2749703007/diff/1/chrome/browser/resources/vr... chrome/browser/resources/vr_shell/vr_shell_ui_api.js:587: setWebVrRenderingEnabled(enabled) { Maybe call it setShowWebVrPresentation or so? Since WebVR is still rendered. https://codereview.chromium.org/2749703007/diff/1/chrome/browser/resources/vr... File chrome/browser/resources/vr_shell/vr_shell_ui_scene.js (left): https://codereview.chromium.org/2749703007/diff/1/chrome/browser/resources/vr... chrome/browser/resources/vr_shell/vr_shell_ui_scene.js:133: * Set the background color of the scene. On 2017/03/16 15:44:37, cjgrant wrote: > Dropping comments as these are direct wrappers on the documented API. Acknowledged.
Couple of quick responses and an update: This CL exposes an issue in which WebVR rendering causes controller input processing to be skipped. Working on a workaround or fix now. https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... File chrome/browser/android/vr_shell/vr_shell_gl.cc (right): https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... chrome/browser/android/vr_shell/vr_shell_gl.cc:705: // TODO(cjgrant): Do not compare the constants on every frame. On 2017/03/16 19:40:22, tiborg wrote: > Make a bug for this? For something this trivial, I don't think a bug is justified - nobody would ever prioritize it. https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... chrome/browser/android/vr_shell/vr_shell_gl.cc:913: if (draw_cursor) On 2017/03/16 19:40:22, tiborg wrote: > Isn't it better, i.e. less error prone, to use curly braces even though there is > only one line in the if? I prefer to, but senior reviewers keep telling me not to use braces in the one-line case.
https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... File chrome/browser/android/vr_shell/vr_shell_gl.cc (right): https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... chrome/browser/android/vr_shell/vr_shell_gl.cc:913: if (draw_cursor) On 2017/03/16 21:21:51, cjgrant wrote: > On 2017/03/16 19:40:22, tiborg wrote: > > Isn't it better, i.e. less error prone, to use curly braces even though there > is > > only one line in the if? > > I prefer to, but senior reviewers keep telling me not to use braces in the > one-line case. Interesting. The style guide says you can do either: https://google.github.io/styleguide/cppguide.html Even the chromium style guide doesn't say anything about it: https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md I would probably do whatever senior reviewers say as well, but I think you could make a case for always using braces (which I prefer as well) as long as it didn't feel like it changed the conventions in an existing file.
https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... File chrome/browser/android/vr_shell/vr_shell_gl.cc (right): https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... chrome/browser/android/vr_shell/vr_shell_gl.cc:705: // TODO(cjgrant): Do not compare the constants on every frame. On 2017/03/16 21:21:51, cjgrant wrote: > On 2017/03/16 19:40:22, tiborg wrote: > > Make a bug for this? > > For something this trivial, I don't think a bug is justified - nobody would ever > prioritize it. Yes, it was just a suggestion. You could make a bug and assign it to yourself. Than it is tracked. But I will not fight on this. https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... chrome/browser/android/vr_shell/vr_shell_gl.cc:913: if (draw_cursor) On 2017/03/17 00:26:46, amp wrote: > On 2017/03/16 21:21:51, cjgrant wrote: > > On 2017/03/16 19:40:22, tiborg wrote: > > > Isn't it better, i.e. less error prone, to use curly braces even though > there > > is > > > only one line in the if? > > > > I prefer to, but senior reviewers keep telling me not to use braces in the > > one-line case. > > Interesting. The style guide says you can do either: > https://google.github.io/styleguide/cppguide.html > > Even the chromium style guide doesn't say anything about it: > https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md > > I would probably do whatever senior reviewers say as well, but I think you could > make a case for always using braces (which I prefer as well) as long as it > didn't feel like it changed the conventions in an existing file. If we all prefer it why not embrace it in our code? Unless senior reviewers would not let us through then.
https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... File chrome/browser/android/vr_shell/ui_scene.cc (right): https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... chrome/browser/android/vr_shell/ui_scene.cc:325: data->GetBoolean("drawWebVr", &webvr_rendering_enabled_); On 2017/03/16 19:40:22, tiborg wrote: > On 2017/03/16 15:44:36, cjgrant wrote: > > We could merge this with the action "pause content", or keep them separate. > I'd > > prefer to merge them, unless there's disagreement. > > +1 for merging. In the end, I merged the two rendering properties, but not the pausing. Pausing is done by a separate native system, and linking them is messy. https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... File chrome/browser/android/vr_shell/vr_shell_gl.cc (right): https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... chrome/browser/android/vr_shell/vr_shell_gl.cc:702: if (ShouldDrawWebVr()) { On 2017/03/16 19:40:22, tiborg wrote: > On 2017/03/16 15:44:36, cjgrant wrote: > > Seeing the number of spots we call this, a couple things come to mind: > > - Do we set web_vr_mode_ on this thread, such that it can't change > > mid-render-loop. > > - Will skipping WebVR rendering while in the menu cause any pose queues (or > > related) to get out of sync? > > Yes, web_vr_mode_ should only be set by the GL thread. So, it can't change in > the middle of the render loop. The second question I don't know. So it turns out that WebVR changes have fully broken the app button press when WebVR is rendering, anyway. I made a bug to work on that, after this plumbing lands. https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... chrome/browser/android/vr_shell/vr_shell_gl.cc:705: // TODO(cjgrant): Do not compare the constants on every frame. On 2017/03/17 15:13:44, tiborg wrote: > On 2017/03/16 21:21:51, cjgrant wrote: > > On 2017/03/16 19:40:22, tiborg wrote: > > > Make a bug for this? > > > > For something this trivial, I don't think a bug is justified - nobody would > ever > > prioritize it. > > Yes, it was just a suggestion. You could make a bug and assign it to yourself. > Than it is tracked. But I will not fight on this. So I've tied this cleanup to the UI math optimization bug. It's not really related, but that seems like a great time to circle back and clean up this kind of thing (there may be other cleanups too of course). https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... chrome/browser/android/vr_shell/vr_shell_gl.cc:913: if (draw_cursor) On 2017/03/17 15:13:43, tiborg wrote: > On 2017/03/17 00:26:46, amp wrote: > > On 2017/03/16 21:21:51, cjgrant wrote: > > > On 2017/03/16 19:40:22, tiborg wrote: > > > > Isn't it better, i.e. less error prone, to use curly braces even though > > there > > > is > > > > only one line in the if? > > > > > > I prefer to, but senior reviewers keep telling me not to use braces in the > > > one-line case. > > > > Interesting. The style guide says you can do either: > > https://google.github.io/styleguide/cppguide.html > > > > Even the chromium style guide doesn't say anything about it: > > https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md > > > > I would probably do whatever senior reviewers say as well, but I think you > could > > make a case for always using braces (which I prefer as well) as long as it > > didn't feel like it changed the conventions in an existing file. > > If we all prefer it why not embrace it in our code? Unless senior reviewers > would not let us through then. We're the owners, so we can go either way. And, we already mix styles in this module. But I like braces too, so lets go for it. https://codereview.chromium.org/2749703007/diff/1/chrome/browser/resources/vr... File chrome/browser/resources/vr_shell/vr_shell_ui_api.js (right): https://codereview.chromium.org/2749703007/diff/1/chrome/browser/resources/vr... chrome/browser/resources/vr_shell/vr_shell_ui_api.js:587: setWebVrRenderingEnabled(enabled) { On 2017/03/16 19:40:23, tiborg wrote: > Maybe call it setShowWebVrPresentation or so? Since WebVR is still rendered. Now that this controls rendering of WebVR vs UI and cursor, I like the original name better. Or, it could be setWebVrRenderingModeEnabled(), to show it has a broader impact... Thoughts?
LGTM after the renaming. https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... File chrome/browser/android/vr_shell/ui_scene.cc (right): https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... chrome/browser/android/vr_shell/ui_scene.cc:325: data->GetBoolean("drawWebVr", &webvr_rendering_enabled_); On 2017/03/20 15:34:57, cjgrant wrote: > On 2017/03/16 19:40:22, tiborg wrote: > > On 2017/03/16 15:44:36, cjgrant wrote: > > > We could merge this with the action "pause content", or keep them separate. > > I'd > > > prefer to merge them, unless there's disagreement. > > > > +1 for merging. > > In the end, I merged the two rendering properties, but not the pausing. Pausing > is done by a separate native system, and linking them is messy. SGTM. https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... File chrome/browser/android/vr_shell/vr_shell_gl.cc (right): https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... chrome/browser/android/vr_shell/vr_shell_gl.cc:702: if (ShouldDrawWebVr()) { On 2017/03/20 15:34:57, cjgrant wrote: > On 2017/03/16 19:40:22, tiborg wrote: > > On 2017/03/16 15:44:36, cjgrant wrote: > > > Seeing the number of spots we call this, a couple things come to mind: > > > - Do we set web_vr_mode_ on this thread, such that it can't change > > > mid-render-loop. > > > - Will skipping WebVR rendering while in the menu cause any pose queues (or > > > related) to get out of sync? > > > > Yes, web_vr_mode_ should only be set by the GL thread. So, it can't change in > > the middle of the render loop. The second question I don't know. > > So it turns out that WebVR changes have fully broken the app button press when > WebVR is rendering, anyway. I made a bug to work on that, after this plumbing > lands. Acknowledged. https://codereview.chromium.org/2749703007/diff/1/chrome/browser/android/vr_s... chrome/browser/android/vr_shell/vr_shell_gl.cc:705: // TODO(cjgrant): Do not compare the constants on every frame. On 2017/03/20 15:34:57, cjgrant wrote: > On 2017/03/17 15:13:44, tiborg wrote: > > On 2017/03/16 21:21:51, cjgrant wrote: > > > On 2017/03/16 19:40:22, tiborg wrote: > > > > Make a bug for this? > > > > > > For something this trivial, I don't think a bug is justified - nobody would > > ever > > > prioritize it. > > > > Yes, it was just a suggestion. You could make a bug and assign it to yourself. > > Than it is tracked. But I will not fight on this. > > So I've tied this cleanup to the UI math optimization bug. It's not really > related, but that seems like a great time to circle back and clean up this kind > of thing (there may be other cleanups too of course). Great, thanks. https://codereview.chromium.org/2749703007/diff/1/chrome/browser/resources/vr... File chrome/browser/resources/vr_shell/vr_shell_ui_api.js (right): https://codereview.chromium.org/2749703007/diff/1/chrome/browser/resources/vr... chrome/browser/resources/vr_shell/vr_shell_ui_api.js:587: setWebVrRenderingEnabled(enabled) { On 2017/03/20 15:34:57, cjgrant wrote: > On 2017/03/16 19:40:23, tiborg wrote: > > Maybe call it setShowWebVrPresentation or so? Since WebVR is still rendered. > > Now that this controls rendering of WebVR vs UI and cursor, I like the original > name better. Or, it could be setWebVrRenderingModeEnabled(), to show it has a > broader impact... Thoughts? setWebVrRenderingModeEnabled() sounds good to me.
https://codereview.chromium.org/2749703007/diff/20001/chrome/browser/android/... File chrome/browser/android/vr_shell/ui_scene.cc (right): https://codereview.chromium.org/2749703007/diff/20001/chrome/browser/android/... chrome/browser/android/vr_shell/ui_scene.cc:366: return webvr_rendering_enabled_; This defaulting to false is going to dramatically slow down the transition into WebVR. As in, we won't show webVR until the UI was loaded, where this previously wasn't the case. This might be fine, but at the very least we shouldn't be requesting webVR frames until we're showing them. https://codereview.chromium.org/2749703007/diff/20001/chrome/browser/android/... File chrome/browser/android/vr_shell/vr_shell_gl.cc (right): https://codereview.chromium.org/2749703007/diff/20001/chrome/browser/android/... chrome/browser/android/vr_shell/vr_shell_gl.cc:723: // TODO(crbug.com/680253): Do not compare the constants on every frame. Note that this is a static assert and is checked by the compiler, not run on every frame. https://codereview.chromium.org/2749703007/diff/20001/chrome/browser/android/... chrome/browser/android/vr_shell/vr_shell_gl.cc:795: // TODO(crbug.com/680253): Do not compare the constants on every frame. Same as above.
lgtm https://codereview.chromium.org/2749703007/diff/20001/chrome/browser/android/... File chrome/browser/android/vr_shell/ui_scene.cc (right): https://codereview.chromium.org/2749703007/diff/20001/chrome/browser/android/... chrome/browser/android/vr_shell/ui_scene.cc:366: return webvr_rendering_enabled_; On 2017/03/20 17:32:16, mthiesse wrote: > This defaulting to false is going to dramatically slow down the transition into > WebVR. As in, we won't show webVR until the UI was loaded, where this previously > wasn't the case. > > This might be fine, but at the very least we shouldn't be requesting webVR > frames until we're showing them. Disregard this I misread.
https://codereview.chromium.org/2749703007/diff/1/chrome/browser/resources/vr... File chrome/browser/resources/vr_shell/vr_shell_ui_api.js (right): https://codereview.chromium.org/2749703007/diff/1/chrome/browser/resources/vr... chrome/browser/resources/vr_shell/vr_shell_ui_api.js:587: setWebVrRenderingEnabled(enabled) { On 2017/03/20 15:56:08, tiborg wrote: > On 2017/03/20 15:34:57, cjgrant wrote: > > On 2017/03/16 19:40:23, tiborg wrote: > > > Maybe call it setShowWebVrPresentation or so? Since WebVR is still rendered. > > > > Now that this controls rendering of WebVR vs UI and cursor, I like the > original > > name better. Or, it could be setWebVrRenderingModeEnabled(), to show it has a > > broader impact... Thoughts? > > setWebVrRenderingModeEnabled() sounds good to me. Done. https://codereview.chromium.org/2749703007/diff/20001/chrome/browser/android/... File chrome/browser/android/vr_shell/vr_shell_gl.cc (right): https://codereview.chromium.org/2749703007/diff/20001/chrome/browser/android/... chrome/browser/android/vr_shell/vr_shell_gl.cc:723: // TODO(crbug.com/680253): Do not compare the constants on every frame. On 2017/03/20 17:32:16, mthiesse wrote: > Note that this is a static assert and is checked by the compiler, not run on > every frame. Done. Sweet, I learned something cool today!
Description was changed from ========== Enable menu mode while in WebVR. - Allow the UI to suppress rendering of WebVR content. - Allow the UI to enable/disable the cursor. - Move API-related Javascript to the API module. - Other related cleanup. BUG=687960 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Add menu mode plumbing for WebVR mode. - Allow the UI to suppress rendering of WebVR content. - Allow the UI to enable/disable the cursor. - Move API-related Javascript to the API module. - Other related cleanup. This change does not see menu mode work yet. crbug.com/703177 needs to be resolved first. BUG=687960 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
The CQ bit was checked by cjgrant@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mthiesse@chromium.org, tiborg@chromium.org Link to the patchset: https://codereview.chromium.org/2749703007/#ps60001 (title: "Rename JS WebVR rendering functions for clarity.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by cjgrant@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1490039539879070,
"parent_rev": "ce4800c8fc7658f2862f1b92852b1d657d7fdf6a", "commit_rev":
"314b385358094f67c6afaaf5dabe3cbe1955dbd4"}
Message was sent while issue was closed.
Description was changed from ========== Add menu mode plumbing for WebVR mode. - Allow the UI to suppress rendering of WebVR content. - Allow the UI to enable/disable the cursor. - Move API-related Javascript to the API module. - Other related cleanup. This change does not see menu mode work yet. crbug.com/703177 needs to be resolved first. BUG=687960 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Add menu mode plumbing for WebVR mode. - Allow the UI to suppress rendering of WebVR content. - Allow the UI to enable/disable the cursor. - Move API-related Javascript to the API module. - Other related cleanup. This change does not see menu mode work yet. crbug.com/703177 needs to be resolved first. BUG=687960 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2749703007 Cr-Commit-Position: refs/heads/master@{#458175} Committed: https://chromium.googlesource.com/chromium/src/+/314b385358094f67c6afaaf5dabe... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/src/+/314b385358094f67c6afaaf5dabe... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
