Store Media gallery permissions in extension prefs.

Store Media gallery permissions in extension prefs.

BUG=NONE


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

[Evan Stade, 2012-07-31 20:29:47]

http://codereview.chromium.org/10824116/diff/1/chrome/browser/extensions/exte...
File chrome/browser/extensions/extension_prefs.h (right):

http://codereview.chromium.org/10824116/diff/1/chrome/browser/extensions/exte...
chrome/browser/extensions/extension_prefs.h:328: MediaGalleryPrefId gallery,
bool has_access);
The object should be a const&, and I think it makes sense to pass a
MediaGalleryPermission struct (although only the id would be used).

http://codereview.chromium.org/10824116/diff/1/chrome/browser/extensions/exte...
chrome/browser/extensions/extension_prefs.h:330: const std::string&
extension_id);
nit: too much indent

http://codereview.chromium.org/10824116/diff/1/chrome/browser/media_gallery/m...
File chrome/browser/media_gallery/media_galleries_preferences.h (right):

http://codereview.chromium.org/10824116/diff/1/chrome/browser/media_gallery/m...
chrome/browser/media_gallery/media_galleries_preferences.h:26:
MediaGalleryPrefId pref_id;
nit: just 2 spaces of indent.

http://codereview.chromium.org/10824116/diff/1/chrome/browser/media_gallery/m...
chrome/browser/media_gallery/media_galleries_preferences.h:26:
MediaGalleryPrefId pref_id;
I think this would be more useful as a full-on MediaGallery.

[vandebo (ex-Chrome), 2012-08-01 01:01:10]

https://chromiumcodereview.appspot.com/10824116/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/extension_prefs.h (right):

https://chromiumcodereview.appspot.com/10824116/diff/1/chrome/browser/extensi...
chrome/browser/extensions/extension_prefs.h:328: MediaGalleryPrefId gallery,
bool has_access);
On 2012/07/31 20:29:47, Evan Stade wrote:
> The object should be a const&, and I think it makes sense to pass a
> MediaGalleryPermission struct (although only the id would be used).

The extension prefs don't need to know any of the gallery metadata, I think
passing just the id is the right abstraction here.

https://chromiumcodereview.appspot.com/10824116/diff/1/chrome/browser/media_g...
File chrome/browser/media_gallery/media_galleries_preferences.h (right):

https://chromiumcodereview.appspot.com/10824116/diff/1/chrome/browser/media_g...
chrome/browser/media_gallery/media_galleries_preferences.h:26:
MediaGalleryPrefId pref_id;
On 2012/07/31 20:29:47, Evan Stade wrote:
> I think this would be more useful as a full-on MediaGallery.

This is used to return the permissions from the extension prefs.  If we put a
full on MediaGallery here, then the extensions prefs need to query the media
gallery prefs to lookup the full MediaGallery.  I think returning just the id is
the right way to go here.

[Evan Stade, 2012-08-01 04:29:37]

http://codereview.chromium.org/10824116/diff/1/chrome/browser/extensions/exte...
File chrome/browser/extensions/extension_prefs.h (right):

http://codereview.chromium.org/10824116/diff/1/chrome/browser/extensions/exte...
chrome/browser/extensions/extension_prefs.h:328: MediaGalleryPrefId gallery,
bool has_access);
On 2012/08/01 01:01:10, vandebo wrote:
> On 2012/07/31 20:29:47, Evan Stade wrote:
> > The object should be a const&, and I think it makes sense to pass a
> > MediaGalleryPermission struct (although only the id would be used).
> 
> The extension prefs don't need to know any of the gallery metadata, I think
> passing just the id is the right abstraction here.

yea, you're right. I failed to look at what file this was.

[vandebo (ex-Chrome), 2012-08-01 18:28:25]

Do you want to do a full review and we can get this part in?

http://codereview.chromium.org/10824116/diff/1/chrome/browser/extensions/exte...
File chrome/browser/extensions/extension_prefs.h (right):

http://codereview.chromium.org/10824116/diff/1/chrome/browser/extensions/exte...
chrome/browser/extensions/extension_prefs.h:330: const std::string&
extension_id);
On 2012/07/31 20:29:47, Evan Stade wrote:
> nit: too much indent

Oops, need to fix my home vim settings.  Fixed.

[vandebo (ex-Chrome), 2012-08-01 23:33:49]

PTAL

[Evan Stade, 2012-08-02 00:53:57]

lgtm

http://codereview.chromium.org/10824116/diff/6/chrome/browser/extensions/exte...
File chrome/browser/extensions/extension_prefs.cc (right):

http://codereview.chromium.org/10824116/diff/6/chrome/browser/extensions/exte...
chrome/browser/extensions/extension_prefs.cc:1171:
&out_permission->has_permission)) {
indent to be flush with kMediaGallery

http://codereview.chromium.org/10824116/diff/6/chrome/browser/extensions/exte...
chrome/browser/extensions/extension_prefs.cc:1178: void
RemoveMediaGalleryPermissionsFromExtension(PrefService * prefs,
no space before *

http://codereview.chromium.org/10824116/diff/6/chrome/browser/extensions/exte...
chrome/browser/extensions/extension_prefs.cc:1189: it++) {
++it here and elsewhere

[asargent_no_longer_on_chrome, 2012-08-02 16:04:56]

lgtm

[commit-bot: I haz the power, 2012-08-02 17:24:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vandebo@chromium.org/10824116/9001

[commit-bot: I haz the power, 2012-08-02 18:42:54]

Change committed as 149658