Make the notification builder more XPC friendly

Make the notification builder more XPC friendly

Move it to ui/cocoa/notifications where the xpc service will eventually live.
Make it a static library so it can be linked by both the app and the XPC.
Make sure the build operation only uses information from the underlying
dictionary.

BUG=571056
Committed: https://crrev.com/aafb03bf0df164a2fb86db117ae6053e9578c437
Cr-Commit-Position: refs/heads/master@{#400672}

[Miguel Garcia, 2016-06-13 10:49:59]

Description was changed from

==========
Make the notification builder more XPC friendly

Move it ui/cocoa where the xpc will eventually live
Make it a static library
Make sure the build operation only uses information from the underlying
dictionary.
==========

to

==========
Make the notification builder more XPC friendly

Move it to ui/cocoa/notifications where the xpc service will eventually live.
Make it a static library so it can be linked by both the app and the XPC.
Make sure the build operation only uses information from the underlying
dictionary.
==========

[Miguel Garcia, 2016-06-14 08:51:42]

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

[Miguel Garcia, 2016-06-14 08:51:43]

I will work on the windows failures as soon as I get my hands on a Windows
machine but in the meantime it would be great if you could look at the first
step of the refactor needed for dealing with XPC services.

Full picture CL for reference:
https://chromiumcodereview.appspot.com/2047643003/

[Miguel Garcia, 2016-06-14 11:13:01]

Description was changed from

==========
Make the notification builder more XPC friendly

Move it to ui/cocoa/notifications where the xpc service will eventually live.
Make it a static library so it can be linked by both the app and the XPC.
Make sure the build operation only uses information from the underlying
dictionary.
==========

to

==========
Make the notification builder more XPC friendly

Move it to ui/cocoa/notifications where the xpc service will eventually live.
Make it a static library so it can be linked by both the app and the XPC.
Make sure the build operation only uses information from the underlying
dictionary.

BUG=571056
==========

[Robert Sesek, 2016-06-14 14:06:31]

https://codereview.chromium.org/2053613002/diff/40001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/notifications/BUILD.gn (right):

https://codereview.chromium.org/2053613002/diff/40001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/notifications/BUILD.gn:5:
static_library("notification_common") {
This can just be "common" in GN.

https://codereview.chromium.org/2053613002/diff/40001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/notifications/notification_common.gyp (right):

https://codereview.chromium.org/2053613002/diff/40001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/notifications/notification_common.gyp:11: 'type':
'static_library',
GYP uses 2-space indents.

https://codereview.chromium.org/2053613002/diff/40001/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

https://codereview.chromium.org/2053613002/diff/40001/chrome/chrome_browser.g...
chrome/chrome_browser.gypi:3740: 'dependencies' : [
Same.

[Miguel Garcia, 2016-06-15 14:43:31]

https://chromiumcodereview.appspot.com/2053613002/diff/40001/chrome/browser/u...
File chrome/browser/ui/cocoa/notifications/BUILD.gn (right):

https://chromiumcodereview.appspot.com/2053613002/diff/40001/chrome/browser/u...
chrome/browser/ui/cocoa/notifications/BUILD.gn:5:
static_library("notification_common") {
On 2016/06/14 14:06:30, Robert Sesek wrote:
> This can just be "common" in GN.

Done.

https://chromiumcodereview.appspot.com/2053613002/diff/40001/chrome/browser/u...
File chrome/browser/ui/cocoa/notifications/notification_common.gyp (right):

https://chromiumcodereview.appspot.com/2053613002/diff/40001/chrome/browser/u...
chrome/browser/ui/cocoa/notifications/notification_common.gyp:11: 'type':
'static_library',
On 2016/06/14 14:06:31, Robert Sesek wrote:
> GYP uses 2-space indents.

Done.

https://chromiumcodereview.appspot.com/2053613002/diff/40001/chrome/chrome_br...
File chrome/chrome_browser.gypi (right):

https://chromiumcodereview.appspot.com/2053613002/diff/40001/chrome/chrome_br...
chrome/chrome_browser.gypi:3740: 'dependencies' : [
On 2016/06/14 14:06:31, Robert Sesek wrote:
> Same.

Done.

[Miguel Garcia, 2016-06-15 15:28:39]

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

[Miguel Garcia, 2016-06-15 15:28:39]

+Jochen for the BUILD.gn file

Jochen let me know if you need additional context on why adding a static library
is needed. In a nutshell we are planning to support XPC services in Chrome mac
so we can send different kind of native notifications. Some more info in
https://docs.google.com/document/d/1h7eNZbdFyrM6r6taJxHuW-GQm34FJSFqmfwZkEj3e...

BTW It's strange that I can do the gyp changes without a review but not the
BUILD.gn ones. Is that on purpose?

[Peter Beverloo, 2016-06-16 17:01:30]

From the notifications side this lgtm, but I don't think it's common to have
sub-libraries living in //chrome/browser/. 

Wouldn't this be better suited in //chrome/common/?

https://codereview.chromium.org/2053613002/diff/60001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/notifications/BUILD.gn (right):

https://codereview.chromium.org/2053613002/diff/60001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/notifications/BUILD.gn:12: "//ui/base:base",
+//base (various stuff from //base/mac/)

Probably //chrome:strings too due to grit?

https://codereview.chromium.org/2053613002/diff/60001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/notifications/notification_common.gyp (right):

https://codereview.chromium.org/2053613002/diff/60001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/notifications/notification_common.gyp:7: 'targets': [
micro nits: (a) no blank line on :6, (b) 2-space indent on :7, (c) +2-space
indent elsewhere, (d) everything in gyp ends with a comma (:19, :20)

[Miguel Garcia, 2016-06-17 12:44:46]

Robert can you have a final look?

https://codereview.chromium.org/2053613002/diff/60001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/notifications/BUILD.gn (right):

https://codereview.chromium.org/2053613002/diff/60001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/notifications/BUILD.gn:12: "//ui/base:base",
On 2016/06/16 17:01:30, Peter Beverloo - OOO June 8-16 wrote:
> +//base (various stuff from //base/mac/)
> 
> Probably //chrome:strings too due to grit?

Done.

https://codereview.chromium.org/2053613002/diff/60001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/notifications/notification_common.gyp (right):

https://codereview.chromium.org/2053613002/diff/60001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/notifications/notification_common.gyp:7: 'targets': [
On 2016/06/16 17:01:30, Peter Beverloo - OOO June 8-16 wrote:
> micro nits: (a) no blank line on :6, (b) 2-space indent on :7, (c) +2-space
> indent elsewhere, (d) everything in gyp ends with a comma (:19, :20)

Done.

[Robert Sesek, 2016-06-17 12:46:16]

LGTM % one thing

https://codereview.chromium.org/2053613002/diff/80001/build/config/mac/rules.gni
File build/config/mac/rules.gni (left):

https://codereview.chromium.org/2053613002/diff/80001/build/config/mac/rules....
build/config/mac/rules.gni:341: product_type =
"com.apple.product-type.application"
Remove this change.

[Miguel Garcia, 2016-06-17 17:10:49]

Will wait for jochen to LGTM the BUILD.gn file before landing it. (Even though
the equivalent gyp changes don't need a chrome/OWNER review)

https://chromiumcodereview.appspot.com/2053613002/diff/80001/build/config/mac...
File build/config/mac/rules.gni (left):

https://chromiumcodereview.appspot.com/2053613002/diff/80001/build/config/mac...
build/config/mac/rules.gni:341: product_type =
"com.apple.product-type.application"
On 2016/06/17 12:46:16, Robert Sesek wrote:
> Remove this change.

Done.

[jochen (gone - plz use gerrit), 2016-06-20 11:37:07]

lgtm

[Miguel Garcia, 2016-06-20 12:46:50]

The CQ bit was checked by miguelg@chromium.org

[Miguel Garcia, 2016-06-20 12:46:50]

The patchset sent to the CQ was uploaded after l-g-t-m from peter@chromium.org,
rsesek@chromium.org
Link to the patchset:
https://chromiumcodereview.appspot.com/2053613002/#ps100001 (title: "revert
unintended change")

[commit-bot: I haz the power, 2016-06-20 12:47:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2053613002/100001

[commit-bot: I haz the power, 2016-06-20 14:15:04]

Description was changed from

==========
Make the notification builder more XPC friendly

Move it to ui/cocoa/notifications where the xpc service will eventually live.
Make it a static library so it can be linked by both the app and the XPC.
Make sure the build operation only uses information from the underlying
dictionary.

BUG=571056
==========

to

==========
Make the notification builder more XPC friendly

Move it to ui/cocoa/notifications where the xpc service will eventually live.
Make it a static library so it can be linked by both the app and the XPC.
Make sure the build operation only uses information from the underlying
dictionary.

BUG=571056
==========

[commit-bot: I haz the power, 2016-06-20 14:15:05]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-06-20 14:17:00]

Description was changed from

==========
Make the notification builder more XPC friendly

Move it to ui/cocoa/notifications where the xpc service will eventually live.
Make it a static library so it can be linked by both the app and the XPC.
Make sure the build operation only uses information from the underlying
dictionary.

BUG=571056
==========

to

==========
Make the notification builder more XPC friendly

Move it to ui/cocoa/notifications where the xpc service will eventually live.
Make it a static library so it can be linked by both the app and the XPC.
Make sure the build operation only uses information from the underlying
dictionary.

BUG=571056

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

[commit-bot: I haz the power, 2016-06-20 14:17:01]

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

[vasilii, 2016-06-20 14:54:43]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/2080733002/ by vasilii@chromium.org.

The reason for reverting is: Broke compilation on Mac
https://build.chromium.org/p/chromium/builders/Mac/builds/16903/steps/compile...

../../chrome/browser/ui/cocoa/notifications/notification_builder_mac.mm:11:10:
fatal error: 'chrome/grit/generated_resources.h' file not found
#include "chrome/grit/generated_resources.h".