🌞 Upstream targets for building native libraries for chrome/monochrome.

Upstream targets for building native libraries for chrome/monochrome.

These no longer contain private sources so can be build upstream.
Lots of clean-up and unification remains but this is the first
step to building these targets upstream.
BUG=607207
Committed: https://crrev.com/561f24772cc36822699e90e7ee3842f7c301b0cb
Cr-Commit-Position: refs/heads/master@{#391501}

[Yaron, 2016-05-03 15:07:24]

Description was changed from

==========
Upstream targets for building official libraries for native chrome/monochrome.

These no longer contain private sources so can be build upstream.
Lots of clean-up and unification remains but this is the first
step to building these targets upstream.
BUG=607207
==========

to

==========
Upstream targets for building natiive libraries for chrome/monochrome.

These no longer contain private sources so can be build upstream.
Lots of clean-up and unification remains but this is the first
step to building these targets upstream.
BUG=607207
==========

[Yaron, 2016-05-03 15:17:23]

yfriedman@chromium.org changed reviewers:
+ agrieve@chromium.org, torne@chromium.org

[Yaron, 2016-05-03 15:17:24]

agrive - chrome builds
torne - webview bits (specifically the dep)

+sgurun/michaelbai FYI (or if you have issues)

My plan was to land this and the corresponding internal CL. Once that's done I
can proceed to clean-up internal machinery and should be done EOW.

Verified that everything worked with a clean build for  chrome_apk
chrome_test_apk chrome_public_apk chrome_public_test_apk monochrome

[Yaron, 2016-05-03 15:18:52]

Description was changed from

==========
Upstream targets for building natiive libraries for chrome/monochrome.

These no longer contain private sources so can be build upstream.
Lots of clean-up and unification remains but this is the first
step to building these targets upstream.
BUG=607207
==========

to

==========
Upstream targets for building native libraries for chrome/monochrome.

These no longer contain private sources so can be build upstream.
Lots of clean-up and unification remains but this is the first
step to building these targets upstream.
BUG=607207
==========

[Yaron, 2016-05-03 15:19:41]

https://codereview.chromium.org/1948503002/diff/20001/chrome/android/BUILD.gn
File chrome/android/BUILD.gn (right):

https://codereview.chromium.org/1948503002/diff/20001/chrome/android/BUILD.gn...
chrome/android/BUILD.gn:457: shared_library("monochrome") {
err, I also omitted the "lib" prefix in these versions which I thought is
redundant in GN

[agrieve, 2016-05-03 15:39:50]

lgtm

[michaelbai, 2016-05-03 19:30:17]

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

[michaelbai, 2016-05-03 19:30:18]

https://codereview.chromium.org/1948503002/diff/20001/chrome/android/BUILD.gn
File chrome/android/BUILD.gn (right):

https://codereview.chromium.org/1948503002/diff/20001/chrome/android/BUILD.gn...
chrome/android/BUILD.gn:466: deps += [ "//tools/cygprofile" ]
Question, Is this sufficient to use order profiling? Any other needed?

https://codereview.chromium.org/1948503002/diff/20001/chrome/browser/android/...
File chrome/browser/android/monochrome_entry_point.cc (right):

https://codereview.chromium.org/1948503002/diff/20001/chrome/browser/android/...
chrome/browser/android/monochrome_entry_point.cc:21: }  // namespace
We don't need above two methods, right?, there is no warning?

[Yaron, 2016-05-03 20:14:49]

https://codereview.chromium.org/1948503002/diff/20001/chrome/android/BUILD.gn
File chrome/android/BUILD.gn (right):

https://codereview.chromium.org/1948503002/diff/20001/chrome/android/BUILD.gn...
chrome/android/BUILD.gn:466: deps += [ "//tools/cygprofile" ]
On 2016/05/03 19:30:17, michaelbai wrote:
> Question, Is this sufficient to use order profiling? Any other needed?

the shared library config below is the key thing. you can see the definition of
it in the internal repo:
https://chrome-internal-review.googlesource.com/#/c/257658/

https://codereview.chromium.org/1948503002/diff/20001/chrome/browser/android/...
File chrome/browser/android/monochrome_entry_point.cc (right):

https://codereview.chromium.org/1948503002/diff/20001/chrome/browser/android/...
chrome/browser/android/monochrome_entry_point.cc:21: }  // namespace
On 2016/05/03 19:30:17, michaelbai wrote:
> We don't need above two methods, right?, there is no warning?

the chrome case does need them - note the callbacks passed.

[michaelbai, 2016-05-03 20:54:01]

just one question about order profiling, see comment

https://codereview.chromium.org/1948503002/diff/20001/chrome/android/BUILD.gn
File chrome/android/BUILD.gn (right):

https://codereview.chromium.org/1948503002/diff/20001/chrome/android/BUILD.gn...
chrome/android/BUILD.gn:466: deps += [ "//tools/cygprofile" ]
On 2016/05/03 20:14:48, Yaron wrote:
> On 2016/05/03 19:30:17, michaelbai wrote:
> > Question, Is this sufficient to use order profiling? Any other needed?
> 
> the shared library config below is the key thing. you can see the definition
of
> it in the internal repo:
> https://chrome-internal-review.googlesource.com/#/c/257658/

I have never applied the order profiling to monochrome, should it just work?

https://codereview.chromium.org/1948503002/diff/20001/chrome/browser/android/...
File chrome/browser/android/monochrome_entry_point.cc (right):

https://codereview.chromium.org/1948503002/diff/20001/chrome/browser/android/...
chrome/browser/android/monochrome_entry_point.cc:21: }  // namespace
On 2016/05/03 20:14:48, Yaron wrote:
> On 2016/05/03 19:30:17, michaelbai wrote:
> > We don't need above two methods, right?, there is no warning?
> 
> the chrome case does need them - note the callbacks passed.

I see, you changed it to call the chrome one, but If you use way in clank, you
basically save one copy of these methods, but it is trivial.

[Torne, 2016-05-04 12:33:05]

LGTM, just one question

https://codereview.chromium.org/1948503002/diff/20001/chrome/android/BUILD.gn
File chrome/android/BUILD.gn (right):

https://codereview.chromium.org/1948503002/diff/20001/chrome/android/BUILD.gn...
chrome/android/BUILD.gn:457: shared_library("monochrome") {
On 2016/05/03 15:19:41, Yaron wrote:
> err, I also omitted the "lib" prefix in these versions which I thought is
> redundant in GN

It's also redundant in gyp ;)

https://codereview.chromium.org/1948503002/diff/20001/chrome/browser/android/...
File chrome/browser/android/DEPS (right):

https://codereview.chromium.org/1948503002/diff/20001/chrome/browser/android/...
chrome/browser/android/DEPS:17: ".*monochrome_entry_point\.cc": [
Why is this .* ? Not familiar with this rule syntax so not sure what it's
matching against, but it looks weird.

[Torne, 2016-05-04 12:33:05]

LGTM, just one question

[Yaron, 2016-05-04 13:07:28]

The CQ bit was checked by yfriedman@chromium.org

[Yaron, 2016-05-04 13:07:28]

The patchset sent to the CQ was uploaded after l-g-t-m from
agrieve@chromium.org, torne@chromium.org
Link to the patchset: https://codereview.chromium.org/1948503002/#ps40001
(title: "fixdeps")

[Yaron, 2016-05-04 13:07:44]

thanks

https://codereview.chromium.org/1948503002/diff/20001/chrome/android/BUILD.gn
File chrome/android/BUILD.gn (right):

https://codereview.chromium.org/1948503002/diff/20001/chrome/android/BUILD.gn...
chrome/android/BUILD.gn:457: shared_library("monochrome") {
On 2016/05/04 12:33:04, Torne wrote:
> On 2016/05/03 15:19:41, Yaron wrote:
> > err, I also omitted the "lib" prefix in these versions which I thought is
> > redundant in GN
> 
> It's also redundant in gyp ;)

oh. I vaguely recall it not being redundant at one point back in the android
backend generator days but perhaps you fixed that? :)

https://codereview.chromium.org/1948503002/diff/20001/chrome/browser/android/...
File chrome/browser/android/DEPS (right):

https://codereview.chromium.org/1948503002/diff/20001/chrome/browser/android/...
chrome/browser/android/DEPS:17: ".*monochrome_entry_point\.cc": [
On 2016/05/04 12:33:04, Torne wrote:
> Why is this .* ? Not familiar with this rule syntax so not sure what it's
> matching against, but it looks weird.

you're right that it's unnecessary. removed

https://codereview.chromium.org/1948503002/diff/20001/chrome/browser/android/...
File chrome/browser/android/monochrome_entry_point.cc (right):

https://codereview.chromium.org/1948503002/diff/20001/chrome/browser/android/...
chrome/browser/android/monochrome_entry_point.cc:21: }  // namespace
On 2016/05/03 20:54:01, michaelbai wrote:
> On 2016/05/03 20:14:48, Yaron wrote:
> > On 2016/05/03 19:30:17, michaelbai wrote:
> > > We don't need above two methods, right?, there is no warning?
> > 
> > the chrome case does need them - note the callbacks passed.
> 
> I see, you changed it to call the chrome one, but If you use way in clank, you
> basically save one copy of these methods, but it is trivial.

Ya, understood but I removed the clank ones. We could do a follow-up CL to
remove the callbacks from the chrome::android ones.

[commit-bot: I haz the power, 2016-05-04 13:07:46]

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

[commit-bot: I haz the power, 2016-05-04 14:10:58]

Description was changed from

==========
Upstream targets for building native libraries for chrome/monochrome.

These no longer contain private sources so can be build upstream.
Lots of clean-up and unification remains but this is the first
step to building these targets upstream.
BUG=607207
==========

to

==========
Upstream targets for building native libraries for chrome/monochrome.

These no longer contain private sources so can be build upstream.
Lots of clean-up and unification remains but this is the first
step to building these targets upstream.
BUG=607207
==========

[commit-bot: I haz the power, 2016-05-04 14:10:59]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-04 14:12:01]

Description was changed from

==========
Upstream targets for building native libraries for chrome/monochrome.

These no longer contain private sources so can be build upstream.
Lots of clean-up and unification remains but this is the first
step to building these targets upstream.
BUG=607207
==========

to

==========
Upstream targets for building native libraries for chrome/monochrome.

These no longer contain private sources so can be build upstream.
Lots of clean-up and unification remains but this is the first
step to building these targets upstream.
BUG=607207

Committed: https://crrev.com/561f24772cc36822699e90e7ee3842f7c301b0cb
Cr-Commit-Position: refs/heads/master@{#391501}
==========

[commit-bot: I haz the power, 2016-05-04 14:12:02]

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