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

Issue 11304022: [Media Gallery] Add existing attached devices to the prefs in time for media galleries dialog. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 5 months ago by kmadhusu
Modified:
1 year, 5 months ago
Reviewers:
vandebo, csilv
CC:
chromium-reviews_chromium.org, Aaron Boodman, chromium-apps-reviews_chromium.org
Visibility:
Public.

Description

[Media Gallery] Add existing attached devices to the prefs in time for media galleries dialog.

BUG=149154
TEST=none


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=164856

Patch Set 1 : '' #

Total comments: 4

Patch Set 2 : '' #

Patch Set 3 : '' #

Total comments: 4

Patch Set 4 : Addressed review comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+41 lines, -37 lines) Lint Patch
M chrome/browser/media_gallery/media_file_system_registry.h View 1 2 3 2 chunks +7 lines, -4 lines 0 comments ? errors Download
M chrome/browser/media_gallery/media_file_system_registry.cc View 1 2 3 5 chunks +26 lines, -27 lines 0 comments ? errors Download
M chrome/browser/media_gallery/media_galleries_dialog_controller.cc View 1 2 3 2 chunks +2 lines, -2 lines 0 comments ? errors Download
M chrome/browser/media_gallery/media_galleries_preferences_factory.h View 1 2 3 1 chunk +2 lines, -0 lines 0 comments ? errors Download
M chrome/browser/ui/webui/options/media_galleries_handler.cc View 1 2 3 4 chunks +4 lines, -4 lines 0 comments ? errors Download
Commit:

Messages

Total messages: 16
kmadhusu
1 year, 5 months ago #1
vandebo
http://codereview.chromium.org/11304022/diff/2001/chrome/browser/extensions/api/media_galleries/media_galleries_api.cc File chrome/browser/extensions/api/media_galleries/media_galleries_api.cc (right): http://codereview.chromium.org/11304022/diff/2001/chrome/browser/extensions/api/media_galleries/media_galleries_api.cc#newcode73 chrome/browser/extensions/api/media_galleries/media_galleries_api.cc:73: MediaFileSystemRegistry::GetInstance()->AddAttachedMediaDeviceGalleries( I think putting this here abuses it's use. ...
1 year, 5 months ago #2
kmadhusu
Addressed review comments. PTAL. Thanks. http://codereview.chromium.org/11304022/diff/2001/chrome/browser/extensions/api/media_galleries/media_galleries_api.cc File chrome/browser/extensions/api/media_galleries/media_galleries_api.cc (right): http://codereview.chromium.org/11304022/diff/2001/chrome/browser/extensions/api/media_galleries/media_galleries_api.cc#newcode73 chrome/browser/extensions/api/media_galleries/media_galleries_api.cc:73: MediaFileSystemRegistry::GetInstance()->AddAttachedMediaDeviceGalleries( On 2012/10/27 19:57:57, ...
1 year, 5 months ago #3
vandebo
On 2012/10/28 01:13:43, kmadhusu wrote: > Addressed review comments. PTAL. > > Thanks. > > ...
1 year, 5 months ago #4
kmadhusu
On 2012/10/28 02:49:27, vandebo wrote: > On 2012/10/28 01:13:43, kmadhusu wrote: > > Addressed review ...
1 year, 5 months ago #5
kmadhusu
Modified code to get MediaGalleriesPreferences from MediaFileSystemRegistry. PTAL. Thanks.
1 year, 5 months ago #6
vandebo
LGTM with comments. http://codereview.chromium.org/11304022/diff/13002/chrome/browser/media_gallery/media_file_system_registry.h File chrome/browser/media_gallery/media_file_system_registry.h (right): http://codereview.chromium.org/11304022/diff/13002/chrome/browser/media_gallery/media_file_system_registry.h#newcode94 chrome/browser/media_gallery/media_file_system_registry.h:94: // |profile|. Registers all the media ...
1 year, 5 months ago #7
kmadhusu
Addressed review comments. Thanks. http://codereview.chromium.org/11304022/diff/13002/chrome/browser/media_gallery/media_file_system_registry.h File chrome/browser/media_gallery/media_file_system_registry.h (right): http://codereview.chromium.org/11304022/diff/13002/chrome/browser/media_gallery/media_file_system_registry.h#newcode94 chrome/browser/media_gallery/media_file_system_registry.h:94: // |profile|. Registers all the ...
1 year, 5 months ago #8
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kmadhusu@chromium.org/11304022/15004
1 year, 5 months ago #9
I haz the power (commit-bot)
Presubmit check for 11304022-15004 failed and returned exit status 1. Running presubmit commit checks ...
1 year, 5 months ago #10
kmadhusu
csilv@: Can you do the OWNER's check for ui/webui/options file change? Thanks.
1 year, 5 months ago #11
csilv
ui/webui/options LGTM
1 year, 5 months ago #12
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kmadhusu@chromium.org/11304022/15004
1 year, 5 months ago #13
I haz the power (commit-bot)
Retried try job too often for step(s) browser_tests
1 year, 5 months ago #14
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kmadhusu@chromium.org/11304022/15004
1 year, 5 months ago #15
I haz the power (commit-bot)
1 year, 5 months ago #16
Change committed as 164856
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6