Move chromeos::device_event_log into components.

Move chromeos::device_event_log into components.

There is device code in Chrome on non-Chrome OS platforms that would
benefit from a user-visible log dashboard like chrome://device-log. This
change moves the code for managing the device log into a component and
enables chrome://device-log on all versions of Chrome.

BUG=452609
Committed: https://crrev.com/4a84927b28b6b55d0ef9e4df59d2e1a14b2b3de2
Cr-Commit-Position: refs/heads/master@{#317406}

[Reilly Grant (use Gerrit), 2015-02-12 23:56:24]

Patchset #1 (id:1) has been deleted

[Reilly Grant (use Gerrit), 2015-02-13 01:23:01]

Patchset #1 (id:20001) has been deleted

[Reilly Grant (use Gerrit), 2015-02-13 01:25:21]

reillyg@chromium.org changed reviewers:
+ stevenjb@chromium.org

[Reilly Grant (use Gerrit), 2015-02-13 01:25:23]

Steven, please take a look at this patch.

[stevenjb, 2015-02-17 19:24:35]

https://codereview.chromium.org/919183002/diff/60001/chrome/browser/browser_p...
File chrome/browser/browser_process_impl.cc (right):

https://codereview.chromium.org/919183002/diff/60001/chrome/browser/browser_p...
chrome/browser/browser_process_impl.cc:190: device_event_log::Initialize(0 /*
default max entries */);
I think this would actually be better in chrome_browser_main_parts, specifically
ChromeBrowserMainParts::PreMainMessageLoopStart, since this doesn't have
anything to do with the "browser" per-se, and so that it is available earlier
on, e.g. for ChromeOS which does a lot of initialization (which may include
logging) in PostMainMessageLoopStart.

https://codereview.chromium.org/919183002/diff/60001/chromeos/dbus/DEPS
File chromeos/dbus/DEPS (right):

https://codereview.chromium.org/919183002/diff/60001/chromeos/dbus/DEPS#newcode2
chromeos/dbus/DEPS:2: "+components/device_event_log",
This can just be in chromeos/DEPS since chromeos/network also depends on it, and
others may start to.

https://codereview.chromium.org/919183002/diff/60001/chromeos/dbus/dbus_threa...
File chromeos/dbus/dbus_thread_manager.cc (right):

https://codereview.chromium.org/919183002/diff/60001/chromeos/dbus/dbus_threa...
chromeos/dbus/dbus_thread_manager.cc:53: #include
"components/device_event_log/device_event_log.h"
We shouldn't need this now.

https://codereview.chromium.org/919183002/diff/60001/chromeos/dbus/dbus_threa...
chromeos/dbus/dbus_thread_manager.cc:97: device_event_log::Shutdown();
This needs to move also.

https://codereview.chromium.org/919183002/diff/60001/chromeos/login/DEPS
File chromeos/login/DEPS (right):

https://codereview.chromium.org/919183002/diff/60001/chromeos/login/DEPS#newc...
chromeos/login/DEPS:2: "+components/device_event_log",
Just in chromeos/DEPS

https://codereview.chromium.org/919183002/diff/60001/chromeos/network/DEPS
File chromeos/network/DEPS (right):

https://codereview.chromium.org/919183002/diff/60001/chromeos/network/DEPS#ne...
chromeos/network/DEPS:3: "+components/device_event_log",
Just in chromeos/DEPS

https://codereview.chromium.org/919183002/diff/60001/components/device_event_...
File components/device_event_log/device_event_log.h (right):

https://codereview.chromium.org/919183002/diff/60001/components/device_event_...
components/device_event_log/device_event_log.h:15: namespace device_event_log {
nit: I think it's more clear to leave the device_event_log scope below the
macros, which aren't actually scoped.

https://codereview.chromium.org/919183002/diff/60001/components/device_event_...
File components/device_event_log/device_event_log_impl.cc (right):

https://codereview.chromium.org/919183002/diff/60001/components/device_event_...
components/device_event_log/device_event_log_impl.cc:392: if (filedesc) {
nit: if (!filedesc) return;

[Reilly Grant (use Gerrit), 2015-02-17 21:13:46]

https://codereview.chromium.org/919183002/diff/60001/chrome/browser/browser_p...
File chrome/browser/browser_process_impl.cc (right):

https://codereview.chromium.org/919183002/diff/60001/chrome/browser/browser_p...
chrome/browser/browser_process_impl.cc:190: device_event_log::Initialize(0 /*
default max entries */);
On 2015/02/17 19:24:35, stevenjb wrote:
> I think this would actually be better in chrome_browser_main_parts,
specifically
> ChromeBrowserMainParts::PreMainMessageLoopStart, since this doesn't have
> anything to do with the "browser" per-se, and so that it is available earlier
> on, e.g. for ChromeOS which does a lot of initialization (which may include
> logging) in PostMainMessageLoopStart.
> 

Done.

https://codereview.chromium.org/919183002/diff/60001/chromeos/dbus/DEPS
File chromeos/dbus/DEPS (right):

https://codereview.chromium.org/919183002/diff/60001/chromeos/dbus/DEPS#newcode2
chromeos/dbus/DEPS:2: "+components/device_event_log",
On 2015/02/17 19:24:35, stevenjb wrote:
> This can just be in chromeos/DEPS since chromeos/network also depends on it,
and
> others may start to.

Done.

https://codereview.chromium.org/919183002/diff/60001/chromeos/dbus/dbus_threa...
File chromeos/dbus/dbus_thread_manager.cc (right):

https://codereview.chromium.org/919183002/diff/60001/chromeos/dbus/dbus_threa...
chromeos/dbus/dbus_thread_manager.cc:53: #include
"components/device_event_log/device_event_log.h"
On 2015/02/17 19:24:35, stevenjb wrote:
> We shouldn't need this now.
> 

Done.

https://codereview.chromium.org/919183002/diff/60001/chromeos/dbus/dbus_threa...
chromeos/dbus/dbus_thread_manager.cc:97: device_event_log::Shutdown();
On 2015/02/17 19:24:35, stevenjb wrote:
> This needs to move also.

Done.

https://codereview.chromium.org/919183002/diff/60001/chromeos/login/DEPS
File chromeos/login/DEPS (right):

https://codereview.chromium.org/919183002/diff/60001/chromeos/login/DEPS#newc...
chromeos/login/DEPS:2: "+components/device_event_log",
On 2015/02/17 19:24:35, stevenjb wrote:
> Just in chromeos/DEPS

Done.

https://codereview.chromium.org/919183002/diff/60001/chromeos/network/DEPS
File chromeos/network/DEPS (right):

https://codereview.chromium.org/919183002/diff/60001/chromeos/network/DEPS#ne...
chromeos/network/DEPS:3: "+components/device_event_log",
On 2015/02/17 19:24:35, stevenjb wrote:
> Just in chromeos/DEPS

Done.

https://codereview.chromium.org/919183002/diff/60001/components/device_event_...
File components/device_event_log/device_event_log.h (right):

https://codereview.chromium.org/919183002/diff/60001/components/device_event_...
components/device_event_log/device_event_log.h:15: namespace device_event_log {
On 2015/02/17 19:24:35, stevenjb wrote:
> nit: I think it's more clear to leave the device_event_log scope below the
> macros, which aren't actually scoped.

Done.

[stevenjb, 2015-02-17 21:29:51]

lgtm with one change

https://codereview.chromium.org/919183002/diff/80001/chrome/browser/chrome_br...
File chrome/browser/chrome_browser_main.cc (right):

https://codereview.chromium.org/919183002/diff/80001/chrome/browser/chrome_br...
chrome/browser/chrome_browser_main.cc:1698: device_event_log::Shutdown();
Lets do this in PostDestryThreads which matches where we currently do it in
Chrome OS and shouldn't cause any problems.

[Reilly Grant (use Gerrit), 2015-02-17 21:35:26]

New patchsets have been uploaded after l-g-t-m from stevenjb@chromium.org

[Reilly Grant (use Gerrit), 2015-02-17 21:40:53]

reillyg@chromium.org changed reviewers:
+ blundell@chromium.org, scheib@chromium.org, sky@chromium.org,
ttuttle@chromium.org

[Reilly Grant (use Gerrit), 2015-02-17 21:40:54]

Adding sky@ for //ash and //chrome, scheib@ for //extensions, blundell@ for
//components and ttuttle@ for the //net dependency in
components/device_event_log.

[scheib, 2015-02-17 21:47:13]

extensions/extensions.gyp lgtm

[blundell, 2015-02-18 10:37:18]

//components lgtm

[sky, 2015-02-19 16:17:09]

From the bug it seems as though you want to enable chrome://device-log on all
platforms with the hopes that developers will use it. Is there a project that
wants this functionality now? I want to make sure that the added
code/complexity/size has a real user. If this went through eng-review that would
make me happy too.

[Reilly Grant (use Gerrit), 2015-02-19 23:45:18]

On 2015/02/19 16:17:09, sky wrote:
> From the bug it seems as though you want to enable chrome://device-log on all
> platforms with the hopes that developers will use it. Is there a project that
> wants this functionality now? I want to make sure that the added
> code/complexity/size has a real user. If this went through eng-review that
would
> make me happy too.

I am the developer that wants to use it. The next step is landing patchs such as
this one (https://codereview.chromium.org/941063003/) that moves logs from
device/usb that are currently hidden behind VLOG(1) into chrome://device-log.
Making these logs accessible in the field will make it a lot easier for app
developers as otherwise hardware behavior can be quite opaque.

[sky, 2015-02-20 17:59:24]

Ok, LGTM

[Deprecated (see juliatuttle), 2015-02-20 20:14:43]

On 2015/02/17 21:40:54, reillyg wrote:
> Adding sky@ for //ash and //chrome, scheib@ for //extensions, blundell@ for
> //components and ttuttle@ for the //net dependency in
> components/device_event_log.

net dep lgtm. Shame that you need it for that one function.

[Reilly Grant (use Gerrit), 2015-02-20 20:37:31]

The CQ bit was checked by reillyg@chromium.org

[commit-bot: I haz the power, 2015-02-20 20:39:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/919183002/100001

[commit-bot: I haz the power, 2015-02-20 21:38:53]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2015-02-20 21:39:20]

Patchset 4 (id:??) landed as
https://crrev.com/4a84927b28b6b55d0ef9e4df59d2e1a14b2b3de2
Cr-Commit-Position: refs/heads/master@{#317406}