Introduced AndroidDeviceProvider to simplify testing.

Introduced AndroidDeviceProvider to simplify testing.

TBR=pfeldman
BUG=308514
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=230374

[Vladislav Kaznacheev, 2013-10-16 11:27:04]

https://chromiumcodereview.appspot.com/26568004/diff/2001/chrome/browser/devt...
File chrome/browser/devtools/adb_web_socket.cc (right):

https://chromiumcodereview.appspot.com/26568004/diff/2001/chrome/browser/devt...
chrome/browser/devtools/adb_web_socket.cc:11: #include "net/base/io_buffer.h"
Is this include necessary?

https://chromiumcodereview.appspot.com/26568004/diff/2001/chrome/browser/devt...
File chrome/browser/devtools/devtools_adb_bridge.h (right):

https://chromiumcodereview.appspot.com/26568004/diff/2001/chrome/browser/devt...
chrome/browser/devtools/devtools_adb_bridge.h:85:
RemotePage(scoped_refptr<RefCountedAdbThread> adb_thread,
This patch is obviously not rebased on the latest version.

https://chromiumcodereview.appspot.com/26568004/diff/2001/chrome/browser/devt...
chrome/browser/devtools/devtools_adb_bridge.h:217: explicit
DevToolsAdbBridge(Profile* profile,
explicit is no longer needed

https://chromiumcodereview.appspot.com/26568004/diff/2001/chrome/browser/devt...
chrome/browser/devtools/devtools_adb_bridge.h:218: const DeviceProviders&
device_providers_);
indent

https://chromiumcodereview.appspot.com/26568004/diff/2001/chrome/browser/devt...
File chrome/browser/devtools/devtools_device_provider.h (right):

https://chromiumcodereview.appspot.com/26568004/diff/2001/chrome/browser/devt...
chrome/browser/devtools/devtools_device_provider.h:16: typedef
base::Callback<void(int result, net::StreamSocket*)> SocketCallback;
Please do not declare these in a global namespace

https://chromiumcodereview.appspot.com/26568004/diff/2001/chrome/browser/devt...
chrome/browser/devtools/devtools_device_provider.h:18: class RefCountedAdbThread
: public base::RefCounted<RefCountedAdbThread> {
This deserves a separate header

[Dmitry Zvorygin, 2013-10-17 16:19:36]

Please take a look.

https://codereview.chromium.org/26568004/diff/2001/chrome/browser/devtools/ad...
File chrome/browser/devtools/adb_web_socket.cc (right):

https://codereview.chromium.org/26568004/diff/2001/chrome/browser/devtools/ad...
chrome/browser/devtools/adb_web_socket.cc:11: #include "net/base/io_buffer.h"
On 2013/10/16 11:27:04, Vladislav Kaznacheev wrote:
> Is this include necessary?

Not necessary actually.

https://codereview.chromium.org/26568004/diff/2001/chrome/browser/devtools/de...
File chrome/browser/devtools/devtools_adb_bridge.h (right):

https://codereview.chromium.org/26568004/diff/2001/chrome/browser/devtools/de...
chrome/browser/devtools/devtools_adb_bridge.h:85:
RemotePage(scoped_refptr<RefCountedAdbThread> adb_thread,
On 2013/10/16 11:27:04, Vladislav Kaznacheev wrote:
> This patch is obviously not rebased on the latest version.

Done.

https://codereview.chromium.org/26568004/diff/2001/chrome/browser/devtools/de...
chrome/browser/devtools/devtools_adb_bridge.h:217: explicit
DevToolsAdbBridge(Profile* profile,
On 2013/10/16 11:27:04, Vladislav Kaznacheev wrote:
> explicit is no longer needed

Done.

https://codereview.chromium.org/26568004/diff/2001/chrome/browser/devtools/de...
chrome/browser/devtools/devtools_adb_bridge.h:218: const DeviceProviders&
device_providers_);
On 2013/10/16 11:27:04, Vladislav Kaznacheev wrote:
> indent

Done.

https://codereview.chromium.org/26568004/diff/2001/chrome/browser/devtools/de...
File chrome/browser/devtools/devtools_device_provider.h (right):

https://codereview.chromium.org/26568004/diff/2001/chrome/browser/devtools/de...
chrome/browser/devtools/devtools_device_provider.h:16: typedef
base::Callback<void(int result, net::StreamSocket*)> SocketCallback;
On 2013/10/16 11:27:04, Vladislav Kaznacheev wrote:
> Please do not declare these in a global namespace

Done.

You have had them previously declared in global namespace in
devtools_adb_bridge.h

https://codereview.chromium.org/26568004/diff/2001/chrome/browser/devtools/de...
chrome/browser/devtools/devtools_device_provider.h:18: class RefCountedAdbThread
: public base::RefCounted<RefCountedAdbThread> {
On 2013/10/16 11:27:04, Vladislav Kaznacheev wrote:
> This deserves a separate header

Done.

[Vladislav Kaznacheev, 2013-10-17 20:17:36]

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
File chrome/browser/devtools/devtools_adb_bridge.cc (right):

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
chrome/browser/devtools/devtools_adb_bridge.cc:923: DeviceProviders
device_providers;
Here is what I do not understand:

The whole point of this change is to make it possible for DevToolsAdbBridge to
use an externally provided list of device providers so that in a test we can
give it a special test-oriented provider. How is this going to work if you
explicitly create live providers here?

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
chrome/browser/devtools/devtools_adb_bridge.cc:927: new
UsbDeviceProvider(profile_, RefCountedAdbThread::GetInstance()));
It's OK to call RefCountedAdbThread::GetInstance() in UsbDeviceProvider
constructor.

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
chrome/browser/devtools/devtools_adb_bridge.cc:930: new
AdbDeviceProvider(RefCountedAdbThread::GetInstance()));
Same here.

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
File chrome/browser/devtools/devtools_adb_bridge.h (right):

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
chrome/browser/devtools/devtools_adb_bridge.h:14: #include
"chrome/browser/devtools/adb/android_usb_device.h"
Is this header still required?

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
File chrome/browser/devtools/devtools_device_provider.cc (right):

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
chrome/browser/devtools/devtools_device_provider.cc:74: // AdbDeviceImpl
--------------------------------------------------------------
Please hide *Impl classes in an anonymous namespace at the top of the file

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
chrome/browser/devtools/devtools_device_provider.cc:116: void
DevToolsDeviceProvider::RunCallbackOnUIThread(
This method could be static. 
More over, it could live in a cc file outside of any class.
Even cooler, you could try writing this at the call sites:

BrowserThread::PostTask(
    BrowserThread::UI, 
    FROM_HERE,
    base::Bind(&QueryDeviceCallback::Run, callback, result));

I this this should work.

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
chrome/browser/devtools/devtools_device_provider.cc:294: #if
defined(DEBUG_DEVTOOLS)
Strange ident here and inside the #if

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
File chrome/browser/devtools/devtools_device_provider.h (right):

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
chrome/browser/devtools/devtools_device_provider.h:63: class
DevToolsDeviceProvider
AndroidDeviceProvider is much more suitable name

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
chrome/browser/devtools/devtools_device_provider.h:71: 
extra blank line

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
chrome/browser/devtools/devtools_device_provider.h:85: class UsbDeviceProvider:
public DevToolsDeviceProvider {
It could be a good idea to hide the two implementations in the cc file and add a
static method to the base provider class that returns a list of providers.

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/re...
File chrome/browser/devtools/refcounted_adb_thread.cc (right):

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/re...
chrome/browser/devtools/refcounted_adb_thread.cc:13: //
DevToolsAdbBridge::RefCountedAdbThread -------------------------------------
Please delete this line

[Dmitry Zvorygin, 2013-10-21 07:40:29]

Please take a look again.

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
File chrome/browser/devtools/devtools_adb_bridge.cc (right):

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
chrome/browser/devtools/devtools_adb_bridge.cc:923: DeviceProviders
device_providers;
Done.

Overengineered a little. Earlier devices were passed to DevToolsAdbBridge.

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
chrome/browser/devtools/devtools_adb_bridge.cc:927: new
UsbDeviceProvider(profile_, RefCountedAdbThread::GetInstance()));
On 2013/10/17 20:17:37, Vladislav Kaznacheev wrote:
> It's OK to call RefCountedAdbThread::GetInstance() in UsbDeviceProvider
> constructor.

Done.

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
chrome/browser/devtools/devtools_adb_bridge.cc:930: new
AdbDeviceProvider(RefCountedAdbThread::GetInstance()));
On 2013/10/17 20:17:37, Vladislav Kaznacheev wrote:
> Same here.

Done.

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
File chrome/browser/devtools/devtools_adb_bridge.h (right):

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
chrome/browser/devtools/devtools_adb_bridge.h:14: #include
"chrome/browser/devtools/adb/android_usb_device.h"
On 2013/10/17 20:17:37, Vladislav Kaznacheev wrote:
> Is this header still required?

Done.

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
File chrome/browser/devtools/devtools_device_provider.cc (right):

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
chrome/browser/devtools/devtools_device_provider.cc:74: // AdbDeviceImpl
--------------------------------------------------------------
On 2013/10/17 20:17:37, Vladislav Kaznacheev wrote:
> Please hide *Impl classes in an anonymous namespace at the top of the file

Done.

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
chrome/browser/devtools/devtools_device_provider.cc:116: void
DevToolsDeviceProvider::RunCallbackOnUIThread(
On 2013/10/17 20:17:37, Vladislav Kaznacheev wrote:
> This method could be static. 
> More over, it could live in a cc file outside of any class.
> Even cooler, you could try writing this at the call sites:
> 
> BrowserThread::PostTask(
>     BrowserThread::UI, 
>     FROM_HERE,
>     base::Bind(&QueryDeviceCallback::Run, callback, result));
> 
> I this this should work.

This requires callback to be refcounted, so it actually doesn't work.

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
chrome/browser/devtools/devtools_device_provider.cc:294: #if
defined(DEBUG_DEVTOOLS)
On 2013/10/17 20:17:37, Vladislav Kaznacheev wrote:
> Strange ident here and inside the #if

Done.

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
File chrome/browser/devtools/devtools_device_provider.h (right):

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
chrome/browser/devtools/devtools_device_provider.h:63: class
DevToolsDeviceProvider
On 2013/10/17 20:17:37, Vladislav Kaznacheev wrote:
> AndroidDeviceProvider is much more suitable name

Done.

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
chrome/browser/devtools/devtools_device_provider.h:71: 
On 2013/10/17 20:17:37, Vladislav Kaznacheev wrote:
> extra blank line

Done.

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/de...
chrome/browser/devtools/devtools_device_provider.h:85: class UsbDeviceProvider:
public DevToolsDeviceProvider {
On 2013/10/17 20:17:37, Vladislav Kaznacheev wrote:
> It could be a good idea to hide the two implementations in the cc file and add
a
> static method to the base provider class that returns a list of providers.

Done.

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/re...
File chrome/browser/devtools/refcounted_adb_thread.cc (right):

https://codereview.chromium.org/26568004/diff/9001/chrome/browser/devtools/re...
chrome/browser/devtools/refcounted_adb_thread.cc:13: //
DevToolsAdbBridge::RefCountedAdbThread -------------------------------------
On 2013/10/17 20:17:37, Vladislav Kaznacheev wrote:
> Please delete this line

Done.

[Vladislav Kaznacheev, 2013-10-21 10:09:01]

https://chromiumcodereview.appspot.com/26568004/diff/19001/chrome/browser/dev...
File chrome/browser/devtools/android_device_provider.cc (right):

https://chromiumcodereview.appspot.com/26568004/diff/19001/chrome/browser/dev...
chrome/browser/devtools/android_device_provider.cc:74: namespace {
Please move the namespace to the top of the file and try to hide as many
constants as possible (looks like all of them could go there)

https://chromiumcodereview.appspot.com/26568004/diff/19001/chrome/browser/dev...
chrome/browser/devtools/android_device_provider.cc:326: class
AdbCountDevicesCommand :public base::RefCountedThreadSafe<
This should live in the anonymous namespace as well

https://chromiumcodereview.appspot.com/26568004/diff/19001/chrome/browser/dev...
File chrome/browser/devtools/devtools_adb_bridge.h (right):

https://chromiumcodereview.appspot.com/26568004/diff/19001/chrome/browser/dev...
chrome/browser/devtools/devtools_adb_bridge.h:83: class RemotePage : public
base::RefCounted<RemotePage> {
Strange indent

[Dmitry Zvorygin, 2013-10-21 10:22:35]

Please take a look again.

https://chromiumcodereview.appspot.com/26568004/diff/19001/chrome/browser/dev...
File chrome/browser/devtools/android_device_provider.cc (right):

https://chromiumcodereview.appspot.com/26568004/diff/19001/chrome/browser/dev...
chrome/browser/devtools/android_device_provider.cc:74: namespace {
On 2013/10/21 10:09:01, Vladislav Kaznacheev wrote:
> Please move the namespace to the top of the file and try to hide as many
> constants as possible (looks like all of them could go there)

Done.

https://chromiumcodereview.appspot.com/26568004/diff/19001/chrome/browser/dev...
File chrome/browser/devtools/devtools_adb_bridge.h (right):

https://chromiumcodereview.appspot.com/26568004/diff/19001/chrome/browser/dev...
chrome/browser/devtools/devtools_adb_bridge.h:83: class RemotePage : public
base::RefCounted<RemotePage> {
On 2013/10/21 10:09:01, Vladislav Kaznacheev wrote:
> Strange indent

Typo. Done.

[Dmitry Zvorygin, 2013-10-21 11:56:53]

Vlad gave verbal LGTM.

Pavel, please take a look.

[Dmitry Zvorygin, 2013-10-21 14:31:30]

Need OWNERS LGTM.

[Vladislav Kaznacheev, 2013-10-22 08:34:34]

Before this lands I would like to see the patch with the browser test that makes
use of these changes.

https://chromiumcodereview.appspot.com/26568004/diff/89001/chrome/browser/dev...
File chrome/browser/devtools/android_device_provider.cc (right):

https://chromiumcodereview.appspot.com/26568004/diff/89001/chrome/browser/dev...
chrome/browser/devtools/android_device_provider.cc:214: // AdbDeviceProvider
-------------------------------------------
The order of sections is irregular. Should be either:
AdbDeviceImpl, UsbDeviceImpl, AdbDeviceProvider, UsbDeviceProvider
or:
AdbDeviceImpl, AdbDeviceProvider, UsbDeviceImpl, UsbDeviceProvider

https://chromiumcodereview.appspot.com/26568004/diff/89001/chrome/browser/dev...
chrome/browser/devtools/android_device_provider.cc:325: class
AdbCountDevicesCommand :public base::RefCountedThreadSafe<
space after :
strange indent, better break the line as in DevToolsAdbBridge declaration

https://chromiumcodereview.appspot.com/26568004/diff/89001/chrome/browser/dev...
chrome/browser/devtools/android_device_provider.cc:325: class
AdbCountDevicesCommand :public base::RefCountedThreadSafe<
I know you are just moving this class from another file, but I have a suspicion
that it does not do anything that a simple callback cannot do. Something like:
adb_thread_->message_loop()->PostTask(
   FROM_HERE,
   base::Bind(&AdbClientSocket::AdbQuery,
              kAdbPort,
              kHostDevicesCommand,
              base::Bind(&ReceivedDevicesList, callback)))
  

Please add a TODO to remove this class.

https://chromiumcodereview.appspot.com/26568004/diff/89001/chrome/browser/dev...
chrome/browser/devtools/android_device_provider.cc:394: void
AndroidDeviceProvider::RunCallbackOnUIThread(
This can still be a static non-member function

https://chromiumcodereview.appspot.com/26568004/diff/89001/chrome/browser/dev...
File chrome/browser/devtools/android_device_provider.h (right):

https://chromiumcodereview.appspot.com/26568004/diff/89001/chrome/browser/dev...
chrome/browser/devtools/android_device_provider.h:69: typedef
base::Callback<void(const AndroidDevices&)> QueryDeviceCallback;
QueryDevicesCallback name makes more sense

https://chromiumcodereview.appspot.com/26568004/diff/89001/chrome/browser/dev...
chrome/browser/devtools/android_device_provider.h:86: friend struct
Please put the friend declarations right at the top of the protected section.

https://chromiumcodereview.appspot.com/26568004/diff/89001/chrome/browser/dev...
File chrome/browser/devtools/devtools_adb_bridge.h (right):

https://chromiumcodereview.appspot.com/26568004/diff/89001/chrome/browser/dev...
chrome/browser/devtools/devtools_adb_bridge.h:219: explicit
DevToolsAdbBridge(DeviceProviders device_providers);
Why do we need this constructor if we have set_device_providers?

https://chromiumcodereview.appspot.com/26568004/diff/89001/chrome/chrome.gyp
File chrome/chrome.gyp (right):

https://chromiumcodereview.appspot.com/26568004/diff/89001/chrome/chrome.gyp#...
chrome/chrome.gyp:180: 'browser/devtools/android_device_provider.cc',
Lets rename to android_device.cc/h

[Dmitry Zvorygin, 2013-10-22 09:07:55]

https://chromiumcodereview.appspot.com/26568004/diff/89001/chrome/browser/dev...
File chrome/browser/devtools/android_device_provider.cc (right):

https://chromiumcodereview.appspot.com/26568004/diff/89001/chrome/browser/dev...
chrome/browser/devtools/android_device_provider.cc:214: // AdbDeviceProvider
-------------------------------------------
On 2013/10/22 08:34:35, Vladislav Kaznacheev wrote:
> The order of sections is irregular. Should be either:
> AdbDeviceImpl, UsbDeviceImpl, AdbDeviceProvider, UsbDeviceProvider
> or:
> AdbDeviceImpl, AdbDeviceProvider, UsbDeviceImpl, UsbDeviceProvider

Done.

https://chromiumcodereview.appspot.com/26568004/diff/89001/chrome/browser/dev...
chrome/browser/devtools/android_device_provider.cc:325: class
AdbCountDevicesCommand :public base::RefCountedThreadSafe<
On 2013/10/22 08:34:35, Vladislav Kaznacheev wrote:
> space after :
> strange indent, better break the line as in DevToolsAdbBridge declaration

Done.

https://chromiumcodereview.appspot.com/26568004/diff/89001/chrome/browser/dev...
chrome/browser/devtools/android_device_provider.cc:325: class
AdbCountDevicesCommand :public base::RefCountedThreadSafe<
On 2013/10/22 08:34:35, Vladislav Kaznacheev wrote:
> I know you are just moving this class from another file, but I have a
suspicion
> that it does not do anything that a simple callback cannot do. Something like:
> adb_thread_->message_loop()->PostTask(
>    FROM_HERE,
>    base::Bind(&AdbClientSocket::AdbQuery,
>               kAdbPort,
>               kHostDevicesCommand,
>               base::Bind(&ReceivedDevicesList, callback)))
>   
> 
> Please add a TODO to remove this class.

Done.

https://chromiumcodereview.appspot.com/26568004/diff/89001/chrome/browser/dev...
chrome/browser/devtools/android_device_provider.cc:394: void
AndroidDeviceProvider::RunCallbackOnUIThread(
On 2013/10/22 08:34:35, Vladislav Kaznacheev wrote:
> This can still be a static non-member function

Done.

https://chromiumcodereview.appspot.com/26568004/diff/89001/chrome/browser/dev...
File chrome/browser/devtools/android_device_provider.h (right):

https://chromiumcodereview.appspot.com/26568004/diff/89001/chrome/browser/dev...
chrome/browser/devtools/android_device_provider.h:69: typedef
base::Callback<void(const AndroidDevices&)> QueryDeviceCallback;
On 2013/10/22 08:34:35, Vladislav Kaznacheev wrote:
> QueryDevicesCallback name makes more sense

Done.

https://chromiumcodereview.appspot.com/26568004/diff/89001/chrome/browser/dev...
chrome/browser/devtools/android_device_provider.h:86: friend struct
On 2013/10/22 08:34:35, Vladislav Kaznacheev wrote:
> Please put the friend declarations right at the top of the protected section.

Done.

https://chromiumcodereview.appspot.com/26568004/diff/89001/chrome/browser/dev...
File chrome/browser/devtools/devtools_adb_bridge.h (right):

https://chromiumcodereview.appspot.com/26568004/diff/89001/chrome/browser/dev...
chrome/browser/devtools/devtools_adb_bridge.h:219: explicit
DevToolsAdbBridge(DeviceProviders device_providers);
On 2013/10/22 08:34:35, Vladislav Kaznacheev wrote:
> Why do we need this constructor if we have set_device_providers?

Done.

https://chromiumcodereview.appspot.com/26568004/diff/89001/chrome/chrome.gyp
File chrome/chrome.gyp (right):

https://chromiumcodereview.appspot.com/26568004/diff/89001/chrome/chrome.gyp#...
chrome/chrome.gyp:180: 'browser/devtools/android_device_provider.cc',
On 2013/10/22 08:34:35, Vladislav Kaznacheev wrote:
> Lets rename to android_device.cc/h

Done.

[Vladislav Kaznacheev, 2013-10-22 12:42:18]

LGTM, but please hold off landing while we are reviewing the browser test patch

https://chromiumcodereview.appspot.com/26568004/diff/279001/chrome/browser/de...
File chrome/browser/devtools/devtools_adb_bridge.h (right):

https://chromiumcodereview.appspot.com/26568004/diff/279001/chrome/browser/de...
chrome/browser/devtools/devtools_adb_bridge.h:219: explicit DevToolsAdbBridge();
no need for explicit now

[Dmitry Zvorygin, 2013-10-22 15:01:33]

Please take a look.

https://codereview.chromium.org/26568004/diff/279001/chrome/browser/devtools/...
File chrome/browser/devtools/devtools_adb_bridge.h (right):

https://codereview.chromium.org/26568004/diff/279001/chrome/browser/devtools/...
chrome/browser/devtools/devtools_adb_bridge.h:219: explicit DevToolsAdbBridge();
On 2013/10/22 12:42:18, Vladislav Kaznacheev wrote:
> no need for explicit now

Done.

[Vladislav Kaznacheev, 2013-10-22 15:35:11]

lgtm

[commit-bot: I haz the power, 2013-10-22 15:35:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zvorygin@chromium.org/26568004/319001

[commit-bot: I haz the power, 2013-10-22 15:55:19]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[commit-bot: I haz the power, 2013-10-22 17:29:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zvorygin@chromium.org/26568004/319001

[commit-bot: I haz the power, 2013-10-22 18:06:02]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_clang.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...
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, 2013-10-22 19:11:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zvorygin@chromium.org/26568004/319001

[commit-bot: I haz the power, 2013-10-22 19:30:53]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_clang.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...
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, 2013-10-22 20:19:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zvorygin@chromium.org/26568004/319001

[commit-bot: I haz the power, 2013-10-22 20:38:08]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_clang_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...
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, 2013-10-23 05:47:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zvorygin@chromium.org/26568004/749001

[Dmitry Zvorygin, 2013-10-23 05:56:48]

Need OWNERS approval for

sky - chrome.gyp
pfeldman - inspect_ui.cc

[commit-bot: I haz the power, 2013-10-23 08:15:02]

List of reviewers changed. sky@chromium.org,pfeldman@chromium.org did a drive-by
without LGTM'ing!

[commit-bot: I haz the power, 2013-10-23 08:28:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zvorygin@chromium.org/26568004/749001

[commit-bot: I haz the power, 2013-10-23 08:30:51]

Change committed as 230374