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

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

Created:
1 year ago by mmenke
Modified:
1 year 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

Messages

Total messages: 54 (32 generated)
mmenke
[tommi]: Please review everything. [mlerman]: Please review profiles/profile_impl.* (I own the directory, but only for ...
1 year 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!!!! ...
1 year 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!!!! ...
1 year ago (2017-04-18 20:04:57 UTC) #25
tommi (sloooow) - chröme
Guido - can you take a look?
1 year ago (2017-04-19 10:45:40 UTC) #27
Guido Urdaneta
lgtm
1 year 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 ...
1 year 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 ...
1 year 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 ...
1 year 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 ...
1 year 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 ...
1 year 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: > > ...
1 year 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, ...
1 year 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 ...
1 year ago (2017-04-20 15:39:15 UTC) #37
mmenke
On 2017/04/20 15:39:15, Devlin wrote: > extensions lgtm > > Description Nit: > > The ...
1 year 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 ...
1 year ago (2017-04-20 16:07:48 UTC) #40
Zachary Kuznia
lgtm
1 year ago (2017-04-20 16:10:54 UTC) #41
Mike Lerman
lgtm, thanks for filing the bug
1 year ago (2017-04-20 16:53:27 UTC) #42
jochen (gone - plz use gerrit)
lgtm
1 year 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
1 year 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, ...
1 year 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
1 year ago (2017-04-21 15:27:10 UTC) #51
commit-bot: I haz the power
1 year 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...

Powered by Google App Engine
This is Rietveld 408576698