Stub for web-base client for DevTools bridge and tests

Stub for web-base client for DevTools bridge and tests

This patch mostly consists of testing environment for web WebClient. While
WebClient intended to be base for a new DeviceProvider in DevTools frontend
(https://codereview.chromium.org/720133002/) it is convinient to run
tests on android. It lets to test the client directly against server
counterpart avoiding flakiness of network and cloud services.

TBR=jochen@chromium.org
BUG=383418
Committed: https://crrev.com/ff7a801dfddaa22643accaf3e0df192470fb7a7b
Cr-Commit-Position: refs/heads/master@{#305131}

[SeRya, 2014-11-20 13:26:43]

serya@chromium.org changed reviewers:
+ dgozman@chromium.org, mnaganov@chromium.org

[SeRya, 2014-11-20 13:26:44]

PTAL

[mnaganov (inactive), 2014-11-20 19:02:42]

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/a...
File components/devtools_bridge/android/client/javatests/AndroidManifest.xml
(right):

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/a...
components/devtools_bridge/android/client/javatests/AndroidManifest.xml:12:
<uses-permission android:name="android.permission.ACCESS_COARSE_LOCATION"/>
You probably not need most of the permissions here. If you will not be invoking
Chrome code that actually uses them, everything should still work.

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/a...
components/devtools_bridge/android/client/javatests/AndroidManifest.xml:31:
<application android:name=".TestApplication">
I think you may also safely add android:debuggable="true" for Java debuggability
on non-rooted devices.

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/a...
File
components/devtools_bridge/android/client/javatests/src/org/chromium/components/devtools_bridge/WebClientTest.java
(right):

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/a...
components/devtools_bridge/android/client/javatests/src/org/chromium/components/devtools_bridge/WebClientTest.java:27:
protected void setUp() throws Exception {
No need to override if you only call super.

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/a...
components/devtools_bridge/android/client/javatests/src/org/chromium/components/devtools_bridge/WebClientTest.java:35:
} catch (ProcessInitException e) {
You can avoid catching exceptions yourself, if you just add "throws Exception"
to your test methods. This way, the testing framework will automatically log the
exception and mark the test run as failed. You will only need to catch
exceptions if you really intend to handle them in tests.

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/a...
File
components/devtools_bridge/android/client/javatests/src/org/chromium/components/devtools_bridge/browsertests/TestApplication.java
(right):

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/a...
components/devtools_bridge/android/client/javatests/src/org/chromium/components/devtools_bridge/browsertests/TestApplication.java:34:
UmaUtils.recordMainEntryPointTime();
Why do you need UMA here?

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/a...
File components/devtools_bridge/android/session_dependency_factory_android.h
(right):

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/a...
components/devtools_bridge/android/session_dependency_factory_android.h:9:
#include "jni.h"
I think it should be "#include <jni.h>"

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/c...
File components/devtools_bridge/client/web_client.cc (right):

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/c...
components/devtools_bridge/client/web_client.cc:1: // Copyright (c) 2014 The
Chromium Authors. All rights reserved.
nit: remove "(c)"

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/c...
File components/devtools_bridge/client/web_client.h (right):

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/c...
components/devtools_bridge/client/web_client.h:1: // Copyright (c) 2012 The
Chromium Authors. All rights reserved.
nit: "(c) 2012" -> "2014"

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/t...
File components/devtools_bridge/test/android/client/web_client_android.cc
(right):

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/t...
components/devtools_bridge/test/android/client/web_client_android.cc:1: //
Copyright (c) 2014 The Chromium Authors. All rights reserved.
nit: remove "(c)"

[SeRya, 2014-11-20 21:10:39]

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/a...
File components/devtools_bridge/android/client/javatests/AndroidManifest.xml
(right):

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/a...
components/devtools_bridge/android/client/javatests/AndroidManifest.xml:12:
<uses-permission android:name="android.permission.ACCESS_COARSE_LOCATION"/>
On 2014/11/20 19:02:41, mnaganov (cr) wrote:
> You probably not need most of the permissions here. If you will not be
invoking
> Chrome code that actually uses them, everything should still work.

removed some permissions

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/a...
components/devtools_bridge/android/client/javatests/AndroidManifest.xml:31:
<application android:name=".TestApplication">
On 2014/11/20 19:02:41, mnaganov (cr) wrote:
> I think you may also safely add android:debuggable="true" for Java
debuggability
> on non-rooted devices.

Done.

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/a...
File
components/devtools_bridge/android/client/javatests/src/org/chromium/components/devtools_bridge/WebClientTest.java
(right):

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/a...
components/devtools_bridge/android/client/javatests/src/org/chromium/components/devtools_bridge/WebClientTest.java:27:
protected void setUp() throws Exception {
On 2014/11/20 19:02:42, mnaganov (cr) wrote:
> No need to override if you only call super.

Done.

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/a...
components/devtools_bridge/android/client/javatests/src/org/chromium/components/devtools_bridge/WebClientTest.java:35:
} catch (ProcessInitException e) {
On 2014/11/20 19:02:42, mnaganov (cr) wrote:
> You can avoid catching exceptions yourself, if you just add "throws Exception"
> to your test methods. This way, the testing framework will automatically log
the
> exception and mark the test run as failed. You will only need to catch
> exceptions if you really intend to handle them in tests.

It would be so unless this method is used not inside
ThreadUtils.runOnUiThreadBlocking.

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/a...
File
components/devtools_bridge/android/client/javatests/src/org/chromium/components/devtools_bridge/browsertests/TestApplication.java
(right):

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/a...
components/devtools_bridge/android/client/javatests/src/org/chromium/components/devtools_bridge/browsertests/TestApplication.java:34:
UmaUtils.recordMainEntryPointTime();
On 2014/11/20 19:02:42, mnaganov (cr) wrote:
> Why do you need UMA here?

removed.

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/a...
File components/devtools_bridge/android/session_dependency_factory_android.h
(right):

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/a...
components/devtools_bridge/android/session_dependency_factory_android.h:9:
#include "jni.h"
On 2014/11/20 19:02:42, mnaganov (cr) wrote:
> I think it should be "#include <jni.h>"

Done.

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/c...
File components/devtools_bridge/client/web_client.cc (right):

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/c...
components/devtools_bridge/client/web_client.cc:1: // Copyright (c) 2014 The
Chromium Authors. All rights reserved.
On 2014/11/20 19:02:42, mnaganov (cr) wrote:
> nit: remove "(c)"

Done.

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/c...
File components/devtools_bridge/client/web_client.h (right):

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/c...
components/devtools_bridge/client/web_client.h:1: // Copyright (c) 2012 The
Chromium Authors. All rights reserved.
On 2014/11/20 19:02:42, mnaganov (cr) wrote:
> nit: "(c) 2012" -> "2014"

Done.

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/t...
File components/devtools_bridge/test/android/client/web_client_android.cc
(right):

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/t...
components/devtools_bridge/test/android/client/web_client_android.cc:1: //
Copyright (c) 2014 The Chromium Authors. All rights reserved.
On 2014/11/20 19:02:42, mnaganov (cr) wrote:
> nit: remove "(c)"

Done.

[mnaganov (inactive), 2014-11-20 21:54:49]

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/a...
File
components/devtools_bridge/android/client/javatests/src/org/chromium/components/devtools_bridge/WebClientTest.java
(right):

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/a...
components/devtools_bridge/android/client/javatests/src/org/chromium/components/devtools_bridge/WebClientTest.java:35:
} catch (ProcessInitException e) {
On 2014/11/20 21:10:38, SeRya wrote:
> On 2014/11/20 19:02:42, mnaganov (cr) wrote:
> > You can avoid catching exceptions yourself, if you just add "throws
Exception"
> > to your test methods. This way, the testing framework will automatically log
> the
> > exception and mark the test run as failed. You will only need to catch
> > exceptions if you really intend to handle them in tests.
> 
> It would be so unless this method is used not inside
> ThreadUtils.runOnUiThreadBlocking.

I'm not convinced. If the operation has thrown an exception, FutureTask will
raise ExecutionException when retrieving the result on the caller's thread via
get() (and this is what runOnUiThreadBlocking uses). See
http://developer.android.com/reference/java/util/concurrent/FutureTask.html#s...

[SeRya, 2014-11-20 22:37:24]

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/a...
File
components/devtools_bridge/android/client/javatests/src/org/chromium/components/devtools_bridge/WebClientTest.java
(right):

https://codereview.chromium.org/736363002/diff/1/components/devtools_bridge/a...
components/devtools_bridge/android/client/javatests/src/org/chromium/components/devtools_bridge/WebClientTest.java:35:
} catch (ProcessInitException e) {
On 2014/11/20 21:54:48, mnaganov (cr) wrote:
> On 2014/11/20 21:10:38, SeRya wrote:
> > On 2014/11/20 19:02:42, mnaganov (cr) wrote:
> > > You can avoid catching exceptions yourself, if you just add "throws
> Exception"
> > > to your test methods. This way, the testing framework will automatically
log
> > the
> > > exception and mark the test run as failed. You will only need to catch
> > > exceptions if you really intend to handle them in tests.
> > 
> > It would be so unless this method is used not inside
> > ThreadUtils.runOnUiThreadBlocking.
> 
> I'm not convinced. If the operation has thrown an exception, FutureTask will
> raise ExecutionException when retrieving the result on the caller's thread via
> get() (and this is what runOnUiThreadBlocking uses). See
>
http://developer.android.com/reference/java/util/concurrent/FutureTask.html#s...

Done.

[mnaganov (inactive), 2014-11-20 22:53:07]

lgtm

[SeRya, 2014-11-20 23:30:05]

Patchset #4 (id:60001) has been deleted

[SeRya, 2014-11-20 23:31:50]

The CQ bit was checked by serya@chromium.org

[commit-bot: I haz the power, 2014-11-20 23:32:53]

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

[commit-bot: I haz the power, 2014-11-20 23:40:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-11-20 23:40:05]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[SeRya, 2014-11-20 23:52:44]

The CQ bit was checked by serya@chromium.org

[commit-bot: I haz the power, 2014-11-20 23:53:59]

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

[commit-bot: I haz the power, 2014-11-21 01:10:22]

Committed patchset #5 (id:100001)

[commit-bot: I haz the power, 2014-11-21 01:11:07]

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

[SeRya, 2014-12-15 09:32:25]

A revert of this CL (patchset #5 id:100001) has been created in
https://codereview.chromium.org/802293002/ by serya@chromium.org.

The reason for reverting is: Reverting since now DevToolsBridgeClient
(https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/dev...)
is that WebClient supposed to be. All further code will be placed there..