Issue 12211084: [Media Galleries] Populate volume metadata in ChromeOS/Linux

Greg Billock

7 years, 10 months ago (2013-02-08 01:28:59 UTC) #1

vandebo (ex-Chrome)

https://codereview.chromium.org/12211084/diff/1/chrome/browser/system_monitor/removable_storage_notifications.h File chrome/browser/system_monitor/removable_storage_notifications.h (right): https://codereview.chromium.org/12211084/diff/1/chrome/browser/system_monitor/removable_storage_notifications.h#newcode29 chrome/browser/system_monitor/removable_storage_notifications.h:29: StorageInfo(const std::string& id, This constructor implies that sometimes the ...

7 years, 10 months ago (2013-02-08 02:03:31 UTC) #2

Greg Billock

Thanks, Steve. I think the main issue to figure out is approach -- should I ...

7 years, 10 months ago (2013-02-08 16:42:02 UTC) #3

Thanks, Steve. I think the main issue to figure out is approach -- should I try
to add these fields and then modify clients? Or make the changeover more
far-reaching? I'm a bit cautious to fiddle with the name directly, as I don't
have a good sense of where it goes into the registry and permissions system yet.
So my plan was to leave it alone, and just switch clients to building
user-visible names from the parts list. Ultimately, though, we need to store the
user-visible name in prefs. If there are platform-specific name features (using
"D:\ Label" on windows, for instance) I'm not sure where the line is on that --
my thought has been that the platform-specific UIs would build the strings, and
then windows can do it's thing there.

https://codereview.chromium.org/12211084/diff/1/chrome/browser/system_monitor...
File chrome/browser/system_monitor/removable_storage_notifications.h (right):

https://codereview.chromium.org/12211084/diff/1/chrome/browser/system_monitor...
chrome/browser/system_monitor/removable_storage_notifications.h:29:
StorageInfo(const std::string& id,
That's correct -- we won't necessarily collect all this information, or it may
not be collectable. On ChromeOS we can get it all pretty readily. For MTP,
Windows, and Mac some of the fields either aren't known, don't apply, or we
don't currently  collect them. I do think we should start attempting to do this
uniformly, though. I mostly left this alone to not be as invasive while adding
new fields.

On 2013/02/08 02:03:31, vandebo wrote:
> This constructor implies that sometimes the new fields won't be valid.  Can we
> distinguish that case from when the new fields simply aren't available?  I'd
> rather have all the fields be valid all the time, which means the constructor
> should take everything.

https://codereview.chromium.org/12211084/diff/1/chrome/browser/system_monitor...
chrome/browser/system_monitor/removable_storage_notifications.h:40: string16
name;
On 2013/02/08 02:03:31, vandebo wrote:
> Do you want to remove this and instead have a function that takes a
StorageInfo
> and constructors a human readable name?

Yes. I left it alone for now -- my idea was to strictly add fields and leave all
existing functionality untouched, then convert clients to use the new fields.
Shall I just bite off the whole thing?

https://codereview.chromium.org/12211084/diff/1/chrome/browser/system_monitor...
chrome/browser/system_monitor/removable_storage_notifications.h:43: //
TODO(gbillock): Just use a FilePath here?
On 2013/02/08 02:03:31, vandebo wrote:
> For MTP devices it is not a real file path.

Right. They'd leave the FilePath as empty(). Currently we do some very shocking
things with MTP and paths.

https://codereview.chromium.org/12211084/diff/1/chrome/browser/system_monitor...
chrome/browser/system_monitor/removable_storage_notifications.h:64:
MediaStorageUtil::Type device_type;
On 2013/02/08 02:03:31, vandebo wrote:
> This is already in device_id, there's no reason to break it out separately.

The existence of CrackDeviceId and MakeDeviceId mean that field is not opaque,
and is really two fields, both of which are accessible. I think that's the right
visibility choice, but we should move to storing the two fields as two separate
fields, not combining them. Even though it's done losslessly it just makes the
existing code more difficult. Is there a motivation in the way prefs are stored
for this? I haven't read that as carefully as the system_monitor stuff.

https://codereview.chromium.org/12211084/diff/1/chrome/browser/system_monitor...
chrome/browser/system_monitor/removable_storage_notifications.h:89: virtual
uint64 GetStorageSize(const std::string& location) const = 0;
On 2013/02/08 02:03:31, vandebo wrote:
> If we have storage size in the storage info, then we don't need this method
any
> more, right?

Right. I'm planning to get rid of it. The use is in
MediaStorageUtil::GetDeviceInfoFromPath, which needs a lot of attention.

vandebo (ex-Chrome)

As we discussed, it's probably easier to change prefs first. https://codereview.chromium.org/12211084/diff/1/chrome/browser/system_monitor/removable_storage_notifications.h File chrome/browser/system_monitor/removable_storage_notifications.h (right): https://codereview.chromium.org/12211084/diff/1/chrome/browser/system_monitor/removable_storage_notifications.h#newcode29 ...

7 years, 10 months ago (2013-02-08 23:20:35 UTC) #4

As we discussed, it's probably easier to change prefs first.

https://codereview.chromium.org/12211084/diff/1/chrome/browser/system_monitor...
File chrome/browser/system_monitor/removable_storage_notifications.h (right):

https://codereview.chromium.org/12211084/diff/1/chrome/browser/system_monitor...
chrome/browser/system_monitor/removable_storage_notifications.h:29:
StorageInfo(const std::string& id,
On 2013/02/08 16:42:02, Greg Billock wrote:
> That's correct -- we won't necessarily collect all this information, or it may
> not be collectable. On ChromeOS we can get it all pretty readily. For MTP,
> Windows, and Mac some of the fields either aren't known, don't apply, or we
> don't currently  collect them. I do think we should start attempting to do
this
> uniformly, though. I mostly left this alone to not be as invasive while adding
> new fields.
> 
> On 2013/02/08 02:03:31, vandebo wrote:
> > This constructor implies that sometimes the new fields won't be valid.  Can
we
> > distinguish that case from when the new fields simply aren't available?  I'd
> > rather have all the fields be valid all the time, which means the
constructor
> > should take everything.
> 

If we don't collect something on a platform that's fine, but we should fill in
everything that we do populate for that platform.  As such, I think the
constructor should take all of those items.  It's a pet-peeve of mine to have
fields that are only valid some of the time, especially when there's no way to
tell if it should be valid or not.

https://codereview.chromium.org/12211084/diff/1/chrome/browser/system_monitor...
chrome/browser/system_monitor/removable_storage_notifications.h:40: string16
name;
On 2013/02/08 16:42:02, Greg Billock wrote:
> On 2013/02/08 02:03:31, vandebo wrote:
> > Do you want to remove this and instead have a function that takes a
> StorageInfo
> > and constructors a human readable name?
> 
> Yes. I left it alone for now -- my idea was to strictly add fields and leave
all
> existing functionality untouched, then convert clients to use the new fields.
> Shall I just bite off the whole thing?

I think that's a sufficiently focused CL: Push name generation of the stack and
split out the info needed for names here.

https://codereview.chromium.org/12211084/diff/1/chrome/browser/system_monitor...
chrome/browser/system_monitor/removable_storage_notifications.h:43: //
TODO(gbillock): Just use a FilePath here?
On 2013/02/08 16:42:02, Greg Billock wrote:
> On 2013/02/08 02:03:31, vandebo wrote:
> > For MTP devices it is not a real file path.
> 
> Right. They'd leave the FilePath as empty(). Currently we do some very
shocking
> things with MTP and paths.

But we need a location string to be able to connect to the device, where would
we store that?

https://codereview.chromium.org/12211084/diff/1/chrome/browser/system_monitor...
chrome/browser/system_monitor/removable_storage_notifications.h:64:
MediaStorageUtil::Type device_type;
On 2013/02/08 16:42:02, Greg Billock wrote:
> On 2013/02/08 02:03:31, vandebo wrote:
> > This is already in device_id, there's no reason to break it out separately.
> 
> The existence of CrackDeviceId and MakeDeviceId mean that field is not opaque,
> and is really two fields, both of which are accessible. I think that's the
right
> visibility choice, but we should move to storing the two fields as two
separate
> fields, not combining them. Even though it's done losslessly it just makes the
> existing code more difficult. Is there a motivation in the way prefs are
stored
> for this? I haven't read that as carefully as the system_monitor stuff.

Prefs only stores a device id.  To make them store both a device id and a type
would be a non-backward compatible change, requiring conversion code; I don't
think it's worth it.  If we do want to do it though, lets do that in it's own
CL.

Greg Billock

On 2013/02/08 23:20:35, vandebo wrote: > As we discussed, it's probably easier to change prefs ...

7 years, 9 months ago (2013-02-27 23:30:42 UTC) #5

On 2013/02/08 23:20:35, vandebo wrote:
> As we discussed, it's probably easier to change prefs first.
> 
>
https://codereview.chromium.org/12211084/diff/1/chrome/browser/system_monitor...
> File chrome/browser/system_monitor/removable_storage_notifications.h (right):
> 
>
https://codereview.chromium.org/12211084/diff/1/chrome/browser/system_monitor...
> chrome/browser/system_monitor/removable_storage_notifications.h:29:
> StorageInfo(const std::string& id,
> On 2013/02/08 16:42:02, Greg Billock wrote:
> > That's correct -- we won't necessarily collect all this information, or it
may
> > not be collectable. On ChromeOS we can get it all pretty readily. For MTP,
> > Windows, and Mac some of the fields either aren't known, don't apply, or we
> > don't currently  collect them. I do think we should start attempting to do
> this
> > uniformly, though. I mostly left this alone to not be as invasive while
adding
> > new fields.
> > 
> > On 2013/02/08 02:03:31, vandebo wrote:
> > > This constructor implies that sometimes the new fields won't be valid. 
Can
> we
> > > distinguish that case from when the new fields simply aren't available? 
I'd
> > > rather have all the fields be valid all the time, which means the
> constructor
> > > should take everything.
> > 
> 
> If we don't collect something on a platform that's fine, but we should fill in
> everything that we do populate for that platform.  As such, I think the
> constructor should take all of those items.  It's a pet-peeve of mine to have
> fields that are only valid some of the time, especially when there's no way to
> tell if it should be valid or not.
> 
>
https://codereview.chromium.org/12211084/diff/1/chrome/browser/system_monitor...
> chrome/browser/system_monitor/removable_storage_notifications.h:40: string16
> name;
> On 2013/02/08 16:42:02, Greg Billock wrote:
> > On 2013/02/08 02:03:31, vandebo wrote:
> > > Do you want to remove this and instead have a function that takes a
> > StorageInfo
> > > and constructors a human readable name?
> > 
> > Yes. I left it alone for now -- my idea was to strictly add fields and leave
> all
> > existing functionality untouched, then convert clients to use the new
fields.
> > Shall I just bite off the whole thing?
> 
> I think that's a sufficiently focused CL: Push name generation of the stack
and
> split out the info needed for names here.
> 
>
https://codereview.chromium.org/12211084/diff/1/chrome/browser/system_monitor...
> chrome/browser/system_monitor/removable_storage_notifications.h:43: //
> TODO(gbillock): Just use a FilePath here?
> On 2013/02/08 16:42:02, Greg Billock wrote:
> > On 2013/02/08 02:03:31, vandebo wrote:
> > > For MTP devices it is not a real file path.
> > 
> > Right. They'd leave the FilePath as empty(). Currently we do some very
> shocking
> > things with MTP and paths.
> 
> But we need a location string to be able to connect to the device, where would
> we store that?
> 
>
https://codereview.chromium.org/12211084/diff/1/chrome/browser/system_monitor...
> chrome/browser/system_monitor/removable_storage_notifications.h:64:
> MediaStorageUtil::Type device_type;
> On 2013/02/08 16:42:02, Greg Billock wrote:
> > On 2013/02/08 02:03:31, vandebo wrote:
> > > This is already in device_id, there's no reason to break it out
separately.
> > 
> > The existence of CrackDeviceId and MakeDeviceId mean that field is not
opaque,
> > and is really two fields, both of which are accessible. I think that's the
> right
> > visibility choice, but we should move to storing the two fields as two
> separate
> > fields, not combining them. Even though it's done losslessly it just makes
the
> > existing code more difficult. Is there a motivation in the way prefs are
> stored
> > for this? I haven't read that as carefully as the system_monitor stuff.
> 
> Prefs only stores a device id.  To make them store both a device id and a type
> would be a non-backward compatible change, requiring conversion code; I don't
> think it's worth it.  If we do want to do it though, lets do that in it's own
> CL.

Ready for another look. Restricted to ChromeOS/Linux platforms. Uses
pre-existing fields. Much more digestible. :-)

vandebo (ex-Chrome)

https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_monitor/removable_device_notifications_chromeos.cc File chrome/browser/storage_monitor/removable_device_notifications_chromeos.cc (right): https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_monitor/removable_device_notifications_chromeos.cc#newcode268 chrome/browser/storage_monitor/removable_device_notifications_chromeos.cc:268: object_info.total_size_in_bytes = storage_size_in_bytes; This is weird.... The constructor is ...

7 years, 9 months ago (2013-03-01 22:21:48 UTC) #6

Greg Billock

https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_monitor/removable_device_notifications_chromeos.cc File chrome/browser/storage_monitor/removable_device_notifications_chromeos.cc (right): https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_monitor/removable_device_notifications_chromeos.cc#newcode268 chrome/browser/storage_monitor/removable_device_notifications_chromeos.cc:268: object_info.total_size_in_bytes = storage_size_in_bytes; On 2013/03/01 22:21:48, vandebo wrote: > ...

7 years, 9 months ago (2013-03-05 19:20:55 UTC) #7

https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_mo...
File chrome/browser/storage_monitor/removable_device_notifications_chromeos.cc
(right):

https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_mo...
chrome/browser/storage_monitor/removable_device_notifications_chromeos.cc:268:
object_info.total_size_in_bytes = storage_size_in_bytes;
On 2013/03/01 22:21:48, vandebo wrote:
> This is weird....  The constructor is just a helper to populate fields, so
> should it take all the fields?

Yes, we should have a constructor like that. I'm going to leave that for after
the refactor though.

https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_mo...
File chrome/browser/storage_monitor/removable_device_notifications_linux.cc
(right):

https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_mo...
chrome/browser/storage_monitor/removable_device_notifications_linux.cc:240: bool
has_dcim = IsMediaDevice(mount_point.value());
On 2013/03/01 22:21:48, vandebo wrote:
> This block should all be in if(device_id) {

Done.

https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_mo...
chrome/browser/storage_monitor/removable_device_notifications_linux.cc:332:
base::FilePath RemovableDeviceNotificationsLinux::GetDeviceForMountPoint(
On 2013/03/01 22:21:48, vandebo wrote:
> Not a fan of this.  Yes all the information is here, but it's complicated to
get
> at.  How about changing MountPointInfo to:
> 
> struct MountPointInfo {
>   StorageInfo storage_info;
>   base::FilePath mount_device;
> };

I think it's complicated because of the priority map, but I don't have a
suggested simplification for that -- tracking multiple mount points is just
complex.

One thing to like about this is that the two data structures are now storing
disjoint information -- I think even better would be to rekey mount_info_map_,
but I didn't do that here because it's kind of hard to follow.

https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_mo...
chrome/browser/storage_monitor/removable_device_notifications_linux.cc:473:
StorageInfo mount_point_info;
On 2013/03/01 22:21:48, vandebo wrote:
> use the constructor?

Done.

https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_mo...
File chrome/browser/storage_monitor/removable_device_notifications_linux.h
(right):

https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_mo...
chrome/browser/storage_monitor/removable_device_notifications_linux.h:34:
typedef void (*GetDeviceInfoFunc)(const base::FilePath& device_path,
On 2013/03/01 22:21:48, vandebo wrote:
> So I think the signature of this function is pretty consisten across
platforms. 
> I'm not sure how I feel about changing that.  Also, this is a function that
gets
> changed out for tests.  So the less code we have in here the better.

We are in the position now where we can use inheritance for the tests instead of
the swappable function. I've changed a couple to use the mocks, I think the rest
can follow, or perhaps delegate to a protected method (which we are also doing
in a couple spots...)

This really should take a StorageInfo*.

vandebo (ex-Chrome)

https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_monitor/removable_device_notifications_chromeos.cc File chrome/browser/storage_monitor/removable_device_notifications_chromeos.cc (right): https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_monitor/removable_device_notifications_chromeos.cc#newcode268 chrome/browser/storage_monitor/removable_device_notifications_chromeos.cc:268: object_info.total_size_in_bytes = storage_size_in_bytes; On 2013/03/05 19:20:55, Greg Billock wrote: ...

7 years, 9 months ago (2013-03-06 01:34:54 UTC) #8

https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_mo...
File chrome/browser/storage_monitor/removable_device_notifications_chromeos.cc
(right):

https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_mo...
chrome/browser/storage_monitor/removable_device_notifications_chromeos.cc:268:
object_info.total_size_in_bytes = storage_size_in_bytes;
On 2013/03/05 19:20:55, Greg Billock wrote:
> On 2013/03/01 22:21:48, vandebo wrote:
> > This is weird....  The constructor is just a helper to populate fields, so
> > should it take all the fields?
> 
> Yes, we should have a constructor like that. I'm going to leave that for after
> the refactor though.

If you like.  But if so, add a TODO here so that we don't forget about it.

https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_mo...
File chrome/browser/storage_monitor/removable_device_notifications_linux.cc
(right):

https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_mo...
chrome/browser/storage_monitor/removable_device_notifications_linux.cc:332:
base::FilePath RemovableDeviceNotificationsLinux::GetDeviceForMountPoint(
On 2013/03/05 19:20:55, Greg Billock wrote:
> On 2013/03/01 22:21:48, vandebo wrote:
> > Not a fan of this.  Yes all the information is here, but it's complicated to
> get
> > at.  How about changing MountPointInfo to:
> > 
> > struct MountPointInfo {
> >   StorageInfo storage_info;
> >   base::FilePath mount_device;
> > };
> 
> I think it's complicated because of the priority map, but I don't have a
> suggested simplification for that -- tracking multiple mount points is just
> complex.
> 
> One thing to like about this is that the two data structures are now storing
> disjoint information -- I think even better would be to rekey mount_info_map_,
> but I didn't do that here because it's kind of hard to follow.

So the way it was before was storing some redundant information, but that made
it fast to access.  At the least, this seems like an unrelated change that
should go into its own CL.

https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_mo...
File chrome/browser/storage_monitor/removable_device_notifications_linux.h
(right):

https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_mo...
chrome/browser/storage_monitor/removable_device_notifications_linux.h:34:
typedef void (*GetDeviceInfoFunc)(const base::FilePath& device_path,
On 2013/03/05 19:20:55, Greg Billock wrote:
> On 2013/03/01 22:21:48, vandebo wrote:
> > So I think the signature of this function is pretty consisten across
> platforms. 
> > I'm not sure how I feel about changing that.  Also, this is a function that
> gets
> > changed out for tests.  So the less code we have in here the better.
> 
> We are in the position now where we can use inheritance for the tests instead
of
> the swappable function. I've changed a couple to use the mocks, I think the
rest
> can follow, or perhaps delegate to a protected method (which we are also doing
> in a couple spots...)
> 
> This really should take a StorageInfo*.

Inheritance for the test sounds good.  The swappable functions are ugly.  But
every bit of code that is in a function with a OS specific APIs that we can't
mock is code that can't be tested.  As much as possible, I'd like to keep the OS
calls in isolated functions that do as little as possible in order to increase
our testability.

Greg Billock

https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_monitor/removable_device_notifications_chromeos.cc File chrome/browser/storage_monitor/removable_device_notifications_chromeos.cc (right): https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_monitor/removable_device_notifications_chromeos.cc#newcode268 chrome/browser/storage_monitor/removable_device_notifications_chromeos.cc:268: object_info.total_size_in_bytes = storage_size_in_bytes; On 2013/03/06 01:34:54, vandebo wrote: > ...

7 years, 9 months ago (2013-03-06 23:24:25 UTC) #9

https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_mo...
File chrome/browser/storage_monitor/removable_device_notifications_chromeos.cc
(right):

https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_mo...
chrome/browser/storage_monitor/removable_device_notifications_chromeos.cc:268:
object_info.total_size_in_bytes = storage_size_in_bytes;
On 2013/03/06 01:34:54, vandebo wrote:
> On 2013/03/05 19:20:55, Greg Billock wrote:
> > On 2013/03/01 22:21:48, vandebo wrote:
> > > This is weird....  The constructor is just a helper to populate fields, so
> > > should it take all the fields?
> > 
> > Yes, we should have a constructor like that. I'm going to leave that for
after
> > the refactor though.
> 
> If you like.  But if so, add a TODO here so that we don't forget about it.

Refactor is in, just updated constructor.

https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_mo...
File chrome/browser/storage_monitor/removable_device_notifications_linux.h
(right):

https://codereview.chromium.org/12211084/diff/34008/chrome/browser/storage_mo...
chrome/browser/storage_monitor/removable_device_notifications_linux.h:34:
typedef void (*GetDeviceInfoFunc)(const base::FilePath& device_path,
On 2013/03/06 01:34:54, vandebo wrote:
> On 2013/03/05 19:20:55, Greg Billock wrote:
> > On 2013/03/01 22:21:48, vandebo wrote:
> > > So I think the signature of this function is pretty consisten across
> > platforms. 
> > > I'm not sure how I feel about changing that.  Also, this is a function
that
> > gets
> > > changed out for tests.  So the less code we have in here the better.
> > 
> > We are in the position now where we can use inheritance for the tests
instead
> of
> > the swappable function. I've changed a couple to use the mocks, I think the
> rest
> > can follow, or perhaps delegate to a protected method (which we are also
doing
> > in a couple spots...)
> > 
> > This really should take a StorageInfo*.
> 
> Inheritance for the test sounds good.  The swappable functions are ugly.  But
> every bit of code that is in a function with a OS specific APIs that we can't
> mock is code that can't be tested.  As much as possible, I'd like to keep the
OS
> calls in isolated functions that do as little as possible in order to increase
> our testability. 

I think we have a plan for this now.

vandebo (ex-Chrome)

LGTM https://codereview.chromium.org/12211084/diff/51001/chrome/browser/storage_monitor/storage_info.h File chrome/browser/storage_monitor/storage_info.h (right): https://codereview.chromium.org/12211084/diff/51001/chrome/browser/storage_monitor/storage_info.h#newcode15 chrome/browser/storage_monitor/storage_info.h:15: StorageInfo(const std::string& id, Is this one staying just ...

7 years, 9 months ago (2013-03-06 23:39:12 UTC) #10

Greg Billock

https://codereview.chromium.org/12211084/diff/51001/chrome/browser/storage_monitor/storage_info.h File chrome/browser/storage_monitor/storage_info.h (right): https://codereview.chromium.org/12211084/diff/51001/chrome/browser/storage_monitor/storage_info.h#newcode15 chrome/browser/storage_monitor/storage_info.h:15: StorageInfo(const std::string& id, On 2013/03/06 23:39:12, vandebo wrote: > ...

7 years, 9 months ago (2013-03-07 21:06:06 UTC) #11

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/gbillock@chromium.org/12211084/55001

7 years, 9 months ago (2013-03-07 21:31:50 UTC) #13

Message was sent while issue was closed.

Change committed as 186835

Issue 12211084: [Media Galleries] Populate volume metadata in ChromeOS/Linux (Closed)

Description

Patch Set 1 #

Patch Set 2 : Add mac impl #

Patch Set 3 : Add windows impl #

Patch Set 4 : Windows work #

Patch Set 5 : Add windows tests. #

Patch Set 6 : Add linux -- get volume metadata; use StorageInfo #

Patch Set 7 : Merge to head #

Patch Set 8 : Rebase to head #

Patch Set 9 : Restrict to linux/chromeos #

Patch Set 10 : Reset header file #

Patch Set 11 : Use constructor #

Patch Set 12 : Rebase #

Patch Set 13 : Add larger constructor, put linux data structures back #

Patch Set 14 : Fix linux problems #

Messages