smoothness.scrolling_tough_ad_cases to use browser driven scrolls

smoothness.scrolling_tough_ad_cases to use browser driven scrolls

Patch 3 of 4 to fix the overly long delay between
each scroll in smoothness.scrolling_tough_ad_cases caused by the render
thread being unresponsive.

Change ScrollingPage to use the new browser driven
Input.synthesizeScrollGesture api for scrolling.

This should regularize scrolls and make the scrolling
metrics more robust during loading, because otherwise we end up getting
much longer delays between each scroll than intended.  These long delays
bias the results towards the phase when the page is fully loaded,
which defeats the point of this benchmarks.

Patch 1: https://codereview.chromium.org/1299643004/
Patch 2: https://codereview.chromium.org/1296993002/
Patch 4: https://codereview.chromium.org/1285133008/

BUG=510398
Committed: https://crrev.com/af373ea5b3d3777344c44d29520886b113843fbd
Cr-Commit-Position: refs/heads/master@{#344493}

[alex clarke (OOO till 29th), 2015-08-17 15:47:53]

alexclarke@chromium.org changed reviewers:
+ skyostil@chromium.org

[Sami, 2015-08-17 17:06:44]

https://codereview.chromium.org/1291513004/diff/1/tools/perf/page_sets/tough_...
File tools/perf/page_sets/tough_ad_cases.py (right):

https://codereview.chromium.org/1291513004/diff/1/tools/perf/page_sets/tough_...
tools/perf/page_sets/tough_ad_cases.py:69: 'x': 500,
Can we compute these to always be the middle of screen? Otherwise this might be
flaky depending on the screen size.

https://codereview.chromium.org/1291513004/diff/1/tools/perf/page_sets/tough_...
tools/perf/page_sets/tough_ad_cases.py:74: 'repeatCount': 9,
10 assuming we change repeatCount to mean the total number of gestures.

https://codereview.chromium.org/1291513004/diff/1/tools/telemetry/telemetry/i...
File
tools/telemetry/telemetry/internal/backends/chrome_inspector/inspector_backend.py
(right):

https://codereview.chromium.org/1291513004/diff/1/tools/telemetry/telemetry/i...
tools/telemetry/telemetry/internal/backends/chrome_inspector/inspector_backend.py:227:
def RunInspectorCommand(self, command, timeout=60):
It feels like this is a too level interface to be exposing here. How about we
model this after, say, Screenshot(), and call it something like
SynthesizeScrollGesture() and have a synthesize_scroll_gesture_supported
property? Remember we need to check for support anyway since telemetry might be
running against an older version of Chrome.

[alex clarke (OOO till 29th), 2015-08-18 13:40:52]

PTAL

https://codereview.chromium.org/1291513004/diff/1/tools/perf/page_sets/tough_...
File tools/perf/page_sets/tough_ad_cases.py (right):

https://codereview.chromium.org/1291513004/diff/1/tools/perf/page_sets/tough_...
tools/perf/page_sets/tough_ad_cases.py:69: 'x': 500,
On 2015/08/17 17:06:44, Sami wrote:
> Can we compute these to always be the middle of screen? Otherwise this might
be
> flaky depending on the screen size.

Done.

https://codereview.chromium.org/1291513004/diff/1/tools/perf/page_sets/tough_...
tools/perf/page_sets/tough_ad_cases.py:74: 'repeatCount': 9,
On 2015/08/17 17:06:44, Sami wrote:
> 10 assuming we change repeatCount to mean the total number of gestures.

Acknowledged.

https://codereview.chromium.org/1291513004/diff/1/tools/telemetry/telemetry/i...
File
tools/telemetry/telemetry/internal/backends/chrome_inspector/inspector_backend.py
(right):

https://codereview.chromium.org/1291513004/diff/1/tools/telemetry/telemetry/i...
tools/telemetry/telemetry/internal/backends/chrome_inspector/inspector_backend.py:227:
def RunInspectorCommand(self, command, timeout=60):
On 2015/08/17 17:06:44, Sami wrote:
> It feels like this is a too level interface to be exposing here. How about we
> model this after, say, Screenshot(), and call it something like
> SynthesizeScrollGesture() and have a synthesize_scroll_gesture_supported
> property? Remember we need to check for support anyway since telemetry might
be
> running against an older version of Chrome.

Done.

[alex clarke (OOO till 29th), 2015-08-18 13:41:10]

The CQ bit was checked by alexclarke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-18 13:41:30]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1291513004/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1291513004/40001

[Sami, 2015-08-18 15:06:46]

https://codereview.chromium.org/1291513004/diff/40001/tools/perf/page_sets/re...
File tools/perf/page_sets/repeatable_synthesize_scroll_gesture_shared_state.py
(right):

https://codereview.chromium.org/1291513004/diff/40001/tools/perf/page_sets/re...
tools/perf/page_sets/repeatable_synthesize_scroll_gesture_shared_state.py:9:
class RepeatableSynthesizeScrollGestureSharedState(
nit: should this be "synthetic" or "synthesized" instead of "synthesize"?

https://codereview.chromium.org/1291513004/diff/40001/tools/perf/page_sets/to...
File tools/perf/page_sets/tough_ad_cases.py (right):

https://codereview.chromium.org/1291513004/diff/40001/tools/perf/page_sets/to...
tools/perf/page_sets/tough_ad_cases.py:50: self._scroll_multiplier = 2
Could you make this a named parameter?

https://codereview.chromium.org/1291513004/diff/40001/tools/perf/page_sets/to...
tools/perf/page_sets/tough_ad_cases.py:51: print "ScrollingPage"
Probably want to leave these out :)

https://codereview.chromium.org/1291513004/diff/40001/tools/perf/page_sets/to...
tools/perf/page_sets/tough_ad_cases.py:72: # Get the dimensions of the screen.
This is another round trip through the main thread, so should we do it as the
first thing in navigate steps instead (before we start loading the page)?

https://codereview.chromium.org/1291513004/diff/40001/tools/perf/page_sets/to...
tools/perf/page_sets/tough_ad_cases.py:80: flags =
[timeline_interaction_record.REPEATABLE]
Could you make this into a new Action instance (with a test)?

https://codereview.chromium.org/1291513004/diff/40001/tools/perf/page_sets/to...
tools/perf/page_sets/tough_ad_cases.py:93:
action_runner.tab.SynthesizeScrollGesture(scroll_params)
Two blank lines here please.

https://codereview.chromium.org/1291513004/diff/40001/tools/telemetry/telemet...
File tools/telemetry/telemetry/internal/browser/web_contents.py (right):

https://codereview.chromium.org/1291513004/diff/40001/tools/telemetry/telemet...
tools/telemetry/telemetry/internal/browser/web_contents.py:300: """Runs an
inspector command.
This could use some documentation and |params| would work better as a set of
named arguments I think.

[commit-bot: I haz the power, 2015-08-18 15:52:36]

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

[commit-bot: I haz the power, 2015-08-18 15:52:36]

Dry run: This issue passed the CQ dry run.

[alex clarke (OOO till 29th), 2015-08-19 11:42:20]

PTAL

https://codereview.chromium.org/1291513004/diff/40001/tools/perf/page_sets/re...
File tools/perf/page_sets/repeatable_synthesize_scroll_gesture_shared_state.py
(right):

https://codereview.chromium.org/1291513004/diff/40001/tools/perf/page_sets/re...
tools/perf/page_sets/repeatable_synthesize_scroll_gesture_shared_state.py:9:
class RepeatableSynthesizeScrollGestureSharedState(
On 2015/08/18 15:06:46, Sami wrote:
> nit: should this be "synthetic" or "synthesized" instead of "synthesize"?

I can reorder  Repeatable and Synthesize

https://codereview.chromium.org/1291513004/diff/40001/tools/perf/page_sets/to...
File tools/perf/page_sets/tough_ad_cases.py (right):

https://codereview.chromium.org/1291513004/diff/40001/tools/perf/page_sets/to...
tools/perf/page_sets/tough_ad_cases.py:50: self._scroll_multiplier = 2
On 2015/08/18 15:06:46, Sami wrote:
> Could you make this a named parameter?

Done.

https://codereview.chromium.org/1291513004/diff/40001/tools/perf/page_sets/to...
tools/perf/page_sets/tough_ad_cases.py:51: print "ScrollingPage"
On 2015/08/18 15:06:46, Sami wrote:
> Probably want to leave these out :)

Done.

https://codereview.chromium.org/1291513004/diff/40001/tools/perf/page_sets/to...
tools/perf/page_sets/tough_ad_cases.py:72: # Get the dimensions of the screen.
On 2015/08/18 15:06:46, Sami wrote:
> This is another round trip through the main thread, so should we do it as the
> first thing in navigate steps instead (before we start loading the page)?

Done.

https://codereview.chromium.org/1291513004/diff/40001/tools/perf/page_sets/to...
tools/perf/page_sets/tough_ad_cases.py:80: flags =
[timeline_interaction_record.REPEATABLE]
On 2015/08/18 15:06:46, Sami wrote:
> Could you make this into a new Action instance (with a test)?

Seems like a lot of boilerplate, I'm not sure I'm seeing the benefit here?

https://codereview.chromium.org/1291513004/diff/40001/tools/perf/page_sets/to...
tools/perf/page_sets/tough_ad_cases.py:93:
action_runner.tab.SynthesizeScrollGesture(scroll_params)
On 2015/08/18 15:06:46, Sami wrote:
> Two blank lines here please.

Done.

https://codereview.chromium.org/1291513004/diff/40001/tools/telemetry/telemet...
File tools/telemetry/telemetry/internal/browser/web_contents.py (right):

https://codereview.chromium.org/1291513004/diff/40001/tools/telemetry/telemet...
tools/telemetry/telemetry/internal/browser/web_contents.py:300: """Runs an
inspector command.
On 2015/08/18 15:06:46, Sami wrote:
> This could use some documentation and |params| would work better as a set of
> named arguments I think.

Done.

[Sami, 2015-08-19 11:54:46]

https://codereview.chromium.org/1291513004/diff/40001/tools/perf/page_sets/to...
File tools/perf/page_sets/tough_ad_cases.py (right):

https://codereview.chromium.org/1291513004/diff/40001/tools/perf/page_sets/to...
tools/perf/page_sets/tough_ad_cases.py:80: flags =
[timeline_interaction_record.REPEATABLE]
On 2015/08/19 11:42:20, alexclarke1 wrote:
> On 2015/08/18 15:06:46, Sami wrote:
> > Could you make this into a new Action instance (with a test)?
> 
> Seems like a lot of boilerplate, I'm not sure I'm seeing the benefit here?

The idea was that we could use this in all page sets and not just this one. I'd
just use scroll.py as a template and remove everything that isn't needed for
this browser side scroll. In any case we need a test because otherwise this will
bitrot pretty quickly.

https://codereview.chromium.org/1291513004/diff/60001/tools/perf/page_sets/to...
File tools/perf/page_sets/tough_ad_cases.py (right):

https://codereview.chromium.org/1291513004/diff/60001/tools/perf/page_sets/to...
tools/perf/page_sets/tough_ad_cases.py:40: make_javascript_deterministic=True,
scroll_multiplier=0.5):
What does it mean to scroll .5 times?-)

[alex clarke (OOO till 29th), 2015-08-19 15:40:58]

I added a test, PTAL

https://codereview.chromium.org/1291513004/diff/60001/tools/perf/page_sets/to...
File tools/perf/page_sets/tough_ad_cases.py (right):

https://codereview.chromium.org/1291513004/diff/60001/tools/perf/page_sets/to...
tools/perf/page_sets/tough_ad_cases.py:40: make_javascript_deterministic=True,
scroll_multiplier=0.5):
On 2015/08/19 11:54:46, Sami wrote:
> What does it mean to scroll .5 times?-)

It's a distance multiplier, hopefully that is more obvious now.

[alex clarke (OOO till 29th), 2015-08-19 15:41:18]

The CQ bit was checked by alexclarke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-19 15:42:02]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1291513004/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1291513004/80001

[commit-bot: I haz the power, 2015-08-19 15:44:26]

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

[commit-bot: I haz the power, 2015-08-19 15:44:27]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[alex clarke (OOO till 29th), 2015-08-19 16:51:51]

The CQ bit was checked by alexclarke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-19 16:52:18]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1291513004/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1291513004/100001

[Sami, 2015-08-19 17:21:19]

Thanks for adding the test!

https://codereview.chromium.org/1291513004/diff/40001/tools/perf/page_sets/re...
File tools/perf/page_sets/repeatable_synthesize_scroll_gesture_shared_state.py
(right):

https://codereview.chromium.org/1291513004/diff/40001/tools/perf/page_sets/re...
tools/perf/page_sets/repeatable_synthesize_scroll_gesture_shared_state.py:9:
class RepeatableSynthesizeScrollGestureSharedState(
On 2015/08/19 11:42:20, alexclarke1 wrote:
> On 2015/08/18 15:06:46, Sami wrote:
> > nit: should this be "synthetic" or "synthesized" instead of "synthesize"?
> 
> I can reorder  Repeatable and Synthesize

I meant that "synthesize scroll gesture" sounds a bit weird. Since we already
have synthetic scroll gestures in telemetry, how about calling this
browser_side_scroll_gesture to distinguish it?

https://codereview.chromium.org/1291513004/diff/100001/tools/telemetry/teleme...
File tools/telemetry/telemetry/internal/actions/repeatable_scroll.py (right):

https://codereview.chromium.org/1291513004/diff/100001/tools/telemetry/teleme...
tools/telemetry/telemetry/internal/actions/repeatable_scroll.py:23:
screen_info_js = 'window.innerWidth + "," + window.innerHeight'
window_info_js?

https://codereview.chromium.org/1291513004/diff/100001/tools/telemetry/teleme...
tools/telemetry/telemetry/internal/actions/repeatable_scroll.py:25: if
self._add_nop_touch_handler:
This feels like an odd place to do this since it's very specific to
tough_ad_cases. How about doing it where you are awaiting for scrollability?

https://codereview.chromium.org/1291513004/diff/100001/tools/telemetry/teleme...
tools/telemetry/telemetry/internal/actions/repeatable_scroll.py:34:
self._screensize = [int(js_result[0]), int(js_result[1])]
windowsize?

https://codereview.chromium.org/1291513004/diff/100001/tools/telemetry/teleme...
tools/telemetry/telemetry/internal/actions/repeatable_scroll.py:41:
y=self._screensize[1] - int(self._screensize[1] * 0.25),
Any reason for not using the center here too? FWIW only the starting coordinate
needs to be inside the screen -- the scroll can progress beyond the screen.

https://codereview.chromium.org/1291513004/diff/100001/tools/telemetry/teleme...
File tools/telemetry/telemetry/internal/actions/repeatable_scroll_unittest.py
(right):

https://codereview.chromium.org/1291513004/diff/100001/tools/telemetry/teleme...
tools/telemetry/telemetry/internal/actions/repeatable_scroll_unittest.py:11: #
Repeatable SynthesizeScrollGesture scrolling was not supported until
Maybe have a setupPage_() function that both tests call? It could be the
standard setUp() function too.

https://codereview.chromium.org/1291513004/diff/100001/tools/telemetry/teleme...
tools/telemetry/telemetry/internal/actions/repeatable_scroll_unittest.py:13:
branch_num = self._tab.browser._browser_backend.devtools_client \
Could we call HasRepeatableSynthesizeScrollGesture() here?

https://codereview.chromium.org/1291513004/diff/100001/tools/telemetry/teleme...
tools/telemetry/telemetry/internal/actions/repeatable_scroll_unittest.py:28:
window_height = int(self._tab.EvaluateJavaScript("window.innerHeight"))
nit: single quotes

https://codereview.chromium.org/1291513004/diff/100001/tools/telemetry/teleme...
File
tools/telemetry/telemetry/internal/backends/chrome_inspector/inspector_backend.py
(right):

https://codereview.chromium.org/1291513004/diff/100001/tools/telemetry/teleme...
tools/telemetry/telemetry/internal/backends/chrome_inspector/inspector_backend.py:227:
def SynthesizeScrollGesture(self, params, timeout=60):
Could you make this take the named arguments too? It doesn't look like any of
the other methods here are exposing params like this.

[commit-bot: I haz the power, 2015-08-19 18:02:49]

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

[commit-bot: I haz the power, 2015-08-19 18:02:50]

Dry run: This issue passed the CQ dry run.

[alex clarke (OOO till 29th), 2015-08-20 09:39:41]

PTAL

https://codereview.chromium.org/1291513004/diff/100001/tools/telemetry/teleme...
File tools/telemetry/telemetry/internal/actions/repeatable_scroll.py (right):

https://codereview.chromium.org/1291513004/diff/100001/tools/telemetry/teleme...
tools/telemetry/telemetry/internal/actions/repeatable_scroll.py:23:
screen_info_js = 'window.innerWidth + "," + window.innerHeight'
On 2015/08/19 17:21:19, Sami wrote:
> window_info_js?

Acknowledged.

https://codereview.chromium.org/1291513004/diff/100001/tools/telemetry/teleme...
tools/telemetry/telemetry/internal/actions/repeatable_scroll.py:25: if
self._add_nop_touch_handler:
On 2015/08/19 17:21:19, Sami wrote:
> This feels like an odd place to do this since it's very specific to
> tough_ad_cases. How about doing it where you are awaiting for scrollability?

Done.

https://codereview.chromium.org/1291513004/diff/100001/tools/telemetry/teleme...
tools/telemetry/telemetry/internal/actions/repeatable_scroll.py:34:
self._screensize = [int(js_result[0]), int(js_result[1])]
On 2015/08/19 17:21:19, Sami wrote:
> windowsize?

Done.

https://codereview.chromium.org/1291513004/diff/100001/tools/telemetry/teleme...
tools/telemetry/telemetry/internal/actions/repeatable_scroll.py:41:
y=self._screensize[1] - int(self._screensize[1] * 0.25),
On 2015/08/19 17:21:19, Sami wrote:
> Any reason for not using the center here too? FWIW only the starting
coordinate
> needs to be inside the screen -- the scroll can progress beyond the screen.

Done.

https://codereview.chromium.org/1291513004/diff/100001/tools/telemetry/teleme...
File tools/telemetry/telemetry/internal/actions/repeatable_scroll_unittest.py
(right):

https://codereview.chromium.org/1291513004/diff/100001/tools/telemetry/teleme...
tools/telemetry/telemetry/internal/actions/repeatable_scroll_unittest.py:11: #
Repeatable SynthesizeScrollGesture scrolling was not supported until
On 2015/08/19 17:21:19, Sami wrote:
> Maybe have a setupPage_() function that both tests call? It could be the
> standard setUp() function too.

Done.

https://codereview.chromium.org/1291513004/diff/100001/tools/telemetry/teleme...
tools/telemetry/telemetry/internal/actions/repeatable_scroll_unittest.py:13:
branch_num = self._tab.browser._browser_backend.devtools_client \
On 2015/08/19 17:21:19, Sami wrote:
> Could we call HasRepeatableSynthesizeScrollGesture() here?

Done.

https://codereview.chromium.org/1291513004/diff/100001/tools/telemetry/teleme...
tools/telemetry/telemetry/internal/actions/repeatable_scroll_unittest.py:28:
window_height = int(self._tab.EvaluateJavaScript("window.innerHeight"))
On 2015/08/19 17:21:19, Sami wrote:
> nit: single quotes

Done.

https://codereview.chromium.org/1291513004/diff/100001/tools/telemetry/teleme...
File
tools/telemetry/telemetry/internal/backends/chrome_inspector/inspector_backend.py
(right):

https://codereview.chromium.org/1291513004/diff/100001/tools/telemetry/teleme...
tools/telemetry/telemetry/internal/backends/chrome_inspector/inspector_backend.py:227:
def SynthesizeScrollGesture(self, params, timeout=60):
On 2015/08/19 17:21:19, Sami wrote:
> Could you make this take the named arguments too? It doesn't look like any of
> the other methods here are exposing params like this.

Done.

[Sami, 2015-08-20 12:39:53]

Nice, lgtm with a couple of nits.

https://codereview.chromium.org/1291513004/diff/200001/tools/perf/page_sets/r...
File tools/perf/page_sets/repeatable_synthesize_scroll_gesture_shared_state.py
(right):

https://codereview.chromium.org/1291513004/diff/200001/tools/perf/page_sets/r...
tools/perf/page_sets/repeatable_synthesize_scroll_gesture_shared_state.py:14:
logging.warning('Browser does not support repatable scroll gestures, '
typo: repeatable

https://codereview.chromium.org/1291513004/diff/200001/tools/perf/page_sets/t...
File tools/perf/page_sets/tough_ad_cases.py (right):

https://codereview.chromium.org/1291513004/diff/200001/tools/perf/page_sets/t...
tools/perf/page_sets/tough_ad_cases.py:65: 'document.body.scrollHeight >
window.screen.height && '
window.innerHeight is probably want you want here (otherwise this is comparing
CSS and physical pixels)

https://codereview.chromium.org/1291513004/diff/200001/tools/telemetry/teleme...
File
tools/telemetry/telemetry/internal/backends/chrome_inspector/inspector_backend.py
(right):

https://codereview.chromium.org/1291513004/diff/200001/tools/telemetry/teleme...
tools/telemetry/telemetry/internal/backends/chrome_inspector/inspector_backend.py:239:
yDistance: Ddistance to scroll along the Y axis (positive to scroll up).
typo: distance

[alex clarke (OOO till 29th), 2015-08-20 12:45:45]

https://codereview.chromium.org/1291513004/diff/200001/tools/perf/page_sets/r...
File tools/perf/page_sets/repeatable_synthesize_scroll_gesture_shared_state.py
(right):

https://codereview.chromium.org/1291513004/diff/200001/tools/perf/page_sets/r...
tools/perf/page_sets/repeatable_synthesize_scroll_gesture_shared_state.py:14:
logging.warning('Browser does not support repatable scroll gestures, '
On 2015/08/20 12:39:53, Sami wrote:
> typo: repeatable

Done.

https://codereview.chromium.org/1291513004/diff/200001/tools/perf/page_sets/t...
File tools/perf/page_sets/tough_ad_cases.py (right):

https://codereview.chromium.org/1291513004/diff/200001/tools/perf/page_sets/t...
tools/perf/page_sets/tough_ad_cases.py:65: 'document.body.scrollHeight >
window.screen.height && '
On 2015/08/20 12:39:53, Sami wrote:
> window.innerHeight is probably want you want here (otherwise this is comparing
> CSS and physical pixels)

Done.

https://codereview.chromium.org/1291513004/diff/200001/tools/telemetry/teleme...
File
tools/telemetry/telemetry/internal/backends/chrome_inspector/inspector_backend.py
(right):

https://codereview.chromium.org/1291513004/diff/200001/tools/telemetry/teleme...
tools/telemetry/telemetry/internal/backends/chrome_inspector/inspector_backend.py:239:
yDistance: Ddistance to scroll along the Y axis (positive to scroll up).
On 2015/08/20 12:39:53, Sami wrote:
> typo: distance

Done.

[alex clarke (OOO till 29th), 2015-08-20 12:47:15]

The CQ bit was checked by alexclarke@chromium.org

[alex clarke (OOO till 29th), 2015-08-20 12:47:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
skyostil@chromium.org
Link to the patchset: https://codereview.chromium.org/1291513004/#ps240001
(title: "Final nits")

[commit-bot: I haz the power, 2015-08-20 12:47:36]

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

[commit-bot: I haz the power, 2015-08-20 14:22:08]

Committed patchset #13 (id:240001)

[commit-bot: I haz the power, 2015-08-20 14:22:58]

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