Use mounted device label as name in media device notification message.

Use mounted device label as name in media device notification message.

BUG=none
TEST=none


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

[kmadhusu, 2012-08-03 01:33:05]

thestig@: Please review. Thanks.

[Lei Zhang, 2012-08-03 01:44:03]

https://chromiumcodereview.appspot.com/10826135/diff/3001/chrome/browser/medi...
File chrome/browser/media_gallery/media_device_notifications_chromeos.cc
(right):

https://chromiumcodereview.appspot.com/10826135/diff/3001/chrome/browser/medi...
chrome/browser/media_gallery/media_device_notifications_chromeos.cc:30:
*device_id_str = it->second->fs_uuid();
The code here and below may be more readable if you first declare const
...::Disk& disk = it->second;

https://chromiumcodereview.appspot.com/10826135/diff/3001/chrome/browser/medi...
chrome/browser/media_gallery/media_device_notifications_chromeos.cc:127:
std::string device_label;
just make this a string16 and do the conversion in GetDeviceInfo.

[Lei Zhang, 2012-08-03 02:05:51]

https://chromiumcodereview.appspot.com/10826135/diff/3001/chrome/browser/medi...
File chrome/browser/media_gallery/media_device_notifications_chromeos.cc
(right):

https://chromiumcodereview.appspot.com/10826135/diff/3001/chrome/browser/medi...
chrome/browser/media_gallery/media_device_notifications_chromeos.cc:32: // from
udevadm and use them as device_label.
You shouldn't have to use udevadm as a workaround. Unlike cases where the API is
final on other OSes, on CrOS, all you have to do is ask benchan@ to add some
more code to the disk manager and expose the needed info.

[kmadhusu, 2012-08-03 05:22:37]

Addressed comments. PTAL. Thanks.

https://chromiumcodereview.appspot.com/10826135/diff/3001/chrome/browser/medi...
File chrome/browser/media_gallery/media_device_notifications_chromeos.cc
(right):

https://chromiumcodereview.appspot.com/10826135/diff/3001/chrome/browser/medi...
chrome/browser/media_gallery/media_device_notifications_chromeos.cc:30:
*device_id_str = it->second->fs_uuid();
On 2012/08/03 01:44:03, Lei Zhang wrote:
> The code here and below may be more readable if you first declare const
> ...::Disk& disk = it->second;

Done.

https://chromiumcodereview.appspot.com/10826135/diff/3001/chrome/browser/medi...
chrome/browser/media_gallery/media_device_notifications_chromeos.cc:32: // from
udevadm and use them as device_label.
On 2012/08/03 02:05:51, Lei Zhang wrote:
> You shouldn't have to use udevadm as a workaround. Unlike cases where the API
is
> final on other OSes, on CrOS, all you have to do is ask benchan@ to add some
> more code to the disk manager and expose the needed info.

sure.  Modified TODO comment.

https://chromiumcodereview.appspot.com/10826135/diff/3001/chrome/browser/medi...
chrome/browser/media_gallery/media_device_notifications_chromeos.cc:127:
std::string device_label;
On 2012/08/03 01:44:03, Lei Zhang wrote:
> just make this a string16 and do the conversion in GetDeviceInfo.

Done.

[Lei Zhang, 2012-08-03 05:45:24]

lgtm with suggestions below:

https://chromiumcodereview.appspot.com/10826135/diff/6001/chrome/browser/medi...
File chrome/browser/media_gallery/media_device_notifications_chromeos.cc
(right):

https://chromiumcodereview.appspot.com/10826135/diff/6001/chrome/browser/medi...
chrome/browser/media_gallery/media_device_notifications_chromeos.cc:22: void
GetDeviceInfo(const std::string& source_path, std::string* device_id_str,
|device_id_str| -> |device_id| here and below.

https://chromiumcodereview.appspot.com/10826135/diff/6001/chrome/browser/medi...
chrome/browser/media_gallery/media_device_notifications_chromeos.cc:29: return;
May want to return bool, so the caller knows |device_id_str| is not empty on
accident.

https://chromiumcodereview.appspot.com/10826135/diff/6001/chrome/browser/medi...
chrome/browser/media_gallery/media_device_notifications_chromeos.cc:36:
*device_label = disk.device_label().empty() ?
Would UTF8ToUTF16(foo ? bar : qux) be better?

[kmadhusu, 2012-08-03 06:07:09]

Addressed comments.

https://chromiumcodereview.appspot.com/10826135/diff/6001/chrome/browser/medi...
File chrome/browser/media_gallery/media_device_notifications_chromeos.cc
(right):

https://chromiumcodereview.appspot.com/10826135/diff/6001/chrome/browser/medi...
chrome/browser/media_gallery/media_device_notifications_chromeos.cc:22: void
GetDeviceInfo(const std::string& source_path, std::string* device_id_str,
On 2012/08/03 05:45:25, Lei Zhang wrote:
> |device_id_str| -> |device_id| here and below.

Done.

https://chromiumcodereview.appspot.com/10826135/diff/6001/chrome/browser/medi...
chrome/browser/media_gallery/media_device_notifications_chromeos.cc:29: return;
On 2012/08/03 05:45:25, Lei Zhang wrote:
> May want to return bool, so the caller knows |device_id_str| is not empty on
> accident.

Done.

https://chromiumcodereview.appspot.com/10826135/diff/6001/chrome/browser/medi...
chrome/browser/media_gallery/media_device_notifications_chromeos.cc:36:
*device_label = disk.device_label().empty() ?
On 2012/08/03 05:45:25, Lei Zhang wrote:
> Would UTF8ToUTF16(foo ? bar : qux) be better?

Done.

[commit-bot: I haz the power, 2012-08-03 06:09:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kmadhusu@chromium.org/10826135/6002

[commit-bot: I haz the power, 2012-08-03 07:31:57]

Change committed as 149809