Share MediaStorageUtil common code among platforms.

Share MediaStorageUtil common code among platforms.

Also hook up Mac implementation.

TBR=thestig@chromium.org
BUG=NONE


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

[kmadhusu, 2012-09-16 21:44:11]

https://codereview.chromium.org/10909259/diff/1/chrome/browser/system_monitor...
File chrome/browser/system_monitor/media_storage_util.cc (right):

https://codereview.chromium.org/10909259/diff/1/chrome/browser/system_monitor...
chrome/browser/system_monitor/media_storage_util.cc:180: #elif defiend(OS_LINUX)
typo: defined

[vandebo (ex-Chrome), 2012-09-16 21:48:40]

https://codereview.chromium.org/10909259/diff/1/chrome/browser/system_monitor...
File chrome/browser/system_monitor/media_storage_util.cc (right):

https://codereview.chromium.org/10909259/diff/1/chrome/browser/system_monitor...
chrome/browser/system_monitor/media_storage_util.cc:180: #elif defiend(OS_LINUX)
On 2012/09/16 21:44:11, kmadhusu wrote:
> typo: defined

Done.

[kmadhusu, 2012-09-16 21:52:53]

(drive-by comments)

https://codereview.chromium.org/10909259/diff/1/chrome/browser/system_monitor...
File chrome/browser/system_monitor/media_storage_util.cc (right):

https://codereview.chromium.org/10909259/diff/1/chrome/browser/system_monitor...
chrome/browser/system_monitor/media_storage_util.cc:190: //   
RemovableDeviceNotificationsWindowWin::GetInstance();
This function has too many if defs. For better readability, can we do? 
 #if defined(OS_CHROMEOS)
typedef class chromeos::RemovableDeviceNotificationsCros
RemovabeDeviceNotications;
#elif defined (OS_LINUX)
typedef class RemovableDeviceNotificationsLinux RemovabeDeviceNotications;
#elif ...
#endif  // OS_CHROMEOS

RemovableDeviceNotifications* notifier =
RemovableDeviceNotifications::GetInstance();

[vandebo (ex-Chrome), 2012-09-16 22:29:40]

https://codereview.chromium.org/10909259/diff/1/chrome/browser/system_monitor...
File chrome/browser/system_monitor/media_storage_util.cc (right):

https://codereview.chromium.org/10909259/diff/1/chrome/browser/system_monitor...
chrome/browser/system_monitor/media_storage_util.cc:190: //   
RemovableDeviceNotificationsWindowWin::GetInstance();
On 2012/09/16 21:52:53, kmadhusu wrote:
> This function has too many if defs. For better readability, can we do? 
>  #if defined(OS_CHROMEOS)
> typedef class chromeos::RemovableDeviceNotificationsCros
> RemovabeDeviceNotications;
> #elif defined (OS_LINUX)
> typedef class RemovableDeviceNotificationsLinux RemovabeDeviceNotications;
> #elif ...
> #endif  // OS_CHROMEOS
> 
> RemovableDeviceNotifications* notifier =
> RemovableDeviceNotifications::GetInstance();

Did a variation of this.

[kmadhusu, 2012-09-16 23:17:17]

lgtm

[vandebo (ex-Chrome), 2012-09-16 23:18:46]

TBRing the small .gypi change.

[commit-bot: I haz the power, 2012-09-16 23:19:00]

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

[commit-bot: I haz the power, 2012-09-17 00:22:33]

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

[Lei Zhang, 2012-09-17 00:25:47]

https://codereview.chromium.org/10909259/diff/11001/chrome/browser/system_mon...
File chrome/browser/system_monitor/media_storage_util.cc (right):

https://codereview.chromium.org/10909259/diff/11001/chrome/browser/system_mon...
chrome/browser/system_monitor/media_storage_util.cc:181: #if 0 &&
defined(OS_CHROMEOS)
Whose job is it to remove the #if 0 bit and use mtp manager?

https://codereview.chromium.org/10909259/diff/11001/chrome/browser/system_mon...
chrome/browser/system_monitor/media_storage_util.cc:200: #if !defined(POSIX)
OS_POSIX?

https://codereview.chromium.org/10909259/diff/11001/chrome/browser/system_mon...
File chrome/browser/system_monitor/removable_device_notifications_linux.h
(right):

https://codereview.chromium.org/10909259/diff/11001/chrome/browser/system_mon...
chrome/browser/system_monitor/removable_device_notifications_linux.h:39: typedef
RemovableDeviceNotificationsLinux RemovableDeviceNotifications;
It would be better to make RemovableDeviceNotifications an interface. Right now
we have several platform impls that just happen to have the same
GetDeviceInfoForPath() method.

[vandebo (ex-Chrome), 2012-09-17 00:32:58]

https://codereview.chromium.org/10909259/diff/11001/chrome/browser/system_mon...
File chrome/browser/system_monitor/media_storage_util.cc (right):

https://codereview.chromium.org/10909259/diff/11001/chrome/browser/system_mon...
chrome/browser/system_monitor/media_storage_util.cc:181: #if 0 &&
defined(OS_CHROMEOS)
On 2012/09/17 00:25:47, Lei Zhang wrote:
> Whose job is it to remove the #if 0 bit and use mtp manager?

http://codereview.chromium.org/10908277/ will take care of it shortly.

https://codereview.chromium.org/10909259/diff/11001/chrome/browser/system_mon...
chrome/browser/system_monitor/media_storage_util.cc:200: #if !defined(POSIX)
On 2012/09/17 00:25:47, Lei Zhang wrote:
> OS_POSIX?

Done.

https://codereview.chromium.org/10909259/diff/11001/chrome/browser/system_mon...
File chrome/browser/system_monitor/removable_device_notifications_linux.h
(right):

https://codereview.chromium.org/10909259/diff/11001/chrome/browser/system_mon...
chrome/browser/system_monitor/removable_device_notifications_linux.h:39: typedef
RemovableDeviceNotificationsLinux RemovableDeviceNotifications;
On 2012/09/17 00:25:47, Lei Zhang wrote:
> It would be better to make RemovableDeviceNotifications an interface. Right
now
> we have several platform impls that just happen to have the same
> GetDeviceInfoForPath() method.

That's about all they share though.  base::SystemMonitor needs to be refactored
and I have a feeling it will have ramifications on this.

[commit-bot: I haz the power, 2012-09-17 03:10:43]

Sorry for I got bad news for ya.
Compile failed with a clobber build.
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-09-17 04:30:28]

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

[commit-bot: I haz the power, 2012-09-17 05:34:26]

Sorry for I got bad news for ya.
Compile failed with a clobber build.
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-09-17 05:34:50]

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

[commit-bot: I haz the power, 2012-09-17 06:51:44]

Change committed as 157085