Use jinja2 to template webview manifest files.

Use jinja2 to template webview manifest files.

BUG=

Committed: https://crrev.com/846177a22c453c33f867d9c4a9f14c80f7f30372
Cr-Commit-Position: refs/heads/master@{#322791}

[Tobias Sargeant, 2015-03-30 12:41:57]

tobiasjs@chromium.org changed reviewers:
+ torne@chromium.org

[Torne, 2015-03-30 13:31:09]

https://codereview.chromium.org/1048463003/diff/40001/android_webview/android...
File android_webview/android_webview.gyp (right):

https://codereview.chromium.org/1048463003/diff/40001/android_webview/android...
android_webview/android_webview.gyp:391: 'jinja_inputs':
['apk/java/AndroidManifest.xml'],
Thinking about it more, all this logic should go in
system_webview_apk_common.gypi really, since the only thing that is different in
different cases is jinja_variables.

[Tobias Sargeant, 2015-03-30 13:53:07]

https://codereview.chromium.org/1048463003/diff/40001/android_webview/android...
File android_webview/android_webview.gyp (right):

https://codereview.chromium.org/1048463003/diff/40001/android_webview/android...
android_webview/android_webview.gyp:391: 'jinja_inputs':
['apk/java/AndroidManifest.xml'],
On 2015/03/30 13:31:09, Torne wrote:
> Thinking about it more, all this logic should go in
> system_webview_apk_common.gypi really, since the only thing that is different
in
> different cases is jinja_variables.

Done.

[Torne, 2015-03-30 14:02:38]

https://codereview.chromium.org/1048463003/diff/60001/android_webview/android...
File android_webview/android_webview.gyp (right):

https://codereview.chromium.org/1048463003/diff/60001/android_webview/android...
android_webview/android_webview.gyp:391: 'android_manifest_template_path':
'apk/java/AndroidManifest.xml',
This will be the same for all the users of system_webview_apk_common as well, we
should put that in the gypi too.

https://codereview.chromium.org/1048463003/diff/60001/android_webview/apk/sys...
File android_webview/apk/system_webview_apk_common.gypi (right):

https://codereview.chromium.org/1048463003/diff/60001/android_webview/apk/sys...
android_webview/apk/system_webview_apk_common.gypi:26: # TODO: crbug.com/405035
Find a better solution for WebView .pak files.
This TODO applies to the additional_input_paths section below, you should insert
your extra lines somewhere else :)

[Tobias Sargeant, 2015-03-30 15:54:15]

https://codereview.chromium.org/1048463003/diff/60001/android_webview/android...
File android_webview/android_webview.gyp (right):

https://codereview.chromium.org/1048463003/diff/60001/android_webview/android...
android_webview/android_webview.gyp:391: 'android_manifest_template_path':
'apk/java/AndroidManifest.xml',
On 2015/03/30 14:02:38, Torne wrote:
> This will be the same for all the users of system_webview_apk_common as well,
we
> should put that in the gypi too.

Done.

https://codereview.chromium.org/1048463003/diff/60001/android_webview/apk/sys...
File android_webview/apk/system_webview_apk_common.gypi (right):

https://codereview.chromium.org/1048463003/diff/60001/android_webview/apk/sys...
android_webview/apk/system_webview_apk_common.gypi:26: # TODO: crbug.com/405035
Find a better solution for WebView .pak files.
On 2015/03/30 14:02:38, Torne wrote:
> This TODO applies to the additional_input_paths section below, you should
insert
> your extra lines somewhere else :)

Done.

[Torne, 2015-03-30 15:56:15]

lgtm

[Tobias Sargeant, 2015-03-30 15:57:27]

The CQ bit was checked by tobiasjs@chromium.org

[commit-bot: I haz the power, 2015-03-30 15:57:54]

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

[commit-bot: I haz the power, 2015-03-30 16:33:53]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-03-30 16:34:46]

Patchset 5 (id:??) landed as
https://crrev.com/846177a22c453c33f867d9c4a9f14c80f7f30372
Cr-Commit-Position: refs/heads/master@{#322791}

[hush (inactive), 2015-03-30 23:47:19]

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

[hush (inactive), 2015-03-30 23:47:19]

https://codereview.chromium.org/1048463003/diff/80001/android_webview/apk/sys...
File android_webview/apk/system_webview_apk_common.gypi (right):

https://codereview.chromium.org/1048463003/diff/80001/android_webview/apk/sys...
android_webview/apk/system_webview_apk_common.gypi:27:
'android_manifest_template_path':
'<(DEPTH)/android_webview/apk/java/AndroidManifest.xml',
I guess this probably overrides the AndroidManifest.xml defined by
system_webview_google_apk/system_webview_google_next_apk target. As a result, I
have seen these targets produce apks whose package names are com.android.webview
instead of com.google.android.webview.

Can you fix this?

[boliu, 2015-03-31 00:30:10]

On 2015/03/30 23:47:19, hush wrote:
>
https://codereview.chromium.org/1048463003/diff/80001/android_webview/apk/sys...
> File android_webview/apk/system_webview_apk_common.gypi (right):
> 
>
https://codereview.chromium.org/1048463003/diff/80001/android_webview/apk/sys...
> android_webview/apk/system_webview_apk_common.gypi:27:
> 'android_manifest_template_path':
> '<(DEPTH)/android_webview/apk/java/AndroidManifest.xml',
> I guess this probably overrides the AndroidManifest.xml defined by
> system_webview_google_apk/system_webview_google_next_apk target. As a result,
I
> have seen these targets produce apks whose package names are
com.android.webview
> instead of com.google.android.webview.
> 
> Can you fix this?

Fix being reviewed in https://chrome-internal-review.googlesource.com/211028

[Tobias Sargeant, 2015-03-31 06:51:23]

The corresponding clank change (identical to Bo's, ultimately) was already
uploaded - https://chrome-internal-review.googlesource.com/210881/ - but I
was waiting for the clank repository to autoroll.


On Tue, Mar 31, 2015 at 1:30 AM, <boliu@chromium.org> wrote:

> On 2015/03/30 23:47:19, hush wrote:
>
> https://codereview.chromium.org/1048463003/diff/80001/
> android_webview/apk/system_webview_apk_common.gypi
>
>> File android_webview/apk/system_webview_apk_common.gypi (right):
>>
>
>
> https://codereview.chromium.org/1048463003/diff/80001/
> android_webview/apk/system_webview_apk_common.gypi#newcode27
>
>> android_webview/apk/system_webview_apk_common.gypi:27:
>> 'android_manifest_template_path':
>> '<(DEPTH)/android_webview/apk/java/AndroidManifest.xml',
>> I guess this probably overrides the AndroidManifest.xml defined by
>> system_webview_google_apk/system_webview_google_next_apk target. As a
>> result,
>>
> I
>
>> have seen these targets produce apks whose package names are
>>
> com.android.webview
>
>> instead of com.google.android.webview.
>>
>
>  Can you fix this?
>>
>
> Fix being reviewed in https://chrome-internal-
> review.googlesource.com/211028
>
> https://codereview.chromium.org/1048463003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.