Add Notification images (Android without custom layouts)

Add Notification images (Android without custom layouts)

Behind --enable-experimental-web-platform-features flag.
Not yet hooked up on Android Marshmallow or below, which
use custom layouts for notifications.

Intent to implement:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/FQxPB5GEQjo/discussion

Screenshot: https://imgur.com/a/SM3tq

Depends on https://codereview.chromium.org/2261283002

BUG=614456
Committed: https://crrev.com/bcfbd47c643580867f4466d80acb0173925a91c8
Cr-Commit-Position: refs/heads/master@{#414466}

[johnme, 2016-08-24 13:36:29]

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

[johnme, 2016-08-24 13:37:08]

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

[commit-bot: I haz the power, 2016-08-24 13:37:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-24 13:49:35]

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

[commit-bot: I haz the power, 2016-08-24 13:49:37]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[Peter Beverloo, 2016-08-24 13:57:41]

lgtm, would you mind adding a screenshot to the description?

https://codereview.chromium.org/2273033002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/notifications/StandardNotificationBuilder.java
(right):

https://codereview.chromium.org/2273033002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/notifications/StandardNotificationBuilder.java:33:
if (Build.VERSION.CODENAME.equals("N")
Do we still need the CODENAME check? That should be a thing of the past, flash
your phone? :)

https://codereview.chromium.org/2273033002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/notifications/StandardNotificationBuilder.java:37:
style.setSummaryText(mBody);
Is there any reason we can't just always set this? Does it yield odd results for
older versions of Android where custom layouts are disabled? (Do we even care
about that?)

https://codereview.chromium.org/2273033002/diff/1/chrome/android/javatests/sr...
File
chrome/android/javatests/src/org/chromium/chrome/browser/notifications/StandardNotificationBuilderTest.java
(right):

https://codereview.chromium.org/2273033002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/notifications/StandardNotificationBuilderTest.java:31:
PendingIntent[] outContentAndDeleteIntents) {
Can we somehow assert that |outContentAndDeleteIntents.length==2|?

[johnme, 2016-08-24 17:42:13]

Addressed review comments - thanks :)

> would you mind adding a screenshot to the description?

Ack, will do that before landing.

https://codereview.chromium.org/2273033002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/notifications/StandardNotificationBuilder.java
(right):

https://codereview.chromium.org/2273033002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/notifications/StandardNotificationBuilder.java:33:
if (Build.VERSION.CODENAME.equals("N")
On 2016/08/24 13:57:41, Peter Beverloo wrote:
> Do we still need the CODENAME check? That should be a thing of the past, flash
> your phone? :)

10/12 existing checks in the codebase include the CODENAME check for N. I'd
rather keep it for now, to avoid confusing people who test this with N DR
devices (which are out there in the wild this time).

https://codereview.chromium.org/2273033002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/notifications/StandardNotificationBuilder.java:37:
style.setSummaryText(mBody);
On 2016/08/24 13:57:41, Peter Beverloo wrote:
> Is there any reason we can't just always set this? Does it yield odd results
for
> older versions of Android where custom layouts are disabled? (Do we even care
> about that?)

Yes :-( Android versions less than N show the ContentText when the
BigPictureStyle notification is expanded. Android N does not, so we have to use
the summary text to compensate. If we always set it, it will be shown twice for
< N.

https://codereview.chromium.org/2273033002/diff/1/chrome/android/javatests/sr...
File
chrome/android/javatests/src/org/chromium/chrome/browser/notifications/StandardNotificationBuilderTest.java
(right):

https://codereview.chromium.org/2273033002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/notifications/StandardNotificationBuilderTest.java:31:
PendingIntent[] outContentAndDeleteIntents) {
On 2016/08/24 13:57:41, Peter Beverloo wrote:
> Can we somehow assert that |outContentAndDeleteIntents.length==2|?

Done.

[johnme, 2016-08-24 17:42:20]

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

[commit-bot: I haz the power, 2016-08-24 17:43:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-24 20:03:32]

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

[commit-bot: I haz the power, 2016-08-24 20:03:33]

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

[johnme, 2016-08-24 21:02:46]

Description was changed from

==========
Add Notification images (Android without custom layouts)

Depends on https://codereview.chromium.org/2261283002

BUG=614456
==========

to

==========
Add Notification images (Android without custom layouts)

Behind --enable-experimental-web-platform-features flag.
Not yet hooked up on Android Marshmallow or below, which
use custom layouts for notifications.

Intent to implement:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/FQxPB5GEQjo/discus...

Screenshot: https://imgur.com/a/SM3tq

Depends on https://codereview.chromium.org/2261283002

BUG=614456
==========

[johnme, 2016-08-25 16:42:01]

The CQ bit was checked by johnme@chromium.org

[johnme, 2016-08-25 16:42:02]

The patchset sent to the CQ was uploaded after l-g-t-m from peter@chromium.org
Link to the patchset: https://codereview.chromium.org/2273033002/#ps40001
(title: "Fix EXTRA_PICTURE test")

[commit-bot: I haz the power, 2016-08-25 16:42:33]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-25 17:34:37]

Description was changed from

==========
Add Notification images (Android without custom layouts)

Behind --enable-experimental-web-platform-features flag.
Not yet hooked up on Android Marshmallow or below, which
use custom layouts for notifications.

Intent to implement:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/FQxPB5GEQjo/discus...

Screenshot: https://imgur.com/a/SM3tq

Depends on https://codereview.chromium.org/2261283002

BUG=614456
==========

to

==========
Add Notification images (Android without custom layouts)

Behind --enable-experimental-web-platform-features flag.
Not yet hooked up on Android Marshmallow or below, which
use custom layouts for notifications.

Intent to implement:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/FQxPB5GEQjo/discus...

Screenshot: https://imgur.com/a/SM3tq

Depends on https://codereview.chromium.org/2261283002

BUG=614456
==========

[commit-bot: I haz the power, 2016-08-25 17:34:38]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-25 17:38:21]

Description was changed from

==========
Add Notification images (Android without custom layouts)

Behind --enable-experimental-web-platform-features flag.
Not yet hooked up on Android Marshmallow or below, which
use custom layouts for notifications.

Intent to implement:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/FQxPB5GEQjo/discus...

Screenshot: https://imgur.com/a/SM3tq

Depends on https://codereview.chromium.org/2261283002

BUG=614456
==========

to

==========
Add Notification images (Android without custom layouts)

Behind --enable-experimental-web-platform-features flag.
Not yet hooked up on Android Marshmallow or below, which
use custom layouts for notifications.

Intent to implement:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/FQxPB5GEQjo/discus...

Screenshot: https://imgur.com/a/SM3tq

Depends on https://codereview.chromium.org/2261283002

BUG=614456

Committed: https://crrev.com/bcfbd47c643580867f4466d80acb0173925a91c8
Cr-Commit-Position: refs/heads/master@{#414466}
==========

[commit-bot: I haz the power, 2016-08-25 17:38:22]

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