Chromium Code Reviews
Help | Chromium Project | Sign in
(24)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 week, 5 days ago by mmenke
Modified:
1 week, 1 day 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 ...
1 week, 4 days 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 week, 4 days 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 week, 4 days ago (2017-04-18 20:04:57 UTC) #25
tommi - chröme
Guido - can you take a look?
1 week, 3 days ago (2017-04-19 10:45:40 UTC) #27
Guido Urdaneta
lgtm
1 week, 3 days 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 week, 3 days 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 week, 3 days 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 week, 3 days 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 week, 2 days 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 week, 2 days 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 week, 2 days 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 week, 2 days 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 week, 2 days 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 week, 2 days 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 week, 2 days ago (2017-04-20 16:07:48 UTC) #40
Zachary Kuznia
lgtm
1 week, 2 days ago (2017-04-20 16:10:54 UTC) #41
Mike Lerman
lgtm, thanks for filing the bug
1 week, 2 days ago (2017-04-20 16:53:27 UTC) #42
jochen (slow until May 2)
lgtm
1 week, 1 day 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 week, 1 day 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 week, 1 day 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 week, 1 day ago (2017-04-21 15:27:10 UTC) #51
commit-bot: I haz the power
1 week, 1 day 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 cc6ac46