Initial skeleton of custom layouts for Web Notifications.

Initial skeleton of custom layouts for Web Notifications.

This skeleton rendering does the following:
* Render the contents of title, body, origin, time, icon, and icon overlay
* Handles the compact form of the notification
* Uses different styles for v17 (Jelly Bean) and v21 (Material)

It explicitly does not:
* Set padding or positioning or really any exact style quite correctly
* Handle the expanded form or do anything special for the private form

BUG=541617
Committed: https://crrev.com/4d99f1d9235c49f77d31e6029dd1c5312836e661
Cr-Commit-Position: refs/heads/master@{#354759}

[Michael van Ouwerkerk, 2015-10-09 16:19:40]

mvanouwerkerk@chromium.org changed reviewers:
+ peter@chromium.org

[Michael van Ouwerkerk, 2015-10-09 16:19:41]

Peter: could you take a look please? There's no tests yet so I suspect that
might be a concern :-) But early feedback could already be valuable. Thanks!

[Peter Beverloo, 2015-10-12 14:08:30]

Great to see this in action, thanks Michael! A few higher level comments

https://codereview.chromium.org/1394423004/diff/20001/chrome/android/java/res...
File chrome/android/java/res/layout/web_notification.xml (right):

https://codereview.chromium.org/1394423004/diff/20001/chrome/android/java/res...
chrome/android/java/res/layout/web_notification.xml:7: Web notification compact
layout.
note: it's going to be a while before I can comfortably review this. Hierarchy
Viewer isn't giving me measurements either.

Reading a bunch on stack overflow and the Android site[1], this should be fine
as the LinearLayouts don't use weights and thus don't require multiple
measurements.

[1]
http://developer.android.com/training/improving-layouts/optimizing-layout.html

https://codereview.chromium.org/1394423004/diff/20001/chrome/android/java/res...
chrome/android/java/res/layout/web_notification.xml:51:
android:layout_width="0dp"
moan: how lovely

https://codereview.chromium.org/1394423004/diff/20001/chrome/android/java/res...
File chrome/android/java/res/values-v21/styles.xml (right):

https://codereview.chromium.org/1394423004/diff/20001/chrome/android/java/res...
chrome/android/java/res/values-v21/styles.xml:66:
parent="@android:style/TextAppearance.Material.Notification.Title"/>
Is this different per theme? Does that mean that it wouldn't pick up custom OEM
colours/settings?

https://codereview.chromium.org/1394423004/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java
(right):

https://codereview.chromium.org/1394423004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java:430:
if (useCustomLayouts()) {
I'm a bit concerned that this isn't going to scale very well. Eventually
buildCustomLayout() would have to take pretty much all the options. Perhaps this
would be a valid place to apply the strategy pattern?

=========================
interface NotificationBuilder {
  NotificationBuilder setBody(String body);
  NotificationBuilder setContent(String content);
}

class NativeNotificationBuilder implements NotificationBuilder {
  NotificationBuilder setBody(String body) {
    mBuilder.setContentText(body);
    return this;
  }
}

class TemplateNotificationBuilder implements NotificationBuilder {
  NotificationBuilder setBody(String body) {
    mView.setTextViewText(R.id.body, body);
    return this;
  }
}
=========================

This would:

  - Clarify which features aren't supported by either implementation (e.g.
custom buttons)
  - Allow us to logically separate the Site Settings button/icon from the other
actions
  - Make the displayNotification() method significantly nicer to read.
  - Improve testability - instrumentation tests could make sure that the NB gets
the right information, unit tests could verify that the NB does the right thing
with it.

The only concern that I have here is that we might need quite some similar
mBuilder.setFoo() calls because of accessibility. More on that later.

https://codereview.chromium.org/1394423004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java:431:
notificationBuilder.setContent(buildCustomLayout(title, body, origin, icon));
Do you have an understanding of how accessibility works for custom layouts? Or
how apps inspecting notifications (e.g. PushBullet or Hotpocket) deal with it?

I'd love to learn more about that, these are cases that we probably don't want
to break too badly.

https://codereview.chromium.org/1394423004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java:474:
view.setTextViewText(R.id.time, DateFormat.getTimeFormat(mAppContext).format(new
Date()));
Hmm. One set of unit tests I can imagine for custom layouts is presentation of
the date and time. (Sorry, I realize that this is a nightmare.)

https://codereview.chromium.org/1394423004/diff/20001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/1394423004/diff/20001/chrome/app/generated_re...
chrome/app/generated_resources.grd:12602: +          Enable custom layouts for
the Notifications API
You refer to the feature in three different ways:

  (1) Web notification compact layout (web_notification.xml)
  (2) Enable custom layouts for Web Notifications (ChromeSwitches.java)
  (3) --enable-notifications-custom-layouts (ChromeSwitches.java)

Could we align these? I'd propose "Custom Web Notification layouts" because it
works in all three situations.

https://codereview.chromium.org/1394423004/diff/20001/chrome/common/chrome_sw...
File chrome/common/chrome_switches.cc (right):

https://codereview.chromium.org/1394423004/diff/20001/chrome/common/chrome_sw...
chrome/common/chrome_switches.cc:468: const char
kDisableNotificationsCustomLayouts[] =
nit: alphabetize please, and have a comment for each no matter how lame.

[Michael van Ouwerkerk, 2015-10-12 17:10:59]

https://codereview.chromium.org/1394423004/diff/20001/chrome/android/java/res...
File chrome/android/java/res/layout/web_notification.xml (right):

https://codereview.chromium.org/1394423004/diff/20001/chrome/android/java/res...
chrome/android/java/res/layout/web_notification.xml:7: Web notification compact
layout.
On 2015/10/12 14:08:30, Peter Beverloo wrote:
> note: it's going to be a while before I can comfortably review this. Hierarchy
> Viewer isn't giving me measurements either.
> 
> Reading a bunch on stack overflow and the Android site[1], this should be fine
> as the LinearLayouts don't use weights and thus don't require multiple
> measurements.
> 
> [1]
> http://developer.android.com/training/improving-layouts/optimizing-layout.html

That's a cool article. As it states, this matters more when layouts are repeated
a lot, but it's a good tool to have. Also, as this work progresses, I'll keep an
eye on how the official layouts were done. For example, the BigText style does
use some LinearLayouts but has a FrameLayout at its root. Performance might be
the reason, or maybe it simplified something else. I'm sure I'll learn more
about the pros and cons of each as this progresses.

https://codereview.chromium.org/1394423004/diff/20001/chrome/android/java/res...
chrome/android/java/res/layout/web_notification.xml:51:
android:layout_width="0dp"
On 2015/10/12 14:08:30, Peter Beverloo wrote:
> moan: how lovely

Yeah, the weight does it.

https://codereview.chromium.org/1394423004/diff/20001/chrome/android/java/res...
File chrome/android/java/res/values-v21/styles.xml (right):

https://codereview.chromium.org/1394423004/diff/20001/chrome/android/java/res...
chrome/android/java/res/values-v21/styles.xml:66:
parent="@android:style/TextAppearance.Material.Notification.Title"/>
On 2015/10/12 14:08:30, Peter Beverloo wrote:
> Is this different per theme? Does that mean that it wouldn't pick up custom
OEM
> colours/settings?

The v21 suffix in the directory is for the API version in which material design
was introduced. I'm not sure yet how OEM themes modify the parent styles used
here.

See this article for the available qualifiers and how they are applied:
http://developer.android.com/guide/topics/resources/providing-resources.html

https://codereview.chromium.org/1394423004/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java
(right):

https://codereview.chromium.org/1394423004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java:430:
if (useCustomLayouts()) {
On 2015/10/12 14:08:30, Peter Beverloo wrote:
> I'm a bit concerned that this isn't going to scale very well. Eventually
> buildCustomLayout() would have to take pretty much all the options. Perhaps
this
> would be a valid place to apply the strategy pattern?
> 
> =========================
> interface NotificationBuilder {
>   NotificationBuilder setBody(String body);
>   NotificationBuilder setContent(String content);
> }
> 
> class NativeNotificationBuilder implements NotificationBuilder {
>   NotificationBuilder setBody(String body) {
>     mBuilder.setContentText(body);
>     return this;
>   }
> }
> 
> class TemplateNotificationBuilder implements NotificationBuilder {
>   NotificationBuilder setBody(String body) {
>     mView.setTextViewText(R.id.body, body);
>     return this;
>   }
> }
> =========================
> 
> This would:
> 
>   - Clarify which features aren't supported by either implementation (e.g.
> custom buttons)
>   - Allow us to logically separate the Site Settings button/icon from the
other
> actions
>   - Make the displayNotification() method significantly nicer to read.
>   - Improve testability - instrumentation tests could make sure that the NB
gets
> the right information, unit tests could verify that the NB does the right
thing
> with it.
> 
> The only concern that I have here is that we might need quite some similar
> mBuilder.setFoo() calls because of accessibility. More on that later.

Seems like a good idea, I'll take a look.

https://codereview.chromium.org/1394423004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java:431:
notificationBuilder.setContent(buildCustomLayout(title, body, origin, icon));
On 2015/10/12 14:08:30, Peter Beverloo wrote:
> Do you have an understanding of how accessibility works for custom layouts? Or
> how apps inspecting notifications (e.g. PushBullet or Hotpocket) deal with it?
> 
> I'd love to learn more about that, these are cases that we probably don't want
> to break too badly.

My understanding is that while this CL introduces a custom layout, it does not
introduce custom views, and there are good a11y hooks for the standard Android
views. So we can use contentDescription for image-only elements, and the
appropriate elements are focusable by default. We can tweak the focus order if
the default is somehow not quite right for the layout we made.

See this article for more details:
http://developer.android.com/guide/topics/ui/accessibility/apps.html

We'll have to test for a11y but that was already part of the plan.

Finally, these notification layouts are not that complex, not when compared to
e.g. a whole app. The layouts are not particularly dynamic, and not very deep or
huge.

https://codereview.chromium.org/1394423004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java:474:
view.setTextViewText(R.id.time, DateFormat.getTimeFormat(mAppContext).format(new
Date()));
On 2015/10/12 14:08:30, Peter Beverloo wrote:
> Hmm. One set of unit tests I can imagine for custom layouts is presentation of
> the date and time. (Sorry, I realize that this is a nightmare.)

Well, this relies heavily on android.text.format.DateFormat using trivial
arguments. It seems that anything testing the time part of the notification
deeply will risk testing DateFormat instead.

https://codereview.chromium.org/1394423004/diff/20001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/1394423004/diff/20001/chrome/app/generated_re...
chrome/app/generated_resources.grd:12602: +          Enable custom layouts for
the Notifications API
On 2015/10/12 14:08:30, Peter Beverloo wrote:
> You refer to the feature in three different ways:
> 
>   (1) Web notification compact layout (web_notification.xml)
>   (2) Enable custom layouts for Web Notifications (ChromeSwitches.java)
>   (3) --enable-notifications-custom-layouts (ChromeSwitches.java)
> 
> Could we align these? I'd propose "Custom Web Notification layouts" because it
> works in all three situations.

I went for "web notification custom layouts" as it keeps the pairs together
("web notification" and "custom layouts") and has the appropriate hierarchy with
the subfeature coming after the main feature.

https://codereview.chromium.org/1394423004/diff/20001/chrome/common/chrome_sw...
File chrome/common/chrome_switches.cc (right):

https://codereview.chromium.org/1394423004/diff/20001/chrome/common/chrome_sw...
chrome/common/chrome_switches.cc:468: const char
kDisableNotificationsCustomLayouts[] =
On 2015/10/12 14:08:30, Peter Beverloo wrote:
> nit: alphabetize please, and have a comment for each no matter how lame.

Done.

[Peter Beverloo, 2015-10-13 12:31:50]

lgtm, thank you! :)

We've got some good ideas about testing both ways of showing notifications, but
those will need additional work that I don't think we should do in this patch.

https://codereview.chromium.org/1394423004/diff/60001/chrome/android/java/res...
File chrome/android/java/res/layout/web_notification.xml (right):

https://codereview.chromium.org/1394423004/diff/60001/chrome/android/java/res...
chrome/android/java/res/layout/web_notification.xml:30: <ImageView
nit: does it make sense to add comments here w/ TODOs, even though it'll be
super obvious from the visual result? (Not everybody might actually try the
code?)

https://codereview.chromium.org/1394423004/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java
(right):

https://codereview.chromium.org/1394423004/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java:521:
CommandLine commandLine = CommandLine.getInstance();
nit: Add a TODO for adding finch so that other reviewers know why you've got
both enable and disable flags?

https://codereview.chromium.org/1394423004/diff/60001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1394423004/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:63887: +  <int value="-1160026273"
label="enable-web-notification-custom-layouts"/>
out of interest, where do these numbers come from?

[Michael van Ouwerkerk, 2015-10-13 15:22:42]

Thanks Peter! I've added some simple unit tests for the builders as well, if you
are interested.

https://codereview.chromium.org/1394423004/diff/60001/chrome/android/java/res...
File chrome/android/java/res/layout/web_notification.xml (right):

https://codereview.chromium.org/1394423004/diff/60001/chrome/android/java/res...
chrome/android/java/res/layout/web_notification.xml:30: <ImageView
On 2015/10/13 12:31:50, Peter Beverloo wrote:
> nit: does it make sense to add comments here w/ TODOs, even though it'll be
> super obvious from the visual result? (Not everybody might actually try the
> code?)

I think there would be too many TODOs, to a distracting level. Plus, there are
unknowns anyway that cannot be highlighted with a TODO. The people interested in
this feature and this code already know it's not finished yet.

https://codereview.chromium.org/1394423004/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java
(right):

https://codereview.chromium.org/1394423004/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java:521:
CommandLine commandLine = CommandLine.getInstance();
On 2015/10/13 12:31:50, Peter Beverloo wrote:
> nit: Add a TODO for adding finch so that other reviewers know why you've got
> both enable and disable flags?

Done.

https://codereview.chromium.org/1394423004/diff/60001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1394423004/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:63887: +  <int value="-1160026273"
label="enable-web-notification-custom-layouts"/>
On 2015/10/13 12:31:50, Peter Beverloo wrote:
> out of interest, where do these numbers come from?

I usually just wait for the test to fail and provide the expected values:
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...

[Michael van Ouwerkerk, 2015-10-13 15:29:42]

mvanouwerkerk@chromium.org changed reviewers:
+ isherman@chromium.org

[Michael van Ouwerkerk, 2015-10-13 15:29:42]

Ilya: could you please take a look at histograms.xml? Thanks!

[Michael van Ouwerkerk, 2015-10-13 15:31:22]

mvanouwerkerk@chromium.org changed reviewers:
+ miguelg@chromium.org

[Michael van Ouwerkerk, 2015-10-13 15:31:22]

Miguel, could you take a look please as owner for ChromeSwitches.java? Thanks!

[Michael van Ouwerkerk, 2015-10-13 16:21:28]

mvanouwerkerk@chromium.org changed reviewers:
+ newt@chromium.org

[Michael van Ouwerkerk, 2015-10-13 16:21:29]

Newton, could you give the UI bits a sanity check please? This is clearly just
the initial form, and while the UI is not final, maybe there are some higher
level things that you could review about this approach. Thanks!

[Miguel Garcia, 2015-10-13 16:35:09]

lgtm for ChromeSwitches.java

[Ilya Sherman, 2015-10-13 19:01:19]

histograms.xml lgtm

[newt (away), 2015-10-14 05:15:49]

Looks reasonable as a starting point. Thanks for the clear overview and
screenshots in the design doc -- that makes reviewing so much easier :)

https://codereview.chromium.org/1394423004/diff/100001/chrome/android/java/re...
File chrome/android/java/res/layout/web_notification.xml (right):

https://codereview.chromium.org/1394423004/diff/100001/chrome/android/java/re...
chrome/android/java/res/layout/web_notification.xml:15: <LinearLayout
xmlns:android="http://schemas.android.com/apk/res/android"
It'd be preferable to use a RelativeLayout here and get rid of all the other
LinearLayouts to reduce nesting. To do that, you'd use attributes on the
children like layout_toEndOf="@id/icon_frame", and layout_alignParentEnd="true".
You could still keep the FrameLayout that contains the two icons though.

https://codereview.chromium.org/1394423004/diff/100001/chrome/android/java/re...
chrome/android/java/res/layout/web_notification.xml:27:
android:scaleType="center"
you probably want "centerInside" to ensure that the image is scaled down if it's
too large to fit in the view.

https://codereview.chromium.org/1394423004/diff/100001/chrome/android/java/re...
chrome/android/java/res/layout/web_notification.xml:31:
android:layout_width="match_parent"
You can put this on the bottom right by making the width and height
wrap_content, and setting layout_gravity="bottom|end" (and you can remove
scaleType)

https://codereview.chromium.org/1394423004/diff/100001/chrome/android/java/re...
File chrome/android/java/res/values-v21/styles.xml (right):

https://codereview.chromium.org/1394423004/diff/100001/chrome/android/java/re...
chrome/android/java/res/values-v21/styles.xml:66:
parent="@android:style/TextAppearance.Material.Notification.Title"/>
Is there a reason to use the framework versions of these styles on L+ (i.e. the
versions with "android:"). I'd expect them to be the same as the app compat
versions of these styles that you're using in values-v17/styles.xml.

OEMs can change the default theme, but can't change the Material theme, AIUI, so
there's no need to worry about these values being different on different
devices.

https://codereview.chromium.org/1394423004/diff/100001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilder.java
(right):

https://codereview.chromium.org/1394423004/diff/100001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilder.java:15:
Notification build();
These methods should be documented somehow. It might be enough to say "@see
NotificationCompat.Builder", since these methods are basically identical to the
methods in NotificationCompat.Builder.

[Michael van Ouwerkerk, 2015-10-16 13:51:10]

Thanks Newton! It looks like you're away for the next 10 days, so I'll ask Dan.

https://codereview.chromium.org/1394423004/diff/100001/chrome/android/java/re...
File chrome/android/java/res/layout/web_notification.xml (right):

https://codereview.chromium.org/1394423004/diff/100001/chrome/android/java/re...
chrome/android/java/res/layout/web_notification.xml:15: <LinearLayout
xmlns:android="http://schemas.android.com/apk/res/android"
On 2015/10/14 05:15:48, newt (OOO through 10-23) wrote:
> It'd be preferable to use a RelativeLayout here and get rid of all the other
> LinearLayouts to reduce nesting. To do that, you'd use attributes on the
> children like layout_toEndOf="@id/icon_frame", and
layout_alignParentEnd="true".
> You could still keep the FrameLayout that contains the two icons though.

Done.

https://codereview.chromium.org/1394423004/diff/100001/chrome/android/java/re...
chrome/android/java/res/layout/web_notification.xml:27:
android:scaleType="center"
On 2015/10/14 05:15:48, newt (OOO through 10-23) wrote:
> you probably want "centerInside" to ensure that the image is scaled down if
it's
> too large to fit in the view.

Done.

https://codereview.chromium.org/1394423004/diff/100001/chrome/android/java/re...
chrome/android/java/res/layout/web_notification.xml:31:
android:layout_width="match_parent"
On 2015/10/14 05:15:48, newt (OOO through 10-23) wrote:
> You can put this on the bottom right by making the width and height
> wrap_content, and setting layout_gravity="bottom|end" (and you can remove
> scaleType)

Done.

https://codereview.chromium.org/1394423004/diff/100001/chrome/android/java/re...
File chrome/android/java/res/values-v21/styles.xml (right):

https://codereview.chromium.org/1394423004/diff/100001/chrome/android/java/re...
chrome/android/java/res/values-v21/styles.xml:66:
parent="@android:style/TextAppearance.Material.Notification.Title"/>
On 2015/10/14 05:15:48, newt (OOO through 10-23) wrote:
> Is there a reason to use the framework versions of these styles on L+ (i.e.
the
> versions with "android:"). I'd expect them to be the same as the app compat
> versions of these styles that you're using in values-v17/styles.xml.
> 
> OEMs can change the default theme, but can't change the Material theme, AIUI,
so
> there's no need to worry about these values being different on different
> devices.

It doesn't compile without the android namespace. Maybe something to explore for
later: does styles_material.xml not get merged in?

https://codereview.chromium.org/1394423004/diff/100001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilder.java
(right):

https://codereview.chromium.org/1394423004/diff/100001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilder.java:15:
Notification build();
On 2015/10/14 05:15:48, newt (OOO through 10-23) wrote:
> These methods should be documented somehow. It might be enough to say "@see
> NotificationCompat.Builder", since these methods are basically identical to
the
> methods in NotificationCompat.Builder.

Done.

[Michael van Ouwerkerk, 2015-10-16 13:57:22]

mvanouwerkerk@chromium.org changed reviewers:
+ dfalcantara@chromium.org

[Michael van Ouwerkerk, 2015-10-16 13:57:22]

Dan, could you take a look please as owner (with set noparent even) of
chrome/android/java/res/ ? Thanks!

[gone, 2015-10-16 17:11:08]

lgtm based on newt's comments being addressed.

[Michael van Ouwerkerk, 2015-10-16 18:37:11]

The CQ bit was checked by mvanouwerkerk@chromium.org

[Michael van Ouwerkerk, 2015-10-16 18:37:12]

The patchset sent to the CQ was uploaded after l-g-t-m from peter@chromium.org,
miguelg@chromium.org, isherman@chromium.org
Link to the patchset: https://codereview.chromium.org/1394423004/#ps140001
(title: "Rebase.")

[commit-bot: I haz the power, 2015-10-16 18:38:40]

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

[commit-bot: I haz the power, 2015-10-16 18:48:49]

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

[commit-bot: I haz the power, 2015-10-16 18:48:50]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Michael van Ouwerkerk, 2015-10-16 19:20:58]

The CQ bit was checked by mvanouwerkerk@chromium.org

[commit-bot: I haz the power, 2015-10-16 19:22:09]

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

[commit-bot: I haz the power, 2015-10-16 19:27:58]

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

[commit-bot: I haz the power, 2015-10-16 19:27:59]

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

[Michael van Ouwerkerk, 2015-10-19 09:58:56]

The CQ bit was checked by mvanouwerkerk@chromium.org

[Michael van Ouwerkerk, 2015-10-19 09:58:57]

The patchset sent to the CQ was uploaded after l-g-t-m from peter@chromium.org,
miguelg@chromium.org, isherman@chromium.org, dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/1394423004/#ps160001
(title: "Rebase.")

[commit-bot: I haz the power, 2015-10-19 09:59:25]

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

[commit-bot: I haz the power, 2015-10-19 11:04:51]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2015-10-19 11:05:33]

Patchset 9 (id:??) landed as
https://crrev.com/4d99f1d9235c49f77d31e6029dd1c5312836e661
Cr-Commit-Position: refs/heads/master@{#354759}