Move BenchTimer to tools as Timer

Move BenchTimer to tools as Timer

This breaks a bunch of circular dependencies between tools and gm and bench.

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/4ed75287aed6371c6e4a41ffcc78c8a49c9810ed

CQ_EXTRA_TRYBOTS=tryserver.skia:Build-Mac10.7-Clang-Arm7-Debug-iOS-Trybot,Test-Ubuntu12-ShuttleA-GTX660-x86-Debug-Trybot

Committed: https://skia.googlesource.com/skia/+/9ac68ee2594f28dfc1bd16a4bf004b1a4f6f228d

[mtklein_C, 2014-06-20 15:12:48]

I think this should let us put sk_tool_utils in tools.gyp.

[tfarina, 2014-06-20 16:20:37]

I loved it.

Thanks Mike!

https://codereview.chromium.org/344213003/diff/40001/gyp/debugger.gyp
File gyp/debugger.gyp (right):

https://codereview.chromium.org/344213003/diff/40001/gyp/debugger.gyp#newcode99
gyp/debugger.gyp:99: 'tools.gyp:timer',
could you sort?

https://codereview.chromium.org/344213003/diff/40001/gyp/tools.gyp
File gyp/tools.gyp (right):

https://codereview.chromium.org/344213003/diff/40001/gyp/tools.gyp#newcode63
gyp/tools.gyp:63: 'target_name': 'crash_handler',
CrashHandler -> crash_handler!

Yes, thanks for renaming it along the way.

https://codereview.chromium.org/344213003/diff/40001/gyp/tools.gyp#newcode64
gyp/tools.gyp:64: 'type': 'static_library',
while you are here, could you use 2 spaces indent, like the rest of the file?

https://codereview.chromium.org/344213003/diff/40001/gyp/tools.gyp#newcode65
gyp/tools.gyp:65: 'sources': [ '../tools/CrashHandler.cpp' ],
for me it is more readable if you write it as:

'sources': [
  ...
],

because it can be more easily expanded in future if someone come and add a new
file to this list.

not a big deal if you want to leave it as is in a single line though.

https://codereview.chromium.org/344213003/diff/40001/tools/timer/Timer.cpp
File tools/timer/Timer.cpp (right):

https://codereview.chromium.org/344213003/diff/40001/tools/timer/Timer.cpp#ne...
tools/timer/Timer.cpp:7: #include "Timer.h"
if you tweak the --similarity passed to git cl upload, say 20 or 10, I'm sure it
will show this as a copy of BenchTimer and not as a totaly new file.

https://codereview.chromium.org/344213003/diff/40001/tools/timer/Timer.cpp#ne...
tools/timer/Timer.cpp:54: 
could you remove this empty/trailing new line?

[tfarina, 2014-06-20 16:24:40]

https://codereview.chromium.org/344213003/diff/40001/gyp/tools.gyp
File gyp/tools.gyp (right):

https://codereview.chromium.org/344213003/diff/40001/gyp/tools.gyp#newcode104
gyp/tools.gyp:104: 'dependencies': [
indent one more space.

https://codereview.chromium.org/344213003/diff/40001/gyp/tools.gyp#newcode105
gyp/tools.gyp:105: 'skia_lib.gyp:skia_lib',
indentation is off here.

[mtklein, 2014-06-20 16:33:27]

PTAL?

https://codereview.chromium.org/344213003/diff/40001/gyp/debugger.gyp
File gyp/debugger.gyp (right):

https://codereview.chromium.org/344213003/diff/40001/gyp/debugger.gyp#newcode99
gyp/debugger.gyp:99: 'tools.gyp:timer',
On 2014/06/20 16:20:37, tfarina wrote:
> could you sort?

Done.

https://codereview.chromium.org/344213003/diff/40001/gyp/tools.gyp
File gyp/tools.gyp (right):

https://codereview.chromium.org/344213003/diff/40001/gyp/tools.gyp#newcode64
gyp/tools.gyp:64: 'type': 'static_library',
On 2014/06/20 16:20:37, tfarina wrote:
> while you are here, could you use 2 spaces indent, like the rest of the file?

Done.

https://codereview.chromium.org/344213003/diff/40001/gyp/tools.gyp#newcode65
gyp/tools.gyp:65: 'sources': [ '../tools/CrashHandler.cpp' ],
On 2014/06/20 16:20:37, tfarina wrote:
> for me it is more readable if you write it as:
> 
> 'sources': [
>   ...
> ],
> 
> because it can be more easily expanded in future if someone come and add a new
> file to this list.
> 
> not a big deal if you want to leave it as is in a single line though.

Let's leave it until we need >1 lines.

https://codereview.chromium.org/344213003/diff/40001/gyp/tools.gyp#newcode104
gyp/tools.gyp:104: 'dependencies': [
On 2014/06/20 16:24:40, tfarina wrote:
> indent one more space.

Done.

https://codereview.chromium.org/344213003/diff/40001/gyp/tools.gyp#newcode105
gyp/tools.gyp:105: 'skia_lib.gyp:skia_lib',
On 2014/06/20 16:24:40, tfarina wrote:
> indentation is off here.

Done.

[tfarina, 2014-06-20 16:41:22]

lgtm

https://codereview.chromium.org/344213003/diff/60001/gyp/tools.gyp
File gyp/tools.gyp (right):

https://codereview.chromium.org/344213003/diff/60001/gyp/tools.gyp#newcode81
gyp/tools.gyp:81: 'sources': [ '../tools/Resources.cpp' ],
did you omit Resources.h intentionally? I think if you list it here, people
using VS IDE and Xcode will see it in their file tree. not a big deal of course.

[mtklein, 2014-06-20 16:43:22]

lgtm

https://codereview.chromium.org/344213003/diff/60001/gyp/tools.gyp
File gyp/tools.gyp (right):

https://codereview.chromium.org/344213003/diff/60001/gyp/tools.gyp#newcode81
gyp/tools.gyp:81: 'sources': [ '../tools/Resources.cpp' ],
On 2014/06/20 16:41:22, tfarina wrote:
> did you omit Resources.h intentionally? I think if you list it here, people
> using VS IDE and Xcode will see it in their file tree. not a big deal of
course.

Yeah mostly because it has no effect on the build.  Things work if you put it in
or leave it out, so might as well leave it out to not mislead us.  Surely IDEs
have a file tree?

[commit-bot: I haz the power, 2014-06-20 16:44:14]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/344213003/60001

[commit-bot: I haz the power, 2014-06-20 16:59:22]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  Build-Ubuntu13.10-GCC4.8-Arm7-Debug-Android-Trybot on tryserver.skia
(http://108.170.220.76:10117/builders/Build-Ubuntu13.10-GCC4.8-Arm7-Debug-Andr...)

[commit-bot: I haz the power, 2014-06-20 17:01:31]

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

[commit-bot: I haz the power, 2014-06-20 17:01:33]

Try jobs failed on following builders:
  Build-Ubuntu13.10-GCC4.8-Arm7-Debug-Android-Trybot on tryserver.skia
(http://108.170.220.76:10117/builders/Build-Ubuntu13.10-GCC4.8-Arm7-Debug-Andr...)

[mtklein, 2014-06-20 17:16:12]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2014-06-20 17:16:31]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/344213003/80001

[commit-bot: I haz the power, 2014-06-20 17:31:55]

Change committed as 4ed75287aed6371c6e4a41ffcc78c8a49c9810ed

[mtklein, 2014-06-20 17:42:03]

A revert of this CL has been created in
https://codereview.chromium.org/346753003/ by mtklein@google.com.

The reason for reverting is: GpuTimer broken.

[mtklein, 2014-06-20 17:56:15]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2014-06-20 17:56:52]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/344213003/100001

[mtklein, 2014-06-20 17:58:53]

The CQ bit was unchecked by mtklein@google.com

[mtklein, 2014-06-20 17:59:17]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2014-06-20 17:59:19]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/344213003/100001

[tfarina, 2014-06-20 18:01:50]

+Leon for platform_tools/android changes, fyi.

[tfarina, 2014-06-20 18:05:58]

https://codereview.chromium.org/344213003/diff/100001/tools/timer/GpuTimer.cpp
File tools/timer/GpuTimer.cpp (right):

https://codereview.chromium.org/344213003/diff/100001/tools/timer/GpuTimer.cp...
tools/timer/GpuTimer.cpp:13: if (fContext) {
if glctx is NULL, aren't we going to leaving fStarted, fSupported and fQuery
uninitialized? Could we initialize them regardless in the initializer list with
some default value?

[tfarina, 2014-06-20 18:06:41]

https://codereview.chromium.org/344213003/diff/100001/tools/timer/GpuTimer.cpp
File tools/timer/GpuTimer.cpp (right):

https://codereview.chromium.org/344213003/diff/100001/tools/timer/GpuTimer.cp...
tools/timer/GpuTimer.cpp:13: if (fContext) {
On 2014/06/20 18:05:58, tfarina wrote:
> if glctx is NULL, aren't we going to leaving fStarted, fSupported
s/leaving/leave

[mtklein, 2014-06-20 18:08:19]

https://codereview.chromium.org/344213003/diff/100001/tools/timer/GpuTimer.cpp
File tools/timer/GpuTimer.cpp (right):

https://codereview.chromium.org/344213003/diff/100001/tools/timer/GpuTimer.cp...
tools/timer/GpuTimer.cpp:13: if (fContext) {
On 2014/06/20 18:05:58, tfarina wrote:
> if glctx is NULL, aren't we going to leaving fStarted, fSupported and fQuery
> uninitialized? Could we initialize them regardless in the initializer list
with
> some default value?

Yes.  If fContext is NULL, we'll never touch any of the other members.  We could
initialize them, but that seems misleading.  They're not meaningful without
fContext.  I'd rather tools yell at us for using uninitialized memory than
silently get some default if we screw up here.

[commit-bot: I haz the power, 2014-06-20 18:29:25]

Change committed as 9ac68ee2594f28dfc1bd16a4bf004b1a4f6f228d