Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(346)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 12208079: Move TransientDeviceId from MediFileSystemRegistry to RemovableStorageNotifications. (Closed)

Created:
7 years, 10 months ago by vandebo (ex-Chrome)
Modified:
7 years, 10 months ago
Reviewers:
Lei Zhang, Greg Billock
CC:
chromium-reviews, Aaron Boodman, chromium-apps-reviews_chromium.org, Greg Billock
Visibility:
Public.

Description

Move TransientDeviceId from MediFileSystemRegistry to RemovableStorageNotifications. BUG=NONE Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=182086

Patch Set 1 #

Patch Set 2 : nit #

Total comments: 2

Patch Set 3 : Abstract a bit better #

Total comments: 4

Patch Set 4 : Address comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+52 lines, -127 lines) Patch
M chrome/browser/extensions/api/media_galleries_private/media_galleries_private_event_router.cc View 1 2 3 2 chunks +4 lines, -5 lines 0 comments Download
M chrome/browser/media_gallery/media_file_system_registry.h View 2 chunks +0 lines, -3 lines 0 comments Download
M chrome/browser/media_gallery/media_file_system_registry.cc View 1 2 2 chunks +3 lines, -8 lines 0 comments Download
D chrome/browser/media_gallery/transient_device_ids.h View 1 chunk +0 lines, -45 lines 0 comments Download
D chrome/browser/media_gallery/transient_device_ids.cc View 1 chunk +0 lines, -32 lines 0 comments Download
M chrome/browser/system_monitor/removable_storage_notifications.h View 1 2 3 chunks +5 lines, -0 lines 0 comments Download
M chrome/browser/system_monitor/removable_storage_notifications.cc View 1 2 3 chunks +34 lines, -28 lines 0 comments Download
A + chrome/browser/system_monitor/transient_device_ids.h View 2 chunks +3 lines, -3 lines 0 comments Download
A + chrome/browser/system_monitor/transient_device_ids.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/chrome_browser.gypi View 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
vandebo (ex-Chrome)
7 years, 10 months ago (2013-02-08 01:51:43 UTC) #1
Greg Billock
https://codereview.chromium.org/12208079/diff/2001/chrome/browser/system_monitor/removable_storage_notifications.cc File chrome/browser/system_monitor/removable_storage_notifications.cc (right): https://codereview.chromium.org/12208079/diff/2001/chrome/browser/system_monitor/removable_storage_notifications.cc#newcode64 chrome/browser/system_monitor/removable_storage_notifications.cc:64: return g_removable_storage_notifications; Why move these above the constructor?
7 years, 10 months ago (2013-02-08 17:54:53 UTC) #2
vandebo (ex-Chrome)
Adding a bit more abstraction as discussed with Lei. https://codereview.chromium.org/12208079/diff/2001/chrome/browser/system_monitor/removable_storage_notifications.cc File chrome/browser/system_monitor/removable_storage_notifications.cc (right): https://codereview.chromium.org/12208079/diff/2001/chrome/browser/system_monitor/removable_storage_notifications.cc#newcode64 chrome/browser/system_monitor/removable_storage_notifications.cc:64: ...
7 years, 10 months ago (2013-02-09 00:49:16 UTC) #3
Lei Zhang
https://chromiumcodereview.appspot.com/12208079/diff/7001/chrome/browser/extensions/api/media_galleries_private/media_galleries_private_event_router.cc File chrome/browser/extensions/api/media_galleries_private/media_galleries_private_event_router.cc (right): https://chromiumcodereview.appspot.com/12208079/diff/7001/chrome/browser/extensions/api/media_galleries_private/media_galleries_private_event_router.cc#newcode14 chrome/browser/extensions/api/media_galleries_private/media_galleries_private_event_router.cc:14: #include "chrome/browser/browser_process.h" You can get rid of this too. ...
7 years, 10 months ago (2013-02-12 16:48:01 UTC) #4
vandebo (ex-Chrome)
https://chromiumcodereview.appspot.com/12208079/diff/7001/chrome/browser/extensions/api/media_galleries_private/media_galleries_private_event_router.cc File chrome/browser/extensions/api/media_galleries_private/media_galleries_private_event_router.cc (right): https://chromiumcodereview.appspot.com/12208079/diff/7001/chrome/browser/extensions/api/media_galleries_private/media_galleries_private_event_router.cc#newcode14 chrome/browser/extensions/api/media_galleries_private/media_galleries_private_event_router.cc:14: #include "chrome/browser/browser_process.h" On 2013/02/12 16:48:01, Lei Zhang wrote: > ...
7 years, 10 months ago (2013-02-12 21:40:57 UTC) #5
Lei Zhang
lgtm
7 years, 10 months ago (2013-02-12 21:49:39 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vandebo@chromium.org/12208079/7002
7 years, 10 months ago (2013-02-12 21:56:33 UTC) #7
commit-bot: I haz the power
7 years, 10 months ago (2013-02-13 02:02:38 UTC) #8
Message was sent while issue was closed. 
Change committed as 182086

Powered by Google App Engine
This is Rietveld 408576698