Assigned to rsesek, since he recently changed the behavior of the Clone() call when resolving  bug 793503 .

Justification for P1: Resolution of  bug 793503  changed the Clone() call in content/browser/renderer_host/media/video_capture_controller.cc from READ_WRITE to READ_ONLY. Previously, via the default argument value, it was a Clone(READ_WRITE) call, which had special semantics: If the handle was READ_WRITE, the cloned handle would be READ_WRITE; but, if the handle was READ_ONLY, the cloned handle would be READ_ONLY even though READ_WRITE was requested.

My WIP code depended on this behavior. Now I am blocked.

FWIW, for the Video Capture use case, it really does not matter if clients can write to the memory. A reasonable short-term fix would be to change the Clone() call in VideoCaptureController back to READ_WRITE.

miu@: Sorry, I signed off on that change without realizing it would cause issues for your work. I did try to think of the screen/content capture use case but could not see/remember why it would need these special semantics.

I think we should do what you proposed in #1 and change the Clone() call in VideoCaptureController back to READ_WRITE. When we do, let us add a code comment there that points out that the special semantics are needed and a pointer to why they are needed.

I am happy to do the CL, but would need your input for that comment.

I'll send a patch for code review shortly. FWIW, once the base::SharedMemory problem is fixed, we should be able to revert the READ_WRITE "hack."

chfremer: Did you mean to take assignment of this? I think the root problem (original description) is in base::SharedMemory and, IIUC, rsesek@ was working to clean that up.

Actually digit@ is working on a larger change to shared memory after  issue 789959 , rather than me. Unless there's an obvious fix to the existing base::SharedMemory API, I think just reverting to READ_WRITE at this callsite is the best course of action.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c86dd6c5c5902c696e7171e9f7afbafab6b416d2

commit c86dd6c5c5902c696e7171e9f7afbafab6b416d2
Author: Yuri Wiitala <miu@chromium.org>
Date: Wed Jan 03 19:04:06 2018

Restore READ_WRITE flag to Clone() call for video capture buffers.

A recent change(*) switched a shmem buffer Clone() call from READ_WRITE
to READ_ONLY. New VIZ tab capture implementation depends on the special
semantics of the READ_WRITE argument, only in-so-much as it avoids a
crash due to what looks like a design flaw in base::SharedMemory
(details in crbug). This change reverts the call point, until a long-
term solution can be worked out.

Testing: Confirmed that the change works with both tab capture and
USB webcam capture.

* See: https://chromium-review.googlesource.com/820011 and
 http://crbug.com/793503 

Bug:  797470 
Change-Id: I9bffdd0b71cbfa37b6b5bda03ce8863b560d8747
Reviewed-on: https://chromium-review.googlesource.com/848239
Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526756}
[modify] https://crrev.com/c86dd6c5c5902c696e7171e9f7afbafab6b416d2/content/browser/renderer_host/media/video_capture_controller.cc

Per #c4, assigning to digit@ for the root-cause issue: The CHECK() fail in base::SharedMemory::GetReadOnlyHandle() when the mojo wrapper calls it. It seems base::SharedMemory has a different design than mojo w.r.t. duplicating RW handles as demoted-to-RO handles.

I should add: In this use case, the RW handle comes from shmem created in the VIZ process, passed through to the browser process, and finally to the renderer process. Both VIZ and the browser process require RW access to the shmem, and the browser process is where we'd want to demote it to RO for the renderer process.

So, base::SharedMemory's design, which requires the creator of the shmem to provide a separate RO handle, doesn't work: It's in a separate process where we want to demote the permissions to RO. The mojo API doesn't seem to account for this case.

miu@: The problem is that with android ashmem, the shared memory permissions are not actually set on the handle, but in the shared region itself. See [1] for details.

It could theoretically work if the VIZ process maps a RW buffer, passes it to the browser process which maps a RW buffer, and then the region is downgraded to RO before being passed to the renderer. It looks like that will be enough for you as you appear to map the shared memory before passing around the handle (eg, [2]). However, that model is not supported by the API proposed in [1].

The advantage of the proposed API [1] is that it is not possible to mistakenly share a writable memory region. But it seems that is too restrictive for your use case.

rsesek@: what is your opinion here from a security perspective? Would it be okay to bolt a downgrading API on top of the proposal in [1] and surround it with scary comments? It would likely be awkward, but that would discourage its use as well which would be a good thing. Maybe have a type of handle that when sent over a channel is immediately mapped RW and then downgraded RO, restricting the RW mappings to at most 2 endpoints. (but we should probably discuss a design not on this bug... ;)


[1] https://docs.google.com/document/d/1eiECEj0x8_VFqkdf61zjmh061rgdkfvIOUxle-q0AIE/edit#  Note this doc is currently google-only, but we will be making a public (Chromium) version shortly.

[2] https://cs.chromium.org/chromium/src/media/capture/video/shared_memory_handle_provider.h?rcl=56f31113259c561a93b5b47f5dc9d6575bc6bab6&l=47

From a security perspective, it is fine to allow something with read-write to drop it to read-only. I think if viz created a region RW, mapped it RW, and then transferred/moved (i.e. relinquished ownership of) the region handle to the browser via IPC, the browser could map RW as well and then drop the region to RO. I don't think exposing a SharedMemoryRegion::ConvertToReadOnly() API would be too bad. The situation to avoid, though, is where two processes have a region handle but have differing views as to the permissions on it.

miu@:

I'd like to ask a clarifying question. In the VIZ process, is the handle ever duplicated shared RO? Or is it only passed RW to the browser process, which then downgrades and passes to the renderer?

That is, the flow is only:

VIZ   |  browser    |  renderer
 RW---+-->RW ~ RO --+--->RO
      |             |

And never:

VIZ        |  browser    |  renderer
 RW--------+-->RW ~ RO --+--->RO
  \~ RO-\  |             |
        |
        |
        v
   somewhere else


(Hopefully the ascii art is more and not less comprehensible).

Alex & I have been talking and probably the reasonable way to do this is to never have a RW handle be able to be duplicated, only to be moved to another process. If a handle is duplicated it must be RO.

No, it's never duplicated as RO within VIZ. So, that's an acceptable assumption. (In fact, why would we want to encourage anyone to dup shmem handles for use inside the same process rather than just pass around raw or scoped pointers, right?)

BTW--In my current use case, the VIZ process is maintaining a shared memory pool; so it retains ownership of the original RW handle. So, wouldn't we still need to dup the RW handle for the browser process?

As I see it, the VIZ process would have a RW mapping to the region, the RW handle is passed to the browser process, who also creates a RW mapping, and then converts the region to RO. At that point all future mappings will be RO, no matter which process creates the mapping.

Ok, so to be clear: VIZ maps the original RW handle and does not unmap it while the handle is being duped for and used by other processes. So later, when in the browser process and we convert the region to RO, the mapping back in the VIZ process should still remain RW, correct?

Oh, maybe I misinterpreted you: When you wrote, "At that point all future mappings will be RO," you meant "all future mappings of the downgraded-to-RO handle." The earlier RW handles can still be mapped RW, correct?

#14 is correct. The writable mapping will stay writable regardless of the state of the region.

#15 is incorrect: there is no such thing as a RW *handle*; the permission are on the region itself. The region does not have per-handle permissions. When one process makes it RO, the region is RO for everyone, and only RO mappings can be created from it. Existing mappings are unchanged, though, as you said in #14.

This is very different from the POSIX file-descripter-based shared memory, which unfortunately does not exist on android AFAIU.

Re #16: I believe that even on android, if you have a file descriptor to the region, you can obtain a duped FD itself O_RDONLY from an O_RDWR by open("/proc/self/fd/$ORIGINAL_FD_NUM", O_RDONLY)

According to my understanding, the FD permissions are ignored when the region is mapped. Let me see if I can code something up to verify.

My understanding seems to be correct. If one creates a writable ashmem region, opens up the same fd from /proc as readonly, one can map that fd writable and successfully write to it. If the ashmem is protected, the mmap fails.

As a curiosity, one can open up the ashmem fd from a different process from /proc/PID/fd/XX, but it can't be mmapped.

This has been quiet for a while. What's the status of this?

Are we happy with the current RW "hack"? Is the flow described in #11 acceptable?

What's described in #14 and diagrammed in #11 is totally acceptable. Using thew new //base SharedMemory API, the viz service would create a WritableSharedMemoryRegion, Map() it, pass the object over IPC to the browser process, the browser would then Map it (read-write) as well, and then the browser converts the region to read-only for vending to renderers.

Super. I'll keep this on my list and we'll update the viz service as we proceed with the deprecation of the old api.

I have a question and then a proposal about the organization of the shared memory buffers for video capture.

It appears that the viz process (eg, components/viz/service/frame_sinks/video_capture/frame_sink_video_capture_impl.cc) maintains a list of available and utilized buffers. A utilized buffer is pushed onto the available list in OnFrameWrapperDestroyed. This list stores both the shared memory handle as well as a pointer to a region mapping.

A shared memory handle is passed to the browser in FrameSinkVideoCaptureDevice::OnFrameCaptured, which makes a writable mapping and then passes the buffer to the renderer. In terms of the discussion in #14 and #11, before passing to the renderer (receiver_->OnNewBuffer(...)), we'd downgrade the region to read-only.

Then when then frame is destroyed everyone releases it and in the viz process the shared memory buffer is moved from the utilized to the available list. The region is now read-only, but this isn't a problem in the viz process as it still has a writable mapping.

However, when this buffer is reused, the browser process will no longer be able to map a writable region. In fact, with the new shared memory API, the viz process will have to std::move the writable region into the IPC, so will no longer retain a region handle to pass to the buffer process to re-use.

I see three options here. The first is just to keep the current READ_WRITE hack. In terms of the new shared memory API, this means using UnsafeSharedMemoryRegions, which are always writable and can never be made read-only.

The second is to ditch the available and utilized lists in the viz process, and create a new shared memory region for each frame. This has obvious performance downsides (but maybe they are minor?).

The third option is much more complicated and would involve adding some sort of the browser essentially replicated the available and utilized buffer lists used in the viz process, and holding on to writable mappings of any buffer it gets, in case it's re-used in the future. This will probably add new corner cases where the browser misses out on the destruction of a frame and hangs on to mapped regions that will never be used.

WDYT?

Thanks

I think it'd be preferable to avoid uses of Unsafe if possible. If the browser intends to reuse the mapping, should it not unmap it? I'm also curious about the real-world performance benefits of the available list to see if option two is viable.

Can you provide a link to the "new shared memory API?" I'm not aware of the new design or what we're working with here. :)

If the FrameSinkVideoCapturer in the VIZ process maintained its pool of writable shmem, just like it does now; but then all other processes only had read-only shmem, would this solve the problem? Or, would we still need the READ_WRITE hack? I could be mistaken, but it almost sounds like the discussion above is saying the new shmem API doesn't have a handle Clone() anymore, just a std::move(handle)?

w.r.t. ditching the available and utilized lists: This seems expensive since every frame (could be 60 times per second) would be making the syscalls to create a new shmem region (tempdir file creation/open/mmap/delete). That said, it might be worth hacking the code to try it: Just pass the shmem handle to the browser process rather than cloning it (and, of course, never insert anything back into the "available" list); and then add some tracing code to see how performance changes. It may perform fine on some platforms and not others, though, so we'd need to be careful about that.

BTW--Running tab capture in a full browser to test "code hack ideas" is relatively simple:

0. Run local chrome build with --enable-features=VizDisplayCompositor (see note below*)
1. Visit chrome://extensions. Turn on developer mode. Click on "Load Unpacked". Select src/chrome/common/extensions/docs/examples/api/tabCapture/ in your source tree on your local machine.
2. Click on Tab Capture Example extension icon to start casting to another tab.

*Note: The same viz::FrameSinkVideoCapturer is used for tab capture with or without VizDisplayCompositor enabled. The only difference is whether it's running in the browser process or the GPU process. We'd want to test the worst-case performance scenario (passing shmem across processes), which is why I'm saying to test with the feature flag turned on.

Oh, I see there was a google.com-domain doc mentioned in an earlier comment. I can start by reading that. (But, is the Chromium one more up-to-date now?)

Yes, here is a more up-to-date public document: https://docs.google.com/document/d/1lk2-7W7Cy7FDqG-6lgS8LRkzXABYTOytf6VUsWNf8-0/edit?usp=sharing

To answer my own question in #c27, it seems that if the new shmem API provides the "CreateForReadOnlySharing(size) -> (rw_mapping, ro_region_handle)" method; then that could be made to work with the existing design/API of the FrameSinkVideoCapturer. Basically, today the browser process only needs the writable mapping because it adds "mouse cursor rendering" on the video frames. If we were to move the cursor rendering upstream, into the VIZ process, then the browser process wouldn't even need to map the shmem at all! And then, the render process could be given a RO handle.

Does this sound ideal to you guys?

From the current, public document it seems that my idea would work.

The only other detail to sort out is how mojo will invoke the new shmem APIs (it's a TODO in the document)? When I originally wrote the code, there was a huge problem on ChromeOS: Directly using base::SharedMemory to allocate shmem from within the VIZ process would fail. For some reason, using the mojo platform API to allocate the shmem worked just fine. I never was able to track down why it only worked that way.

Yup! If you can make it so the browser doesn't need to have a writable mapping, then I think that would solve this issue.

 Bug 810133  was a low-priority fixit bug meant to remove the need for the extra mapping/rendering work in the browser process. I've marked this bug as being blocked by that.

I would need to know how to use the new shmem API from the VIZ process (see "problem" mentioned in #c31) before I can schedule that work.

Yes, your comment #30 sounds ideal.

The mojo work is essentially done, see  crbug.com/826213  for details. I will update the doc, thanks for the reminder.

Since mojo understands the new base types, you should be able to just use them. See crrev.com/c/985420 for an example CL that Ken did.

I don't know about a ChromeOS problem. If it's still an issue it affects more than just you so we'll have to fix it.

I will update the docs now with details on the mojo implementation. Then we can discuss how to do the work, whether it's easiest for you to switch over to the new api as part of  crbug.com/810133 , or to break the work up between us.

I've cc'd you on crbug.com/843117 having to do with VideoCaptureBufferPool which seems to be related.

The documented is updated with details on mojo, LMK if it explains what you need.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6629d45b3851a580b72b28bc1036475e3a4516d6

commit 6629d45b3851a580b72b28bc1036475e3a4516d6
Author: Yuri Wiitala <miu@chromium.org>
Date: Tue Aug 07 21:46:16 2018

Add base::ReadOnlySharedMemoryRegion to media::mojom::VideoBufferHandle

This is the first in a series of changes to migrate the video capture
stack over to the new Chromium shmem APIs. This change adds the use of
base::ReadOnlySharedMemoryRegion as an alternative to mojo shared_buffer
handles.

Bug:  797470 ,843117
Change-Id: Ifa51a4984f48376a5da4b437618d301669f1ae1e
Reviewed-on: https://chromium-review.googlesource.com/1164637
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581347}
[modify] https://crrev.com/6629d45b3851a580b72b28bc1036475e3a4516d6/content/browser/renderer_host/media/video_capture_controller.cc
[modify] https://crrev.com/6629d45b3851a580b72b28bc1036475e3a4516d6/content/renderer/media/video_capture_impl.cc
[modify] https://crrev.com/6629d45b3851a580b72b28bc1036475e3a4516d6/content/renderer/media/video_capture_impl_unittest.cc
[modify] https://crrev.com/6629d45b3851a580b72b28bc1036475e3a4516d6/media/capture/mojom/video_capture_types.mojom

Ok, I completed my plan in comment #30! :) So, to sum up things now, we have:

VIZ           |  browser              |  renderer
 RW mapping   |                       |
 (in-process) |                       |
              |                       |
 RO handle----+-----------------------+--->RO handle + mapping
              | (no mapping anymore!) |

However, the original issue causing me to cut this bug is still present. It is blocking me from being able to use the new base::ReadOnlySharedMemoryRegion APIs in the VIZ mojo service. In fact, it would block any other mojo services outside of the browser process from being able to use the new APIs.

Sorry to write so much, but I've dug a bit deep into this already and would like to share my findings:

For context, here's the CL all ready to go, but failing viz_browser_tests on Linux and CrOS: https://chromium-review.googlesource.com/c/chromium/src/+/1166248

You can see, from the try bot error logs, that the viz_browser_tests is failing because viz::InterprocessFramePool fails when calling base::ReadOnlySharedMemoryRegion::Create(). The following errors are relevant:

  [...:ERROR:platform_shared_memory_region_posix.cc(244)] open("/dev/shm/.org.chromium.Chromium.Pz45Kk", O_RDONLY) failed: Operation not permitted (1)
  [...:WARNING:platform_shared_memory_region_posix.cc(26)] unlink: Operation not permitted (1)

When looking into platform_shared_memory_region_posix.cc code, what's happening is:

Step 1: Call mkstemp(), which the sandbox broker handles by creating a temp file in /dev/shm/, unlinking it immediately, and returning the fd and the file path.

Step 2: Call open("/dev/shm/.org.chromium.Chromium.Pz45Kk", O_RDONLY) so that there is a read-only fd that can be passed to other processes. The sandbox blocks this call with "Operation not permitted."

Step 3: Because Step 2 failed, platform_shared_memory_region_posix.cc fails-out. It has its own call to unlink() that also fails.

Now, it seemed like I just needed to "poke another hole" in the sandbox to get things working. However, if I go and add the following line to content/gpu/gpu_sandbox_hook_linux.cc:

  // In: void AddStandardGpuWhiteList()
  permissions->push_back(BrokerFilePermission::ReadOnlyRecursive(kDevShm));

...it's still a no-go. The error message changes to:

  [...:ERROR:platform_shared_memory_region_posix.cc(250)] open("/dev/shm/.org.chromium.Chromium.om8GiS", O_RDONLY) failed: No such file or directory (2)

And, this is because the sandbox broker is unlinking the temp file *itself* before anything ever sees the fd. Now, if I also remove the unlink() call from OpenFileForIPC() in sandbox/linux/.../broker_host.cc, it works!

At this point, I need you folks to help decide: Is it safe to make these changes to the sandboxing code? Should I just do it in my current CL? Is my approach a no-go? How should we proceed?

If you'd like to play around with this, a quick way to test things is:

1. Start with a tip-of-trunk Chromium checkout on a Linux machine.
2. Patch in my CL (see above), so that you're using the new shmem APIs.
3. ninja -C out/Foo content_browsertests
4. out/Foo/content_browsertests --gtest_filter='*WebContentsVideoCapture*/0' --enable-features=VizDisplayCompositor

Note: The --enable-features=VizDisplayCompositor flag is VERY IMPORTANT, as it is necessary for the VIZ service to run in its own process (instead of the browser process) to repro the problem.

+rockot

Ok, I see what's going on. You need to use the mojo::SharedBufferHandle because you're in a sandboxed process, the SharedBufferHandle behind the scenes knows to create it using a broker process.

What needs to happen is that the broker process creates a WritableSharedMemory region which is then converted to RO in the viz process. The snippet to do it is below. Note Map() is called on the writable region, this produces a writable mapping which remains writable after the region is converted to readonly.

However, I think this is a little ridiculous of an incantation for users of shared memory regions to know, so I'm exploring if adding something like ReadOnlySharedMemoryRegion::CreateInSandbox() would be better (that method would just wrap up the following code.

  mojo::ScopedSharedBufferHandle handle = 
     mojo::SharedBufferHandle::Create(size);
  if (!handle.is_valid()) {
    DLOG(ERROR) << ....
    return;
  }

  base::WritableSharedMemoryRegion region =
          mojo::UnwrapWritableSharedMemoryRegion(std::move(handle));
  if (!region.IsValid()) {
    DLOG(ERROR) << ....
    return;
  }

  base::WritableSharedMemoryMapping mapping = region.Map();
  base::ReadOnlySharedMemoryRegion ro_region = region.ConvertToReadOnly();

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5

commit 4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5
Author: Yuri Wiitala <miu@chromium.org>
Date: Wed Aug 29 06:09:35 2018

Migrate viz::FrameSinkVideoCapturer to use new read-only shmem API.

Now that overlay rendering takes place in VIZ, there is no need to pass
read-write handles across processes. This change migrates to the new
base::ReadOnlySharedMemoryRegion API. It is also a prerequisite to
migrating the rest of the video capture stack to the new read-only API.

Bug:  797470 ,843117
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I87e16de42aa00e1fa0edba55a9cbf013fc345440
Reviewed-on: https://chromium-review.googlesource.com/1166248
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Reviewed-by: Xiangjun Zhang <xjz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587022}
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/chrome/browser/devtools/devtools_eye_dropper.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/chrome/browser/devtools/devtools_eye_dropper.h
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/components/mirroring/service/fake_video_capture_host.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/components/mirroring/service/video_capture_client.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/components/mirroring/service/video_capture_client.h
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/components/mirroring/service/video_capture_client_unittest.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/components/viz/host/client_frame_sink_video_capturer.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/components/viz/host/client_frame_sink_video_capturer.h
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/components/viz/service/frame_sinks/video_capture/frame_sink_video_capturer_impl.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/components/viz/service/frame_sinks/video_capture/frame_sink_video_capturer_impl_unittest.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/components/viz/service/frame_sinks/video_capture/interprocess_frame_pool.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/components/viz/service/frame_sinks/video_capture/interprocess_frame_pool.h
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/components/viz/service/frame_sinks/video_capture/interprocess_frame_pool_unittest.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/content/browser/devtools/devtools_video_consumer.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/content/browser/devtools/devtools_video_consumer.h
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/content/browser/devtools/devtools_video_consumer_unittest.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/content/browser/media/capture/fake_video_capture_stack.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/content/browser/media/capture/frame_sink_video_capture_device.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/content/browser/media/capture/frame_sink_video_capture_device.h
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/content/browser/media/capture/frame_sink_video_capture_device_unittest.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/content/browser/media/capture/lame_window_capturer_chromeos.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/content/browser/media/capture/lame_window_capturer_chromeos.h
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/services/viz/privileged/interfaces/compositing/frame_sink_video_capture.mojom

Marking as resolved. Summary: Those wishing to demote writable shared memory to read-only should be creating the shmem via the new base::ReadOnlySharedMemoryRegion APIs. If not in the browser process, the mojo::CreateReadOnlySharedMemoryRegion() utility should be used instead (see mojo/public/cpp/base/shared_memory_utils.h).