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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 months, 1 week ago by mmenke
Modified:
2 months, 1 week 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 ...
2 months, 1 week 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!!!! ...
2 months, 1 week 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!!!! ...
2 months, 1 week ago (2017-04-18 20:04:57 UTC) #25
tommi (sloooow) - chröme
Guido - can you take a look?
2 months, 1 week ago (2017-04-19 10:45:40 UTC) #27
Guido Urdaneta
lgtm
2 months, 1 week 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 ...
2 months, 1 week 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 ...
2 months, 1 week 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 ...
2 months, 1 week 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 ...
2 months, 1 week 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 ...
2 months, 1 week 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: > > ...
2 months, 1 week 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, ...
2 months, 1 week 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 ...
2 months, 1 week ago (2017-04-20 15:39:15 UTC) #37
mmenke
On 2017/04/20 15:39:15, Devlin wrote: > extensions lgtm > > Description Nit: > > The ...
2 months, 1 week 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 ...
2 months, 1 week ago (2017-04-20 16:07:48 UTC) #40
Zachary Kuznia
lgtm
2 months, 1 week ago (2017-04-20 16:10:54 UTC) #41
Mike Lerman
lgtm, thanks for filing the bug
2 months, 1 week ago (2017-04-20 16:53:27 UTC) #42
jochen
lgtm
2 months, 1 week 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
2 months, 1 week 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, ...
2 months, 1 week 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
2 months, 1 week ago (2017-04-21 15:27:10 UTC) #51
commit-bot: I haz the power
2 months, 1 week 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 cb946e318