Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(3)

Issue 2820163002: Move MediaDeviceIDSalt from ProfileIOData to ProfileImpl. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 months, 1 week ago by mmenke
Modified:
4 months ago
CC:
chromium-reviews
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Move MediaDeviceIDSalt from ProfileIOData to ProfileImpl. The object is never used on the IO thread, and even had DCHECKs to ensure that's the case, so it makes no sense in ProfileIOData. It's also one of the few things exposed by ResourceContext, which we're probably going to get rid of. BUG=712296 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation Review-Url: https://codereview.chromium.org/2820163002 Cr-Commit-Position: refs/heads/master@{#466358} Committed: https://chromium.googlesource.com/chromium/src/+/c0b2b8b1b919b0fcda51e691406d50a62f89771d

Patch Set 1 #

Patch Set 2 : Fix some stuff #

Patch Set 3 : Reorder #

Patch Set 4 : Fix #

Total comments: 3

Patch Set 5 : Add TODO #

Patch Set 6 : Reflow comment #

Patch Set 7 : merge #

Unified diffs Side-by-side diffs Delta from patch set Stats (+84 lines, -130 lines) Patch
M chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_browsertest.cc View 1 2 3 4 5 6 2 chunks +2 lines, -3 lines 0 comments Download
M chrome/browser/media/media_device_id_salt.h View 1 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/media/media_device_id_salt.cc View 1 3 chunks +3 lines, -8 lines 0 comments Download
M chrome/browser/profiles/profile_impl.h View 1 2 3 4 5 3 chunks +7 lines, -0 lines 0 comments Download
M chrome/browser/profiles/profile_impl.cc View 1 2 3 4 5 6 3 chunks +7 lines, -0 lines 0 comments Download
M chrome/browser/profiles/profile_io_data.h View 1 4 chunks +0 lines, -10 lines 0 comments Download
M chrome/browser/profiles/profile_io_data.cc View 1 2 3 4 5 6 5 chunks +0 lines, -18 lines 0 comments Download
M content/browser/browser_context.cc View 1 3 chunks +17 lines, -0 lines 0 comments Download
M content/browser/frame_host/render_frame_host_impl.cc View 1 2 3 4 5 6 2 chunks +0 lines, -2 lines 0 comments Download
M content/browser/renderer_host/media/media_devices_dispatcher_host.cc View 1 2 chunks +2 lines, -2 lines 0 comments Download
M content/browser/renderer_host/media/media_devices_dispatcher_host_unittest.cc View 1 4 chunks +7 lines, -11 lines 0 comments Download
M content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc View 1 2 3 4 5 6 6 chunks +9 lines, -19 lines 0 comments Download
M content/browser/renderer_host/media/video_capture_unittest.cc View 1 3 chunks +6 lines, -8 lines 0 comments Download
M content/browser/renderer_host/render_process_host_impl.cc View 1 2 3 4 5 6 3 chunks +4 lines, -6 lines 0 comments Download
M content/browser/resource_context_impl.cc View 1 2 chunks +1 line, -17 lines 0 comments Download
M content/public/browser/browser_context.h View 1 3 chunks +15 lines, -0 lines 0 comments Download
M content/public/browser/resource_context.h View 1 3 chunks +1 line, -19 lines 0 comments Download
M extensions/browser/api/webcam_private/webcam_private_api_chromeos.cc View 1 2 3 2 chunks +2 lines, -5 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 54 (32 generated)
mmenke
[tommi]: Please review everything. [mlerman]: Please review profiles/profile_impl.* (I own the directory, but only for ...
4 months ago (2017-04-18 18:52:16 UTC) #23
Mike Lerman
https://codereview.chromium.org/2820163002/diff/70001/chrome/browser/profiles/profile_impl.h File chrome/browser/profiles/profile_impl.h (right): https://codereview.chromium.org/2820163002/diff/70001/chrome/browser/profiles/profile_impl.h#newcode258 chrome/browser/profiles/profile_impl.h:258: // STOP!!!! DO NOT ADD ANY MORE ITEMS HERE!!!! ...
4 months ago (2017-04-18 19:09:46 UTC) #24
mmenke
https://codereview.chromium.org/2820163002/diff/70001/chrome/browser/profiles/profile_impl.h File chrome/browser/profiles/profile_impl.h (right): https://codereview.chromium.org/2820163002/diff/70001/chrome/browser/profiles/profile_impl.h#newcode258 chrome/browser/profiles/profile_impl.h:258: // STOP!!!! DO NOT ADD ANY MORE ITEMS HERE!!!! ...
4 months ago (2017-04-18 20:04:57 UTC) #25
tommi (sloooow) - chröme
Guido - can you take a look?
4 months ago (2017-04-19 10:45:40 UTC) #27
Guido Urdaneta
lgtm
4 months ago (2017-04-19 10:55:41 UTC) #28
Mike Lerman
Given neither of us is familiar with the intricacies of this change, are we confident ...
4 months ago (2017-04-19 14:50:16 UTC) #29
mmenke
On 2017/04/19 14:50:16, Mike Lerman wrote: > Given neither of us is familiar with the ...
4 months ago (2017-04-19 15:07:58 UTC) #30
Guido Urdaneta
I did not author this code, but have worked on it before (I changed the ...
4 months ago (2017-04-19 15:15:00 UTC) #31
Mike Lerman
WDYT about creating a bug (tagged against both profiles and media components) and leaving a ...
4 months ago (2017-04-20 13:30:40 UTC) #32
Guido Urdaneta
On 2017/04/20 13:30:40, Mike Lerman wrote: > WDYT about creating a bug (tagged against both ...
4 months ago (2017-04-20 14:14:54 UTC) #33
mmenke
On 2017/04/20 14:14:54, Guido Urdaneta wrote: > On 2017/04/20 13:30:40, Mike Lerman wrote: > > ...
4 months ago (2017-04-20 15:11:24 UTC) #34
mmenke
[+jochen]: Please review content/browser (Moving something from ResourceContext to BrowserContext - ResourceContext is going away, ...
4 months ago (2017-04-20 15:13:50 UTC) #36
Devlin
extensions lgtm Description Nit: The first line of the CL description field in Rietveld is ...
4 months ago (2017-04-20 15:39:15 UTC) #37
mmenke
On 2017/04/20 15:39:15, Devlin wrote: > extensions lgtm > > Description Nit: > > The ...
4 months ago (2017-04-20 15:42:59 UTC) #39
Devlin
On 2017/04/20 15:42:59, mmenke wrote: > On 2017/04/20 15:39:15, Devlin wrote: > > extensions lgtm ...
4 months ago (2017-04-20 16:07:48 UTC) #40
Zachary Kuznia
lgtm
4 months ago (2017-04-20 16:10:54 UTC) #41
Mike Lerman
lgtm, thanks for filing the bug
4 months ago (2017-04-20 16:53:27 UTC) #42
jochen (gone - plz use gerrit)
lgtm
4 months ago (2017-04-21 08:38:45 UTC) #43
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2820163002/110001
4 months ago (2017-04-21 11:42:51 UTC) #46
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/417451) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, ...
4 months ago (2017-04-21 11:45:44 UTC) #48
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2820163002/130001
4 months ago (2017-04-21 15:27:10 UTC) #51
commit-bot: I haz the power
4 months ago (2017-04-21 16:28:12 UTC) #54
Message was sent while issue was closed.
Committed patchset #7 (id:130001) as
https://chromium.googlesource.com/chromium/src/+/c0b2b8b1b919b0fcda51e691406d...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b