Linux: Refactor udev device monitoring code into its own class so it can be reused more easily.

Linux: Refactor udev device monitoring code into its own class so it can be reused more easily.

BUG=none
TEST=none

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

[Lei Zhang, 2012-07-26 01:44:31]

scottmg: Main reviewer
jam: Is there a better location for udev_linux.* than content/browser? wjia@ is
looking to reuse this code for audio/video device detection.

https://chromiumcodereview.appspot.com/10824036/diff/1/content/browser/udev_l...
File content/browser/udev_linux.h (right):

https://chromiumcodereview.appspot.com/10824036/diff/1/content/browser/udev_l...
content/browser/udev_linux.h:17: 
I probably need to add a description here and an example use case... but you can
start reviewing without that.

[jam, 2012-07-26 02:11:33]

http://codereview.chromium.org/10824036/diff/1/content/browser/udev_linux.h
File content/browser/udev_linux.h (right):

http://codereview.chromium.org/10824036/diff/1/content/browser/udev_linux.h#n...
content/browser/udev_linux.h:25: virtual void Notify(udev_device* device) = 0;
Instead of implementation inheritance, can you make the constructor of this
class take a callback? then users of this class can use composition instead,
which makes things easier to read.
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Inheri...

[Lei Zhang, 2012-07-26 04:22:58]

Ok, see patch set 2.

[scottmg, 2012-07-26 16:30:36]

lgtm

(I know not-much about udev, but as far as refactoring, looks good... Manual
test w/ gamepad working)

https://chromiumcodereview.appspot.com/10824036/diff/9/content/browser/udev_l...
File content/browser/udev_linux.h (right):

https://chromiumcodereview.appspot.com/10824036/diff/9/content/browser/udev_l...
content/browser/udev_linux.h:58: // subsystem and devtype paramater for
parameter

[Lei Zhang, 2012-07-26 18:39:44]

https://chromiumcodereview.appspot.com/10824036/diff/9/content/browser/udev_l...
File content/browser/udev_linux.h (right):

https://chromiumcodereview.appspot.com/10824036/diff/9/content/browser/udev_l...
content/browser/udev_linux.h:58: // subsystem and devtype paramater for
On 2012/07/26 16:30:37, scottmg wrote:
> parameter

Done.

[jam, 2012-07-26 19:34:00]

lgtm

[commit-bot: I haz the power, 2012-07-26 20:40:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thestig@chromium.org/10824036/7002

[commit-bot: I haz the power, 2012-07-26 23:49:50]

Try job failure for 10824036-7002 (previous was lost) (retry) on linux_rel for
step "browser_tests".
It's a second try, previously, steps "interactive_ui_tests, browser_tests"
failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-07-27 00:18:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thestig@chromium.org/10824036/7002

[commit-bot: I haz the power, 2012-07-27 02:03:19]

Change committed as 148698