Notification custom layouts: improve origin display.

Notification custom layouts: improve origin display.

* Fix spacing
* Ellipsize from start as that part is less important
* Apply official formatting

Screenshot: http://imgur.com/bJy8hoH

BUG=541617
Committed: https://crrev.com/48bc0eb1fb759e158e4c9e766dd012d4de477b2f
Cr-Commit-Position: refs/heads/master@{#357125}

[Michael van Ouwerkerk, 2015-10-29 18:53:22]

mvanouwerkerk@chromium.org changed reviewers:
+ johnme@chromium.org, miguelg@chromium.org, newt@chromium.org

[Michael van Ouwerkerk, 2015-10-29 18:53:23]

John and Miguel: please review for origin display correctness.
Newton: please review as UI owner.

[johnme, 2015-10-29 19:13:58]

lgtm - omitting the scheme seems to be best practice under "If the feature or
behavior we are trying to protect is available only on authenticated origins —
which we strongly suggest — you could leave off the scheme or the icon."[1],
which is true for Android notifications (it's not true on desktop, where
notifications are also supported over HTTP).

(cc'ing palmer anyway in case we're missing some subtlety)

[1]:
https://www.chromium.org/Home/chromium-security/enamel#TOC-Presenting-Origins...

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

https://codereview.chromium.org/1426453003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java:593:
URI parsedUrl = new URI(origin);
Optional nit: UrlUtilities.formatUrlForSecurityDisplay should probably just take
a String, since it converts it back to a string then parses it in C++.

[newt (away), 2015-10-29 21:07:56]

lgtm after John's comments are addressed

[Michael van Ouwerkerk, 2015-10-30 10:29:50]

Thanks for the quick review.

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

https://codereview.chromium.org/1426453003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java:593:
URI parsedUrl = new URI(origin);
On 2015/10/29 19:13:58, johnme wrote:
> Optional nit: UrlUtilities.formatUrlForSecurityDisplay should probably just
take
> a String, since it converts it back to a string then parses it in C++.

Done, as an overload. Some of the call sites have specific handling for uris
that fail to parse, so I didn't want to change those.

[Michael van Ouwerkerk, 2015-10-30 10:29:54]

The CQ bit was checked by mvanouwerkerk@chromium.org

[Michael van Ouwerkerk, 2015-10-30 10:29:54]

The patchset sent to the CQ was uploaded after l-g-t-m from johnme@chromium.org,
newt@chromium.org
Link to the patchset: https://codereview.chromium.org/1426453003/#ps20001
(title: "Make formatUrlForSecurityDisplay accept a String.")

[commit-bot: I haz the power, 2015-10-30 10:30:15]

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

[commit-bot: I haz the power, 2015-10-30 12:17:16]

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

[commit-bot: I haz the power, 2015-10-30 12:17:16]

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

[Michael van Ouwerkerk, 2015-10-30 16:00:50]

Description was changed from

==========
Notification custom layouts: improve origin display.

* Fix spacing
* Ellipsize from start as that part is less important
* Apply official formatting

Screenshot: http://imgur.com/bJy8hoH

BUG=541617
==========

to

==========
Notification custom layouts: improve origin display.

* Fix spacing
* Ellipsize from start as that part is less important
* Apply official formatting

Screenshot: http://imgur.com/bJy8hoH

BUG=541617
==========

[Michael van Ouwerkerk, 2015-10-30 16:00:50]

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

[Michael van Ouwerkerk, 2015-10-30 16:06:47]

The CQ bit was checked by mvanouwerkerk@chromium.org

[Michael van Ouwerkerk, 2015-10-30 16:06:47]

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

[commit-bot: I haz the power, 2015-10-30 16:07:16]

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

[commit-bot: I haz the power, 2015-10-30 16:49:28]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-10-30 16:50:26]

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

[palmer, 2015-10-30 21:09:30]

palmer@chromium.org changed reviewers:
+ palmer@chromium.org

[palmer, 2015-10-30 21:09:30]

LGTM, thanks.