|
|
DescriptionAdd DirectComposition overlay support.
Promote DC layers to DirectComposition SwapChains. Currently only videos
are supported, so an ID3D11VideoProcessor is used to copy the video data
from the DXGI image's texture into the YUY2 swapchain backbuffer.
BUG=678800
NOTRY=true
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Review-Url: https://codereview.chromium.org/2749023011
Cr-Commit-Position: refs/heads/master@{#460543}
Committed: https://chromium.googlesource.com/chromium/src/+/78f5d7d5bae564e75a5bc7fb2f65a1e67ff9547c
Patch Set 1 #Patch Set 2 : fix deps #Patch Set 3 : fix build #Patch Set 4 : rebase #Patch Set 5 : rebase #Patch Set 6 : minor changes #Patch Set 7 : remove unused method #
Total comments: 19
Patch Set 8 : post-review changes #
Total comments: 8
Patch Set 9 : post-review changes #
Total comments: 2
Patch Set 10 : add comment #
Total comments: 1
Patch Set 11 : remove skia usage in unit test #
Dependent Patchsets: Messages
Total messages: 68 (52 generated)
Description was changed from ========== Add DirectComposition overlay support. Promote DC layers to DirectComposition SwapChains. Currently only videos are supported, so an ID3D11VideoProcessor is used to copy the video data from the DXGI image's texture into the YUY2 swapchain backbuffer. BUG=678800 ========== to ========== Add DirectComposition overlay support. Promote DC layers to DirectComposition SwapChains. Currently only videos are supported, so an ID3D11VideoProcessor is used to copy the video data from the DXGI image's texture into the YUY2 swapchain backbuffer. BUG=678800 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
The CQ bit was checked by jbauman@chromium.org to run a CQ dry run
Dry run: 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
Dry run: Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by jbauman@chromium.org to run a CQ dry run
Dry run: 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
Dry run: Try jobs failed on following builders: win_clang on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)
The CQ bit was checked by jbauman@chromium.org to run a CQ dry run
Dry run: 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
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by jbauman@chromium.org to run a CQ dry run
Dry run: 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
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...) ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by jbauman@chromium.org to run a CQ dry run
Dry run: 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
Dry run: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't started yet, builder probably lacks capacity) ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't started yet, builder probably lacks capacity)
The CQ bit was checked by jbauman@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by jbauman@chromium.org to run a CQ dry run
jbauman@chromium.org changed reviewers: + sunnyps@chromium.org
Dry run: 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
Dry run: This issue passed the CQ dry run.
sunnyps, friendly ping
Sorry for the delay in reviewing. Feel free to use different method/variable names from those suggested below. https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... File gpu/ipc/service/direct_composition_surface_win.cc (right): https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win.cc:85: base::win::ScopedComPtr<ID3D11VideoProcessor> video_processor() { nit: const method and const ref return here and below https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win.cc:100: base::win::ScopedComPtr<IDCompositionVisual2> visual; nit: content_visual? https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win.cc:237: for (size_t i = 0; i < caps.RateConversionCapsCount; i++) { Why is this loop needed? https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win.cc:258: create_surface_handle_function_ = nit: should this be a CHECK? https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win.cc:298: swap_chain_->GetBuffer(0, IID_PPV_ARGS(texture.Receive())); Do we need to cycle through the back buffers of the swap chain? https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win.cc:302: out_view_.Release(); nit: out_view_.Release isn't needed here https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win.cc:403: // This should be hit in production but is a simple fallback for nit: should not be hit? https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win.cc:416: hr = swap_chain3->SetColorSpace1( What's the significance of setting the color space here? https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win.cc:440: while (visual_info_.size() > pending_overlays_.size()) { I think it's OK to keep extra visual infos around or at least have some sort of delay (60 frames?) after which we remove them. Otherwise you can have something like 30 fps video with 60 fps animation alternating between 1 and 2 overlays, in which case you'd end up creating a swap chain every other frame. https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win.cc:448: for (size_t i = 0; i < pending_overlays_.size(); i++) { Can you add a comment saying what the visual tree looks like? AFAICT root visual has a bunch of children clip visuals (sorted by z order) each of which has a child visual (the actual content). https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win.cc:452: base::win::ScopedComPtr<IDCompositionVisual2> this_visual; nit: content_visual? https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win.cc:454: if (!visual_info->visual) { Can you factor this into InitVisual(VisualInfo*)? https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win.cc:471: params.image->GetType() == gl::GLImage::Type::DXGI_IMAGE) { Can you factor the body of this into UpdateVisualForVideo(VisualInfo*, const ui::DCRendererLayerParams&)? https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win.cc:520: // Main backbuffer. Can you factor this into UpdateVisualForBackbuffer? https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win.cc:520: // Main backbuffer. If visual_info was used by a video overlay in the last frame, then both dcomp_surface and swap_chain conditions below will evaluate to true. So, SetContent will be called twice, once with the dcomp_surface and then with the swap_chain. https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win.cc:544: if (params.is_clipped != visual_info->is_clipped || Can you factor this into UpdateVisualClip? https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win.cc:574: pending_overlays_.push_back( Is there a reason for heap allocating DCRendererLayerParams in pending_overlays_? https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... File gpu/ipc/service/direct_composition_surface_win.h (right): https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win.h:60: base::win::ScopedComPtr<IDCompositionSurface> dcomp_surface() { nit: const?
The CQ bit was checked by jbauman@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
PTAL. Thanks for looking at this. On 2017/03/28 00:42:24, sunnyps wrote: > https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... > gpu/ipc/service/direct_composition_surface_win.cc:237: for (size_t i = 0; i < > caps.RateConversionCapsCount; i++) { > Why is this loop needed? > Oops, I was using this for logging but forgot to remove it. > https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... > gpu/ipc/service/direct_composition_surface_win.cc:258: > create_surface_handle_function_ = > nit: should this be a CHECK? Maybe DCHECK is better unless something really is terrible. > > https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... > gpu/ipc/service/direct_composition_surface_win.cc:298: swap_chain_->GetBuffer(0, > IID_PPV_ARGS(texture.Receive())); > Do we need to cycle through the back buffers of the swap chain? > No, DXGI does that behind the scenes for D3D11 contexts. > https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... > gpu/ipc/service/direct_composition_surface_win.cc:302: out_view_.Release(); > nit: out_view_.Release isn't needed here > We still need it in the case where the video processor changes but the swapchain doesn't. I've added a comment. > https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... > gpu/ipc/service/direct_composition_surface_win.cc:416: hr = > swap_chain3->SetColorSpace1( > What's the significance of setting the color space here? This is the source colorspace for the overlay. The current setting makes most videos look correct, but I'm working on another patch to use the real colorspace. > > https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... > gpu/ipc/service/direct_composition_surface_win.cc:440: while > (visual_info_.size() > pending_overlays_.size()) { > I think it's OK to keep extra visual infos around or at least have some sort of > delay (60 frames?) after which we remove them. Otherwise you can have something > like 30 fps video with 60 fps animation alternating between 1 and 2 overlays, in > which case you'd end up creating a swap chain every other frame. > Even if the video is 30fps it should remain visible even when it's not changing, so we'll keep the visual for it around. This current mechanism seems ok for most pages I've tried, though I'm looking into how best to cache SwapChainPresenters when the compositor reconfigures layers. I think that can go into a followup patch. > > https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... > gpu/ipc/service/direct_composition_surface_win.cc:574: > pending_overlays_.push_back( > Is there a reason for heap allocating DCRendererLayerParams in > pending_overlays_? It has a const element so it's not copy assignable and can't be put into a vector. Also it's pretty big, so copying on resize of the vector isn't great.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Can you add a comment explaining how SwapBuffers interacts with ScheduleDCLayer? In particular that we don't need to call ScheduleDCLayer for the main backbuffer. https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... File gpu/ipc/service/direct_composition_surface_win.cc (right): https://codereview.chromium.org/2749023011/diff/120001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win.cc:258: create_surface_handle_function_ = On 2017/03/28 00:42:24, sunnyps wrote: > nit: should this be a CHECK? I actually meant the check for dcomp.dll. https://codereview.chromium.org/2749023011/diff/140001/gpu/ipc/service/direct... File gpu/ipc/service/direct_composition_surface_win.cc (right): https://codereview.chromium.org/2749023011/diff/140001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win.cc:159: const base::win::ScopedComPtr<IDXGISwapChain1> swap_chain() const { nit: I think you meant to return const ref here. https://codereview.chromium.org/2749023011/diff/140001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win.cc:307: out_view_.Release(); nit: out_view_.Release() here is a nop https://codereview.chromium.org/2749023011/diff/140001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win.cc:364: video_processor_ = surface_->video_processor(); nit: should we call out_view_.Release() here like we do in ReallocateSwapChain instead of doing it in PresentToSwapChain? https://codereview.chromium.org/2749023011/diff/140001/gpu/ipc/service/direct... File gpu/ipc/service/direct_composition_surface_win.h (right): https://codereview.chromium.org/2749023011/diff/140001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win.h:60: base::win::ScopedComPtr<IDCompositionSurface> dcomp_surface() const { nit: return const ref here https://codereview.chromium.org/2749023011/diff/140001/gpu/ipc/service/direct... File gpu/ipc/service/direct_composition_surface_win_unittest.cc (right): https://codereview.chromium.org/2749023011/diff/140001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win_unittest.cc:240: bool CheckIfSupported() { nit: Can you move this to a global anonymous namespace and use it in the other tests too? https://codereview.chromium.org/2749023011/diff/140001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win_unittest.cc:268: InitializeSurface(); nit: Can you split this into two tests: DCLayersEnabled and DCLayersDisabled? https://codereview.chromium.org/2749023011/diff/140001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win_unittest.cc:323: const int kMargin = 10; What's the reason for having a margin here? Can you add a comment about that? https://codereview.chromium.org/2749023011/diff/140001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win_unittest.cc:330: TEST_F(DirectCompositionPixelTest, PixelTestVideoSwapchain) { nit: VideoSwapchain instead of PixelTestVideoSwapchain
The CQ bit was checked by jbauman@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
jbauman@chromium.org changed reviewers: + bungeman@chromium.org, sadrul@chromium.org
sunnyps: PTAL sadrul: please review adding a DEPS on ui/platform_window (so we can create a window for a test) bungeman: please review adding a DEPS on skia/ext (so we can create a platform canvas for a test).
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...) win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
lgtm % nits https://codereview.chromium.org/2749023011/diff/160001/gpu/ipc/service/direct... File gpu/ipc/service/direct_composition_surface_win.cc (right): https://codereview.chromium.org/2749023011/diff/160001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win.cc:263: DCHECK(create_surface_handle_function_); nit: should this also be a CHECK? https://codereview.chromium.org/2749023011/diff/160001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win.cc:855: // This surface's backbuffer doesn't have to be scheduled with nit: Can you move this comment to the include file above ScheduleDCLayer? Maybe also mention that the layer tree commit happens in SwapBuffers.
The CQ bit was checked by jbauman@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
jbauman@chromium.org changed reviewers: + bungeman@google.com
(adding the correct bungeman@ email address this time).
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
bungeman@google.com changed reviewers: + reed@chromium.org
reed@ was working on this most recently and probably has a better idea of direction. https://codereview.chromium.org/2749023011/diff/180001/gpu/ipc/service/direct... File gpu/ipc/service/direct_composition_surface_win_unittest.cc (right): https://codereview.chromium.org/2749023011/diff/180001/gpu/ipc/service/direct... gpu/ipc/service/direct_composition_surface_win_unittest.cc:216: std::unique_ptr<SkCanvas> canvas = skia::CreatePlatformCanvas( As far as I know we're trying to remove skia::CreatePlatformCanvas. It appears that there is no actual use of 'canvas' here, the code is just being used for the side effect of creating an HDC. Indeed, there's no actual use of SkBitmap here either (we're also trying to phase out SkBitmap where possible), it's just being used as a means to extract a single pixel value out of an HDC (this function could just take an x,y coordinate and return the pixel value at that location directly instead of a whole bitmap). In other words, it doesn't seem like Skia is used here at all, just the logic in raster_handle_allocator_win.cc around creating and destroying an HDC.
The CQ bit was checked by jbauman@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sunnyps@chromium.org, sadrul@chromium.org Link to the patchset: https://codereview.chromium.org/2749023011/#ps200001 (title: "remove skia usage in unit test")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
PS11 lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
The CQ bit was checked by jbauman@chromium.org
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 jbauman@chromium.org
Description was changed from ========== Add DirectComposition overlay support. Promote DC layers to DirectComposition SwapChains. Currently only videos are supported, so an ID3D11VideoProcessor is used to copy the video data from the DXGI image's texture into the YUY2 swapchain backbuffer. BUG=678800 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Add DirectComposition overlay support. Promote DC layers to DirectComposition SwapChains. Currently only videos are supported, so an ID3D11VideoProcessor is used to copy the video data from the DXGI image's texture into the YUY2 swapchain backbuffer. BUG=678800 NOTRY=true CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
The CQ bit was checked by jbauman@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": 200001, "attempt_start_ts": 1490823536727310, "parent_rev": "eb549628eb3e134f66bebea123a27d89dcf43dbb", "commit_rev": "78f5d7d5bae564e75a5bc7fb2f65a1e67ff9547c"}
Message was sent while issue was closed.
Description was changed from ========== Add DirectComposition overlay support. Promote DC layers to DirectComposition SwapChains. Currently only videos are supported, so an ID3D11VideoProcessor is used to copy the video data from the DXGI image's texture into the YUY2 swapchain backbuffer. BUG=678800 NOTRY=true CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Add DirectComposition overlay support. Promote DC layers to DirectComposition SwapChains. Currently only videos are supported, so an ID3D11VideoProcessor is used to copy the video data from the DXGI image's texture into the YUY2 swapchain backbuffer. BUG=678800 NOTRY=true CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2749023011 Cr-Commit-Position: refs/heads/master@{#460543} Committed: https://chromium.googlesource.com/chromium/src/+/78f5d7d5bae564e75a5bc7fb2f65... ==========
Message was sent while issue was closed.
Committed patchset #11 (id:200001) as https://chromium.googlesource.com/chromium/src/+/78f5d7d5bae564e75a5bc7fb2f65... |