Transfer the notification icon through XPC as NSData for OSX 10.09

Transfer the notification icon through XPC as NSData for OSX 10.09

This is because NSImage only started conforming to NSecureCoding in 10.10

BUG=696759
Review-Url: https://codereview.chromium.org/2728173003
Cr-Commit-Position: refs/heads/master@{#454916}
Committed: https://chromium.googlesource.com/chromium/src/+/52f904d3bf07e423a6bb907ff6f344935532affa

[Miguel Garcia, 2017-03-04 11:41:39]

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

[commit-bot: I haz the power, 2017-03-04 11:41:46]

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

[Miguel Garcia, 2017-03-04 11:42:48]

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

[Miguel Garcia, 2017-03-04 11:42:49]

I tested the 10.09 path by forcing it in 10.13 I will do a final test on Monday
on an actual 10.09 laptop

[commit-bot: I haz the power, 2017-03-04 11:58:37]

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

[commit-bot: I haz the power, 2017-03-04 11:58:38]

Dry run: This issue passed the CQ dry run.

[Miguel Garcia, 2017-03-06 11:01:11]

Ok verified in 10.09 now. Works great

[Robert Sesek, 2017-03-06 16:12:05]

https://codereview.chromium.org/2728173003/diff/1/chrome/browser/ui/cocoa/not...
File chrome/browser/ui/cocoa/notifications/notification_builder_mac.mm (right):

https://codereview.chromium.org/2728173003/diff/1/chrome/browser/ui/cocoa/not...
chrome/browser/ui/cocoa/notifications/notification_builder_mac.mm:74: if
(base::mac::IsAtLeastOS10_10()) {
Rather than using base::mac, use [icon
conformsToProtocol:@protocol(NSSecureCoding)]

https://codereview.chromium.org/2728173003/diff/1/chrome/browser/ui/cocoa/not...
chrome/browser/ui/cocoa/notifications/notification_builder_mac.mm:76: } else { 
// in 10.09 NSImage does not conform to NSecureCoding
It's just "10.9" -- also use proper punctuation and capitalization.

https://codereview.chromium.org/2728173003/diff/1/chrome/browser/ui/cocoa/not...
chrome/browser/ui/cocoa/notifications/notification_builder_mac.mm:139: } else { 
// in 10.09 NSImage does not confirm to NSSecureCoding
Same.

[Miguel Garcia, 2017-03-06 16:30:43]

https://codereview.chromium.org/2728173003/diff/1/chrome/browser/ui/cocoa/not...
File chrome/browser/ui/cocoa/notifications/notification_builder_mac.mm (right):

https://codereview.chromium.org/2728173003/diff/1/chrome/browser/ui/cocoa/not...
chrome/browser/ui/cocoa/notifications/notification_builder_mac.mm:74: if
(base::mac::IsAtLeastOS10_10()) {
On 2017/03/06 16:12:05, Robert Sesek wrote:
> Rather than using base::mac, use [icon
> conformsToProtocol:@protocol(NSSecureCoding)]

So the problem is that down in line 136 I don't have an icon to compare against
so I think I should keep the 10.9 check in there which means (for consistency I
should probably do it here too. Let me know what you think)

https://codereview.chromium.org/2728173003/diff/1/chrome/browser/ui/cocoa/not...
chrome/browser/ui/cocoa/notifications/notification_builder_mac.mm:76: } else { 
// in 10.09 NSImage does not conform to NSecureCoding
On 2017/03/06 16:12:05, Robert Sesek wrote:
> It's just "10.9" -- also use proper punctuation and capitalization.

Done.

https://codereview.chromium.org/2728173003/diff/1/chrome/browser/ui/cocoa/not...
chrome/browser/ui/cocoa/notifications/notification_builder_mac.mm:139: } else { 
// in 10.09 NSImage does not confirm to NSSecureCoding
On 2017/03/06 16:12:05, Robert Sesek wrote:
> Same.

Done.

[Robert Sesek, 2017-03-06 17:34:59]

https://codereview.chromium.org/2728173003/diff/1/chrome/browser/ui/cocoa/not...
File chrome/browser/ui/cocoa/notifications/notification_builder_mac.mm (right):

https://codereview.chromium.org/2728173003/diff/1/chrome/browser/ui/cocoa/not...
chrome/browser/ui/cocoa/notifications/notification_builder_mac.mm:74: if
(base::mac::IsAtLeastOS10_10()) {
On 2017/03/06 16:30:42, Miguel Garcia wrote:
> On 2017/03/06 16:12:05, Robert Sesek wrote:
> > Rather than using base::mac, use [icon
> > conformsToProtocol:@protocol(NSSecureCoding)]
> 
> So the problem is that down in line 136 I don't have an icon to compare
against
> so I think I should keep the 10.9 check in there which means (for consistency
I
> should probably do it here too. Let me know what you think)

You can check the Class too:
https://developer.apple.com/reference/objectivec/nsobject/1418893-conformstop...

[Miguel Garcia, 2017-03-06 18:15:02]

On 2017/03/06 17:34:59, Robert Sesek wrote:
>
https://codereview.chromium.org/2728173003/diff/1/chrome/browser/ui/cocoa/not...
> File chrome/browser/ui/cocoa/notifications/notification_builder_mac.mm
(right):
> 
>
https://codereview.chromium.org/2728173003/diff/1/chrome/browser/ui/cocoa/not...
> chrome/browser/ui/cocoa/notifications/notification_builder_mac.mm:74: if
> (base::mac::IsAtLeastOS10_10()) {
> On 2017/03/06 16:30:42, Miguel Garcia wrote:
> > On 2017/03/06 16:12:05, Robert Sesek wrote:
> > > Rather than using base::mac, use [icon
> > > conformsToProtocol:@protocol(NSSecureCoding)]
> > 
> > So the problem is that down in line 136 I don't have an icon to compare
> against
> > so I think I should keep the 10.9 check in there which means (for
consistency
> I
> > should probably do it here too. Let me know what you think)
> 
> You can check the Class too:
>
https://developer.apple.com/reference/objectivec/nsobject/1418893-conformstop...

Ah great, done now.

[Robert Sesek, 2017-03-06 18:25:32]

LGTM

[Miguel Garcia, 2017-03-06 18:40:02]

The CQ bit was checked by miguelg@chromium.org

[commit-bot: I haz the power, 2017-03-06 18:40:25]

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, 2017-03-06 19:30:40]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1488825602683380,
"parent_rev": "3ae95f98ff637f8cb94de44b82d20d81e7135358", "commit_rev":
"52f904d3bf07e423a6bb907ff6f344935532affa"}

[commit-bot: I haz the power, 2017-03-06 19:31:23]

Description was changed from

==========
Transfer the notification icon through XPC as NSData for OSX 10.09

This is because NSImage only started conforming to NSecureCoding in 10.10

BUG=696759
==========

to

==========
Transfer the notification icon through XPC as NSData for OSX 10.09

This is because NSImage only started conforming to NSecureCoding in 10.10

BUG=696759

Review-Url: https://codereview.chromium.org/2728173003
Cr-Commit-Position: refs/heads/master@{#454916}
Committed:
https://chromium.googlesource.com/chromium/src/+/52f904d3bf07e423a6bb907ff6f3...
==========

[commit-bot: I haz the power, 2017-03-06 19:31:24]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/52f904d3bf07e423a6bb907ff6f3...