Cronet Dynamic Framework for iOS

Cronet Dynamic Framework for iOS

The following line will build all framework variations and copy them to out/Frameworks:
./components/cronet/tools/package_ios.py --framework out/Frameworks

BUG=611176
Committed: https://crrev.com/7f6b00dfb70a9b83876f3c1fce81980ede3000f1
Cr-Commit-Position: refs/heads/master@{#393070}

[kapishnikov, 2016-05-04 16:25:58]

kapishnikov@chromium.org changed reviewers:
+ mef@chromium.org

[kapishnikov, 2016-05-04 16:25:58]

This change allows building Cronet Dynamic Framework in Chromium.

[kapishnikov, 2016-05-06 18:54:50]

Description was changed from

==========
Cronet Dynamic Framework
==========

to

==========
Cronet Dynamic Framework

The following line will build all framework variations and copy them to
out/Frameworks:
./components/cronet/tools/package_ios.py --framework out/Frameworks
==========

[mef, 2016-05-11 12:55:03]

https://codereview.chromium.org/1947903003/diff/20001/components/cronet/ios/I...
File components/cronet/ios/Info.plist (left):

https://codereview.chromium.org/1947903003/diff/20001/components/cronet/ios/I...
components/cronet/ios/Info.plist:25: <key>NSHumanReadableCopyright</key>
Put regular Chromium copyright?

# Copyright 2014 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

https://codereview.chromium.org/1947903003/diff/20001/components/cronet/ios/I...
File components/cronet/ios/Info.plist (right):

https://codereview.chromium.org/1947903003/diff/20001/components/cronet/ios/I...
components/cronet/ios/Info.plist:2: <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST
1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
We should look into using infoplist_strings_tool, although it lives in chrome,
so we may need to move it elsewhere.
It should be fine to do in separate CL.

https://codereview.chromium.org/1947903003/diff/20001/components/cronet/ios/I...
components/cronet/ios/Info.plist:6: <string>en</string>
nit: tabs -> spaces?

https://codereview.chromium.org/1947903003/diff/20001/components/cronet/ios/e...
File components/cronet/ios/empty.cc (right):

https://codereview.chromium.org/1947903003/diff/20001/components/cronet/ios/e...
components/cronet/ios/empty.cc:5: // An empty C++ file that is needed to trigger
the usage of clang++ instead of
Do we still need it? This is weird.

https://codereview.chromium.org/1947903003/diff/20001/components/cronet/tools...
File components/cronet/tools/package_ios.py (right):

https://codereview.chromium.org/1947903003/diff/20001/components/cronet/tools...
components/cronet/tools/package_ios.py:20: def eprint(message):
only used once, inline?

https://codereview.chromium.org/1947903003/diff/20001/components/cronet/tools...
components/cronet/tools/package_ios.py:21: print >>sys.stderr, message
nit: 2 empty lines between functions. Here and elsewhere.

https://codereview.chromium.org/1947903003/diff/20001/components/cronet/tools...
components/cronet/tools/package_ios.py:28: for item in os.listdir(src):
Hrm, I still find it weird that we need a custom function to copy directory.

[kapishnikov, 2016-05-11 19:48:16]

https://codereview.chromium.org/1947903003/diff/20001/components/cronet/ios/I...
File components/cronet/ios/Info.plist (left):

https://codereview.chromium.org/1947903003/diff/20001/components/cronet/ios/I...
components/cronet/ios/Info.plist:25: <key>NSHumanReadableCopyright</key>
On 2016/05/11 12:55:02, mef wrote:
> Put regular Chromium copyright?
> 
> # Copyright 2014 The Chromium Authors. All rights reserved.
> # Use of this source code is governed by a BSD-style license that can be
> # found in the LICENSE file.

Done. Used one line copyright as other components do.

https://codereview.chromium.org/1947903003/diff/20001/components/cronet/ios/I...
File components/cronet/ios/Info.plist (right):

https://codereview.chromium.org/1947903003/diff/20001/components/cronet/ios/I...
components/cronet/ios/Info.plist:2: <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST
1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
On 2016/05/11 12:55:02, mef wrote:
> We should look into using infoplist_strings_tool, although it lives in chrome,
> so we may need to move it elsewhere.
> It should be fine to do in separate CL.

Acknowledged.

https://codereview.chromium.org/1947903003/diff/20001/components/cronet/ios/I...
components/cronet/ios/Info.plist:6: <string>en</string>
On 2016/05/11 12:55:02, mef wrote:
> nit: tabs -> spaces?

Done.

https://codereview.chromium.org/1947903003/diff/20001/components/cronet/ios/e...
File components/cronet/ios/empty.cc (right):

https://codereview.chromium.org/1947903003/diff/20001/components/cronet/ios/e...
components/cronet/ios/empty.cc:5: // An empty C++ file that is needed to trigger
the usage of clang++ instead of
On 2016/05/11 12:55:02, mef wrote:
> Do we still need it? This is weird.

That is the only way I found to make gyp use clang++. We can add a real c++ file
to 'cronet_framework'; however, if the same file compiles as a part of the
static library, there will be duplicate symbol error.

I also tried to add
            'LD': 'clang++',
            'CC': 'clang++',
flags to xcode_settings configuration, but it still uses clang.

https://codereview.chromium.org/1947903003/diff/20001/components/cronet/tools...
File components/cronet/tools/package_ios.py (right):

https://codereview.chromium.org/1947903003/diff/20001/components/cronet/tools...
components/cronet/tools/package_ios.py:20: def eprint(message):
On 2016/05/11 12:55:02, mef wrote:
> only used once, inline?

Done.

https://codereview.chromium.org/1947903003/diff/20001/components/cronet/tools...
components/cronet/tools/package_ios.py:21: print >>sys.stderr, message
On 2016/05/11 12:55:02, mef wrote:
> nit: 2 empty lines between functions. Here and elsewhere.

Done.

https://codereview.chromium.org/1947903003/diff/20001/components/cronet/tools...
components/cronet/tools/package_ios.py:28: for item in os.listdir(src):
On 2016/05/11 12:55:02, mef wrote:
> Hrm, I still find it weird that we need a custom function to copy directory.

Done.

[mef, 2016-05-11 20:17:19]

lgtm

https://codereview.chromium.org/1947903003/diff/20001/components/cronet/ios/e...
File components/cronet/ios/empty.cc (right):

https://codereview.chromium.org/1947903003/diff/20001/components/cronet/ios/e...
components/cronet/ios/empty.cc:5: // An empty C++ file that is needed to trigger
the usage of clang++ instead of
On 2016/05/11 19:48:15, kapishnikov wrote:
> On 2016/05/11 12:55:02, mef wrote:
> > Do we still need it? This is weird.
> 
> That is the only way I found to make gyp use clang++. We can add a real c++
file
> to 'cronet_framework'; however, if the same file compiles as a part of the
> static library, there will be duplicate symbol error.
> 
> I also tried to add
>             'LD': 'clang++',
>             'CC': 'clang++',
> flags to xcode_settings configuration, but it still uses clang.

Acknowledged. Argh! I think we can get rid of cronet_static in near future, so
this hack won't be necessary.

[mef, 2016-05-11 20:18:30]

On 2016/05/11 20:17:19, mef wrote:
> lgtm
> 
>
https://codereview.chromium.org/1947903003/diff/20001/components/cronet/ios/e...
> File components/cronet/ios/empty.cc (right):
> 
>
https://codereview.chromium.org/1947903003/diff/20001/components/cronet/ios/e...
> components/cronet/ios/empty.cc:5: // An empty C++ file that is needed to
trigger
> the usage of clang++ instead of
> On 2016/05/11 19:48:15, kapishnikov wrote:
> > On 2016/05/11 12:55:02, mef wrote:
> > > Do we still need it? This is weird.
> > 
> > That is the only way I found to make gyp use clang++. We can add a real c++
> file
> > to 'cronet_framework'; however, if the same file compiles as a part of the
> > static library, there will be duplicate symbol error.
> > 
> > I also tried to add
> >             'LD': 'clang++',
> >             'CC': 'clang++',
> > flags to xcode_settings configuration, but it still uses clang.
> 
> Acknowledged. Argh! I think we can get rid of cronet_static in near future, so
> this hack won't be necessary.

Could you elaborate in description that this is for iOS, and maybe add a BUG=

[kapishnikov, 2016-05-11 20:38:50]

Description was changed from

==========
Cronet Dynamic Framework

The following line will build all framework variations and copy them to
out/Frameworks:
./components/cronet/tools/package_ios.py --framework out/Frameworks
==========

to

==========
Cronet Dynamic Framework for iOS

The following line will build all framework variations and copy them to
out/Frameworks:
./components/cronet/tools/package_ios.py --framework out/Frameworks

BUG=611176
==========

[kapishnikov, 2016-05-11 20:40:24]

The CQ bit was checked by kapishnikov@chromium.org

[commit-bot: I haz the power, 2016-05-11 20:41:32]

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

[commit-bot: I haz the power, 2016-05-11 21:40:29]

Description was changed from

==========
Cronet Dynamic Framework for iOS

The following line will build all framework variations and copy them to
out/Frameworks:
./components/cronet/tools/package_ios.py --framework out/Frameworks

BUG=611176
==========

to

==========
Cronet Dynamic Framework for iOS

The following line will build all framework variations and copy them to
out/Frameworks:
./components/cronet/tools/package_ios.py --framework out/Frameworks

BUG=611176
==========

[commit-bot: I haz the power, 2016-05-11 21:40:31]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-11 21:42:06]

Description was changed from

==========
Cronet Dynamic Framework for iOS

The following line will build all framework variations and copy them to
out/Frameworks:
./components/cronet/tools/package_ios.py --framework out/Frameworks

BUG=611176
==========

to

==========
Cronet Dynamic Framework for iOS

The following line will build all framework variations and copy them to
out/Frameworks:
./components/cronet/tools/package_ios.py --framework out/Frameworks

BUG=611176

Committed: https://crrev.com/7f6b00dfb70a9b83876f3c1fce81980ede3000f1
Cr-Commit-Position: refs/heads/master@{#393070}
==========

[commit-bot: I haz the power, 2016-05-11 21:42:07]

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