Linux native notifications: Add desktop-entry hint

Linux native notifications: Add desktop-entry hint

This CL adds a desktop-entry hint so that the notification server can
use the app's name and icon if it chooses to.

BUG=676220
R=thestig@chromium.org

Review-Url: https://codereview.chromium.org/2811183009
Cr-Commit-Position: refs/heads/master@{#465138}
Committed: https://chromium.googlesource.com/chromium/src/+/196a305434d54e05c519f1f26e129d7cf2f863ba

[Tom (Use chromium acct), 2017-04-13 20:46:40]

thestig ptal

[Lei Zhang, 2017-04-14 01:23:10]

lgtm

https://codereview.chromium.org/2811183009/diff/20001/chrome/browser/notifica...
File chrome/browser/notifications/notification_platform_bridge_linux.cc (right):

https://codereview.chromium.org/2811183009/diff/20001/chrome/browser/notifica...
chrome/browser/notifications/notification_platform_bridge_linux.cc:391:
desktop_file.substr(0, desktop_file.size() - strlen(kDesktopFileSuffix));
Alternatively, maybe make |desktop_file| a base::FilePath, and then you can use
RemoveFinalExtension()?

[Tom (Use chromium acct), 2017-04-18 03:43:05]

The CQ bit was checked by thomasanderson@google.com

[Tom (Use chromium acct), 2017-04-18 03:43:05]

The patchset sent to the CQ was uploaded after l-g-t-m from thestig@chromium.org
Link to the patchset: https://codereview.chromium.org/2811183009/#ps60001
(title: "RemoveFinalExtension")

[commit-bot: I haz the power, 2017-04-18 03:43:33]

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

[Tom (Use chromium acct), 2017-04-18 03:46:40]

https://codereview.chromium.org/2811183009/diff/20001/chrome/browser/notifica...
File chrome/browser/notifications/notification_platform_bridge_linux.cc (right):

https://codereview.chromium.org/2811183009/diff/20001/chrome/browser/notifica...
chrome/browser/notifications/notification_platform_bridge_linux.cc:391:
desktop_file.substr(0, desktop_file.size() - strlen(kDesktopFileSuffix));
On 2017/04/14 01:23:10, Lei Zhang wrote:
> Alternatively, maybe make |desktop_file| a base::FilePath, and then you can
use
> RemoveFinalExtension()?

Done.

[commit-bot: I haz the power, 2017-04-18 04:34:04]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1492486985150100,
"parent_rev": "4b267ce12fab17dfa38615933571eae96faf63f1", "commit_rev":
"196a305434d54e05c519f1f26e129d7cf2f863ba"}

[commit-bot: I haz the power, 2017-04-18 04:34:37]

Description was changed from

==========
Linux native notifications: Add desktop-entry hint

This CL adds a desktop-entry hint so that the notification server can
use the app's name and icon if it chooses to.

BUG=676220
R=thestig@chromium.org
==========

to

==========
Linux native notifications: Add desktop-entry hint

This CL adds a desktop-entry hint so that the notification server can
use the app's name and icon if it chooses to.

BUG=676220
R=thestig@chromium.org

Review-Url: https://codereview.chromium.org/2811183009
Cr-Commit-Position: refs/heads/master@{#465138}
Committed:
https://chromium.googlesource.com/chromium/src/+/196a305434d54e05c519f1f26e12...
==========

[commit-bot: I haz the power, 2017-04-18 04:34:40]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/196a305434d54e05c519f1f26e12...

[Peter Beverloo, 2017-04-18 10:14:41]

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

[Peter Beverloo, 2017-04-18 10:14:41]

lgtm

https://codereview.chromium.org/2811183009/diff/60001/chrome/browser/notifica...
File chrome/browser/notifications/notification_platform_bridge_linux.cc (right):

https://codereview.chromium.org/2811183009/diff/60001/chrome/browser/notifica...
chrome/browser/notifications/notification_platform_bridge_linux.cc:391:
base::CompareCase::SENSITIVE));
We could hit this DCHECK for non-official builds with the $CHROME_DESKTOP
environment variable set. As I understand it, many Linux distributions
distribute non-official builds. They do so in Release mode - that means this
wouldn't hit, but the call on line 393 might do the wrong thing. Are we at all
concerned about this? (No is a fine answer.)

[Tom (Use chromium acct), 2017-04-18 18:04:56]

https://codereview.chromium.org/2811183009/diff/60001/chrome/browser/notifica...
File chrome/browser/notifications/notification_platform_bridge_linux.cc (right):

https://codereview.chromium.org/2811183009/diff/60001/chrome/browser/notifica...
chrome/browser/notifications/notification_platform_bridge_linux.cc:391:
base::CompareCase::SENSITIVE));
On 2017/04/18 10:14:41, Peter Beverloo wrote:
> We could hit this DCHECK for non-official builds with the $CHROME_DESKTOP
> environment variable set. As I understand it, many Linux distributions
> distribute non-official builds. They do so in Release mode - that means this
> wouldn't hit, but the call on line 393 might do the wrong thing. Are we at all
> concerned about this? (No is a fine answer.)

No, the FDO desktop entry spec requires the files to have a .desktop extension.
https://standards.freedesktop.org/desktop-entry-spec/desktop-entry-spec-lates...