New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 793503 link

Starred by 1 user
Status: Fixed
Owner:
rsesek@chromium.org
Closed: Dec 2017
Cc:
rsesek@chromium.org
█ roc...@chromium.org
dcheng@chromium.org
palmer@chromium.org
ios-bugs@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Chrome , Mac , Fuchsia
Pri: 2
Type: Bug



Sign in to add a comment

Remove or change SharedBufferHandle::Clone's default argument

Project Member Reported by dcheng@chromium.org, Dec 8 2017 
It apparently defaults to READ_WRITE, but for things that often need to across IPC, we should default to the lowest permission by default.

I would be OK with either changing the default to READ_ONLY, or forcing everyone to write it explicitly.

Comment 1 by rsesek@chromium.org, Dec 9 2017 

I don't think this should accept a default argument. Callers should be explicit with their intent.

Comment 2 by rsesek@chromium.org, Dec 11 2017

Cc: roc...@chromium.org
Owner: rsesek@chromium.org
Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 12 2017 

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

commit c9e5af619bf3bd0e3e56b868945f53b7dad2db49
Author: Robert Sesek <rsesek@chromium.org>
Date: Tue Dec 12 23:27:37 2017

Remove default argument from mojo::SharedBufferHandle::Clone().

This also changes wherever possible from the old implicit value of
READ_WRITE to READ_ONLY.

Bug:  793503 ,  793519 
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
Change-Id: I0ebafb2eb93af9718e71a303e239ecaa4e7d8f35
Reviewed-on: https://chromium-review.googlesource.com/820011
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523600}
[modify] https://crrev.com/c9e5af619bf3bd0e3e56b868945f53b7dad2db49/components/visitedlink/test/visitedlink_unittest.cc
[modify] https://crrev.com/c9e5af619bf3bd0e3e56b868945f53b7dad2db49/content/browser/renderer_host/media/video_capture_controller.cc
[modify] https://crrev.com/c9e5af619bf3bd0e3e56b868945f53b7dad2db49/content/common/sandbox_mac_fontloading_unittest.mm
[modify] https://crrev.com/c9e5af619bf3bd0e3e56b868945f53b7dad2db49/content/renderer/device_sensors/device_motion_event_pump_unittest.cc
[modify] https://crrev.com/c9e5af619bf3bd0e3e56b868945f53b7dad2db49/content/renderer/device_sensors/device_orientation_event_pump_unittest.cc
[modify] https://crrev.com/c9e5af619bf3bd0e3e56b868945f53b7dad2db49/device/sensors/data_fetcher_shared_memory_base.cc
[modify] https://crrev.com/c9e5af619bf3bd0e3e56b868945f53b7dad2db49/gpu/command_buffer/common/activity_flags.h
[modify] https://crrev.com/c9e5af619bf3bd0e3e56b868945f53b7dad2db49/media/mojo/interfaces/video_frame_struct_traits.cc
[modify] https://crrev.com/c9e5af619bf3bd0e3e56b868945f53b7dad2db49/mojo/public/cpp/system/buffer.h

Comment 4 by rsesek@chromium.org, Dec 13 2017

Status: Fixed (was: Started)
►

Sign in to add a comment