Visual bench on native android

Visual bench on native android

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/030dc847dc497b011c417dc6694e8b6de290895f

[joshualitt, 2015-06-11 20:02:34]

joshualitt@google.com changed reviewers:
+ bsalomon@google.com, djsollen@google.com, joshualitt@google.com,
robertphillips@google.com

[joshualitt, 2015-06-11 20:02:34]

Derek for the android stuff, Gpu guys for gpu stuff.

[djsollen, 2015-06-11 20:32:00]

https://codereview.chromium.org/1164403002/diff/160001/gyp/most.gyp
File gyp/most.gyp (right):

https://codereview.chromium.org/1164403002/diff/160001/gyp/most.gyp#newcode46
gyp/most.gyp:46: 'android_system.gyp:VisualBench_APK',
through the dependency chain this will build visualbench even if skia_gpu == 0. 
I assume it isn't worth building that APK if there is no GPU support.

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
File platform_tools/android/gyp/skia_android.gypi (right):

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
platform_tools/android/gyp/skia_android.gypi:38: # that this copy requires us to
build VisualbenchApp after those
I don't think you meant to update this comment.

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
platform_tools/android/gyp/skia_android.gypi:155:
'<(PRODUCT_DIR)/../android/VisualBench/bin/VisualBench.apk',
update this to <(ANDROID_OUT)/bin/VisualBench.apk

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
File platform_tools/android/visualbench/AndroidManifest.xml (right):

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
platform_tools/android/visualbench/AndroidManifest.xml:16:
android:label="@string/app_name"
replace the quoted string contents with VisualBenchActivity and then you can
remove the strings.xml file as we don't ever plan on internationalizing our
strings. :)  I should do that for SampleApp too.

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
File platform_tools/android/visualbench/proguard-project.txt (right):

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
platform_tools/android/visualbench/proguard-project.txt:1: # To enable ProGuard
in your project, edit project.properties
We shouldn't need this file as we don't want to ever obfuscate the contents of
our build.

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
File platform_tools/android/visualbench/res/values/strings.xml (left):

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
platform_tools/android/visualbench/res/values/strings.xml:3: <string
name="app_name">HelloSkiaActivity</string>
remove this file.

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
File platform_tools/android/visualbench/src/com/skia/VisualBenchActivity.java
(right):

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
platform_tools/android/visualbench/src/com/skia/VisualBenchActivity.java:26:
getIntent().putExtra("hmm", "woah");
hmm?

[djsollen, 2015-06-11 20:35:43]

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
File platform_tools/android/visualbench/jni/main.cpp (right):

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
platform_tools/android/visualbench/jni/main.cpp:42: args.push_back("SampleApp");
VisualBench?

[bsalomon, 2015-06-11 20:55:33]

The parts I understand, lgtm.

[joshualitt, 2015-06-12 15:40:36]

feedback mostly incorporated

https://codereview.chromium.org/1164403002/diff/160001/gyp/most.gyp
File gyp/most.gyp (right):

https://codereview.chromium.org/1164403002/diff/160001/gyp/most.gyp#newcode46
gyp/most.gyp:46: 'android_system.gyp:VisualBench_APK',
On 2015/06/11 20:31:59, djsollen wrote:
> through the dependency chain this will build visualbench even if skia_gpu ==
0. 
> I assume it isn't worth building that APK if there is no GPU support.

Acknowledged.

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
File platform_tools/android/gyp/skia_android.gypi (right):

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
platform_tools/android/gyp/skia_android.gypi:38: # that this copy requires us to
build VisualbenchApp after those
On 2015/06/11 20:32:00, djsollen wrote:
> I don't think you meant to update this comment.

Acknowledged.

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
platform_tools/android/gyp/skia_android.gypi:155:
'<(PRODUCT_DIR)/../android/VisualBench/bin/VisualBench.apk',
On 2015/06/11 20:32:00, djsollen wrote:
> update this to <(ANDROID_OUT)/bin/VisualBench.apk

Hmm, for some reason gyp really doesn't like this at all.  I get an 'assertion
error'
AssertionError: ../../$!PRODUCT_DIR/android/VisualBench/bin/VisualBench.apk

Thoughts?

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
File platform_tools/android/visualbench/AndroidManifest.xml (right):

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
platform_tools/android/visualbench/AndroidManifest.xml:16:
android:label="@string/app_name"
On 2015/06/11 20:32:00, djsollen wrote:
> replace the quoted string contents with VisualBenchActivity and then you can
> remove the strings.xml file as we don't ever plan on internationalizing our
> strings. :)  I should do that for SampleApp too.

Acknowledged.

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
File platform_tools/android/visualbench/jni/main.cpp (right):

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
platform_tools/android/visualbench/jni/main.cpp:42: args.push_back("SampleApp");
On 2015/06/11 20:35:43, djsollen wrote:
> VisualBench?

Acknowledged.

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
File platform_tools/android/visualbench/proguard-project.txt (right):

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
platform_tools/android/visualbench/proguard-project.txt:1: # To enable ProGuard
in your project, edit project.properties
On 2015/06/11 20:32:00, djsollen wrote:
> We shouldn't need this file as we don't want to ever obfuscate the contents of
> our build.

Acknowledged.

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
File platform_tools/android/visualbench/res/values/strings.xml (left):

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
platform_tools/android/visualbench/res/values/strings.xml:3: <string
name="app_name">HelloSkiaActivity</string>
On 2015/06/11 20:32:00, djsollen wrote:
> remove this file.

Acknowledged.

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
File platform_tools/android/visualbench/src/com/skia/VisualBenchActivity.java
(right):

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
platform_tools/android/visualbench/src/com/skia/VisualBenchActivity.java:26:
getIntent().putExtra("hmm", "woah");
On 2015/06/11 20:32:00, djsollen wrote:
> hmm?

Acknowledged.

[djsollen, 2015-06-12 16:27:15]

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
File platform_tools/android/gyp/skia_android.gypi (right):

https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
platform_tools/android/gyp/skia_android.gypi:155:
'<(PRODUCT_DIR)/../android/VisualBench/bin/VisualBench.apk',
On 2015/06/12 15:40:35, joshualitt wrote:
> On 2015/06/11 20:32:00, djsollen wrote:
> > update this to <(ANDROID_OUT)/bin/VisualBench.apk
> 
> Hmm, for some reason gyp really doesn't like this at all.  I get an 'assertion
> error'
> AssertionError: ../../$!PRODUCT_DIR/android/VisualBench/bin/VisualBench.apk
> 
> Thoughts?

No idea why that is so feel free to leave as is.

https://codereview.chromium.org/1164403002/diff/200001/platform_tools/android...
File platform_tools/android/visualbench/res/values/strings.xml (right):

https://codereview.chromium.org/1164403002/diff/200001/platform_tools/android...
platform_tools/android/visualbench/res/values/strings.xml:1: <?xml version="1.0"
encoding="utf-8"?>
You can delete this file now.

[joshualitt, 2015-06-12 18:24:00]

On 2015/06/12 16:27:15, djsollen wrote:
>
https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
> File platform_tools/android/gyp/skia_android.gypi (right):
> 
>
https://codereview.chromium.org/1164403002/diff/160001/platform_tools/android...
> platform_tools/android/gyp/skia_android.gypi:155:
> '<(PRODUCT_DIR)/../android/VisualBench/bin/VisualBench.apk',
> On 2015/06/12 15:40:35, joshualitt wrote:
> > On 2015/06/11 20:32:00, djsollen wrote:
> > > update this to <(ANDROID_OUT)/bin/VisualBench.apk
> > 
> > Hmm, for some reason gyp really doesn't like this at all.  I get an
'assertion
> > error'
> > AssertionError: ../../$!PRODUCT_DIR/android/VisualBench/bin/VisualBench.apk
> > 
> > Thoughts?
> 
> No idea why that is so feel free to leave as is.
> 
>
https://codereview.chromium.org/1164403002/diff/200001/platform_tools/android...
> File platform_tools/android/visualbench/res/values/strings.xml (right):
> 
>
https://codereview.chromium.org/1164403002/diff/200001/platform_tools/android...
> platform_tools/android/visualbench/res/values/strings.xml:1: <?xml
version="1.0"
> encoding="utf-8"?>
> You can delete this file now.

okay, I think I squashed it, ptal.

[djsollen, 2015-06-12 18:28:20]

lgtm

[joshualitt, 2015-06-12 19:28:11]

The CQ bit was checked by joshualitt@google.com

[joshualitt, 2015-06-12 19:28:12]

The patchset sent to the CQ was uploaded after l-g-t-m from bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/1164403002/#ps240001
(title: "more")

[commit-bot: I haz the power, 2015-06-12 19:28:23]

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

[commit-bot: I haz the power, 2015-06-12 19:29:30]

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

[commit-bot: I haz the power, 2015-06-12 19:29:32]

Try jobs failed on following builders:
  skia_presubmit-Trybot on client.skia.fyi (JOB_FAILED,
http://build.chromium.org/p/client.skia.fyi/builders/skia_presubmit-Trybot/bu...)

[joshualitt, 2015-06-12 19:37:38]

The CQ bit was checked by joshualitt@google.com

[joshualitt, 2015-06-12 19:37:38]

The patchset sent to the CQ was uploaded after l-g-t-m from djsollen@google.com,
bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/1164403002/#ps260001
(title: "updating copyright")

[commit-bot: I haz the power, 2015-06-12 19:37:47]

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

[joshualitt, 2015-06-12 19:39:55]

The CQ bit was unchecked by joshualitt@google.com

[joshualitt, 2015-06-12 19:46:51]

The CQ bit was checked by joshualitt@google.com

[joshualitt, 2015-06-12 19:46:51]

The patchset sent to the CQ was uploaded after l-g-t-m from djsollen@google.com,
bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/1164403002/#ps280001
(title: "tiny cleanup")

[commit-bot: I haz the power, 2015-06-12 19:47:02]

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

[commit-bot: I haz the power, 2015-06-12 19:51:51]

Committed patchset #15 (id:280001) as
https://skia.googlesource.com/skia/+/030dc847dc497b011c417dc6694e8b6de290895f