Android: rename android_build_type gyp variable.

Android: rename android_build_type gyp variable.

Rename the "android_build_type" gyp variable to "android_webview_build"
to better describe what it does. There are still references in WebKit
and V8 that need to be changed before the original variable can be
removed.

BUG=184431
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=187556

[benm (inactive), 2013-03-11 17:23:29]

thanks!

https://codereview.chromium.org/12730006/diff/1/android_webview/android_webvi...
File android_webview/android_webview.gyp (right):

https://codereview.chromium.org/12730006/diff/1/android_webview/android_webvi...
android_webview/android_webview.gyp:27: ['android_webview_build != 0', {
while we're here, would a_w_b == 1 be easier to parse? (i.e get rid of double
negative)

https://codereview.chromium.org/12730006/diff/1/build/android/cpufeatures.gypi
File build/android/cpufeatures.gypi (right):

https://codereview.chromium.org/12730006/diff/1/build/android/cpufeatures.gyp...
build/android/cpufeatures.gypi:10: ['android_webview_build != 0', {
ditto, awb == 1?

https://codereview.chromium.org/12730006/diff/1/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/12730006/diff/1/build/common.gypi#newcode998
build/common.gypi:998: 'android_build_type%': 0
should we set it to android_webview_build ?

https://codereview.chromium.org/12730006/diff/1/content/content.gyp
File content/content.gyp (right):

https://codereview.chromium.org/12730006/diff/1/content/content.gyp#newcode33
content/content.gyp:33: 
nit: remove \n?

[joth, 2013-03-11 17:25:37]

In prior conversations we've highlighted this controls *how* it's built,
not *what* it builds, so in that line would android_[system|tree]_build be
more appropriate?
OTOH android_webview_build is much more readily connected with the
android_webview which in practice is what most readers will want to know.
So maybe just a comment explaining this controls how not what is built
would do.






On 11 March 2013 10:17, <torne@chromium.org> wrote:

> Reviewers: benm,
>
> Description:
> Android: rename android_build_type gyp variable.
>
> Rename the "android_build_type" gyp variable to "android_webview_build"
> to better describe what it does. There are still references in WebKit
> and V8 that need to be changed before the original variable can be
> removed.
>
> BUG=
>
>
> Please review this at
https://codereview.chromium.**org/12730006/<https://codereview.chromium.org/1...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
>
> Affected files:
>   M android_webview/android_**webview.gyp
>   M base/base.gyp
>   M build/android/cpufeatures.gypi
>   M build/android/envsetup_**functions.sh
>   M build/common.gypi
>   M content/content.gyp
>   M content/content_shell.gypi
>   M net/net.gyp
>   M skia/skia.gyp
>   M third_party/eyesfree/eyesfree.**gyp
>   M ui/shell_dialogs.gypi
>   M ui/ui.gyp
>
>
> Index: android_webview/android_**webview.gyp
> diff --git a/android_webview/android_**webview.gyp
> b/android_webview/android_**webview.gyp
> index 87249f9d8f6f6c9c2a25b91cfd114d**e1be80039b..**
> 88fbb672020b3f41db74b347232511**921b5f1ee6 100644
> --- a/android_webview/android_**webview.gyp
> +++ b/android_webview/android_**webview.gyp
> @@ -24,7 +24,7 @@
>          'lib/main/webview_entry_point.**cc',
>        ],
>        'conditions': [
> -        ['android_build_type != 0', {
> +        ['android_webview_build != 0', {
>            'libraries': [
>              # The "android" gyp backend doesn't quite handle static
> libraries'
>              # dependencies correctly; force this to be linked as a
> workaround.
> Index: base/base.gyp
> diff --git a/base/base.gyp b/base/base.gyp
> index 0b08c908d893b9ebeb5b3b6b1d3c36**f1f91652f0..**
> 9123db4ce803c554fd0f1319dff231**0e608ca74e 100644
> --- a/base/base.gyp
> +++ b/base/base.gyp
> @@ -151,7 +151,7 @@
>              '../build/android/cpufeatures.**gypi',
>            ],
>          }],
> -        ['OS == "android" and _toolset == "target" and android_build_type
> == 0', {
> +        ['OS == "android" and _toolset == "target" and
> android_webview_build == 0', {
>            'dependencies': [
>              'base_java',
>            ],
> @@ -1131,7 +1131,7 @@
>            },
>            'includes': [ '../build/java.gypi' ],
>            'conditions': [
> -            ['android_build_type==0', {
> +            ['android_webview_build==0', {
>                'dependencies': [
>                  '../third_party/jsr-305/jsr-**305.gyp:jsr_305_javalib',
>                ],
> Index: build/android/cpufeatures.gypi
> diff --git a/build/android/cpufeatures.**gypi b/build/android/cpufeatures.
> **gypi
> index 17b262c977cc9bb8dad59c403600ee**ae8db0304b..**
> 49b5accf8b4469178285385af2f817**c32aaebc96 100644
> --- a/build/android/cpufeatures.**gypi
> +++ b/build/android/cpufeatures.**gypi
> @@ -7,7 +7,7 @@
>  # gyp file in the checked-in NDK to build it.
>  {
>    'conditions': [
> -    ['android_build_type != 0', {
> +    ['android_webview_build != 0', {
>        'libraries': [
>          'cpufeatures.a'
>        ],
> Index: build/android/envsetup_**functions.sh
> diff --git a/build/android/envsetup_**functions.sh
> b/build/android/envsetup_**functions.sh
> index 5ad34b7b8e6d81fdc99bb53fd8615a**7296fd3fec..**
> 511e8d233e0a08ce6873cc12df6ead**85b9e1de5a 100755
> --- a/build/android/envsetup_**functions.sh
> +++ b/build/android/envsetup_**functions.sh
> @@ -284,6 +284,8 @@ ${ANDROID_SDK_VERSION}
>            '${ANDROID_BUILD_TOP}')")
>        ;;
>    esac
> +  DEFINES+=" android_webview_build=1"
> +  # temporary until all uses of android_build_type are gone
>    DEFINES+=" android_build_type=1"
>    DEFINES+=" android_src=\$(GYP_ABS_**ANDROID_TOP_DIR)"
>    DEFINES+=" android_sdk=\$(GYP_ABS_**ANDROID_TOP_DIR)/${ANDROID_**SDK}"
> Index: build/common.gypi
> diff --git a/build/common.gypi b/build/common.gypi
> index c861e63bf78376390a2d36958a49a8**bcb2c99de1..**
> 54e237e2b072b0ca8af6ab0dab77d0**a2b8a7bc2d 100644
> --- a/build/common.gypi
> +++ b/build/common.gypi
> @@ -102,10 +102,8 @@
>          # building on.
>          'target_arch%': '<(host_arch)',
>
> -        # Sets whether we're building with the Android SDK/NDK (and hence
> with
> -        # Ant, value 0), or as part of the Android system (and hence with
> the
> -        # Android build system, value 1).
> -        'android_build_type%': 0,
> +        # Set when building the Android WebView inside the Android build
> system.
> +        'android_webview_build%': 0,
>
>          # Sets whether chrome is built for google tv device.
>          'google_tv%': 0,
> @@ -169,7 +167,7 @@
>        'enable_viewport%': '<(enable_viewport)',
>        'enable_hidpi%': '<(enable_hidpi)',
>        'enable_touch_ui%': '<(enable_touch_ui)',
> -      'android_build_type%': '<(android_build_type)',
> +      'android_webview_build%': '<(android_webview_build)',
>        'google_tv%': '<(google_tv)',
>        'enable_app_list%': '<(enable_app_list)',
>        'enable_message_center%': '<(enable_message_center)',
> @@ -488,18 +486,18 @@
>          }],
>
>          # Enable autofill dialog for Android and Views-enabled platforms
> for now.
> -        ['toolkit_views==1 or (OS=="android" and android_build_type==0)',
> {
> +        ['toolkit_views==1 or (OS=="android" and
> android_webview_build==0)', {
>            'enable_autofill_dialog%': 1
>          }],
>
> -        ['OS=="android" and android_build_type==0', {
> +        ['OS=="android" and android_webview_build==0', {
>            'enable_webrtc%': 1,
>          }],
>
>          # Disable WebRTC for building WebView as part of Android system.
>          # TODO(boliu): Decide if we want WebRTC, and if so, also merge
>          # the necessary third_party repositories.
> -        ['OS=="android" and android_build_type==1', {
> +        ['OS=="android" and android_webview_build==1', {
>            'enable_webrtc%': 0,
>          }],
>
> @@ -593,7 +591,7 @@
>          ['OS=="android"', {
>            # When building as part of the Android system, use system
> libraries
>            # where possible to reduce ROM size.
> -          'use_system_libjpeg%': '<(android_build_type)',
> +          'use_system_libjpeg%': '<(android_webview_build)',
>          }],
>
>          # Enable Settings App only on Windows.
> @@ -733,7 +731,7 @@
>      'wix_path%': '<(wix_path)',
>      'use_libjpeg_turbo%': '<(use_libjpeg_turbo)',
>      'use_system_libjpeg%': '<(use_system_libjpeg)',
> -    'android_build_type%': '<(android_build_type)',
> +    'android_webview_build%': '<(android_webview_build)',
>      'google_tv%': '<(google_tv)',
>      'enable_app_list%': '<(enable_app_list)',
>      'enable_message_center%': '<(enable_message_center)',
> @@ -996,6 +994,9 @@
>      'android_app_version_name%': 'Developer Build',
>      'android_app_version_code%': 0,
>
> +    # Temporarily set android_build_type until all uses of it have been
> renamed.
> +    'android_build_type%': 0
> +
>      'sas_dll_exists': '<!(python <(DEPTH)/build/dir_exists.py
> <(sas_dll_path))',
>      'wix_exists': '<!(python <(DEPTH)/build/dir_exists.py <(wix_path))',
>
> @@ -1211,15 +1212,15 @@
>          # When building as part of the Android system, use system
> libraries
>          # where possible to reduce ROM size.
>          # TODO(steveblock): Investigate using the system version of
> sqlite.
> -        'use_system_sqlite%': 0,  # '<(android_build_type)',
> -        'use_system_expat%': '<(android_build_type)',
> -        'use_system_icu%': '<(android_build_type)',
> -        'use_system_stlport%': '<(android_build_type)',
> +        'use_system_sqlite%': 0,  # '<(android_webview_build)',
> +        'use_system_expat%': '<(android_webview_build)',
> +        'use_system_icu%': '<(android_webview_build)',
> +        'use_system_stlport%': '<(android_webview_build)',
>
>          'enable_managed_users%': 0,
>
>          # Copy it out one scope.
> -        'android_build_type%': '<(android_build_type)',
> +        'android_webview_build%': '<(android_webview_build)',
>        }],  # OS=="android"
>        ['OS=="mac"', {
>          'variables': {
> @@ -3208,7 +3209,7 @@
>                    '-mllvm -asan-globals=0',
>                  ],
>                }],
> -              ['android_build_type==0', {
> +              ['android_webview_build==0', {
>                  'defines': [
>                    # The NDK has these things, but doesn't define the
> constants
>                    # to say that it does. Define them here instead.
> @@ -3221,7 +3222,7 @@
>                    '--sysroot=<(android_ndk_**sysroot)',
>                  ],
>                }],
> -              ['android_build_type==1', {
> +              ['android_webview_build==1', {
>                  'include_dirs': [
>                    # OpenAL headers from the Android tree.
>                    '<(android_src)/frameworks/**wilhelm/include',
> @@ -3250,7 +3251,7 @@
>                    '-Wno-sign-promo',
>                  ],
>                }],
> -              ['android_build_type==1 and chromium_code==0', {
> +              ['android_webview_build==1 and chromium_code==0', {
>                  'cflags': [
>                    # There is a class of warning which:
>                    #  1) Android always enables and also treats as errors
> Index: content/content.gyp
> diff --git a/content/content.gyp b/content/content.gyp
> index d01e7d7132dd65dc53c07362bbeac0**6eb496bef1..**
> f0e280408a814b31bc7508822b9ee0**b1f996b808 100644
> --- a/content/content.gyp
> +++ b/content/content.gyp
> @@ -30,7 +30,7 @@
>            },
>          },
>        }],
> -
> +
>      ],
>    },
>    'conditions': [
> @@ -322,7 +322,7 @@
>              'java_strings_grd': 'android_content_strings.grd',
>            },
>            'conditions': [
> -            ['android_build_type == 0', {
> +            ['android_webview_build == 0', {
>                'dependencies': [
>                  '../third_party/eyesfree/**eyesfree.gyp:eyesfree_java',
>                  '../third_party/guava/guava.**gyp:guava_javalib',
> Index: content/content_shell.gypi
> diff --git a/content/content_shell.gypi b/content/content_shell.gypi
> index 9042c5c5c12283610b8beb8002c25d**987fd45620..**
> 28404e33243008e3d6b4e124dff4cc**ac243735ef 100644
> --- a/content/content_shell.gypi
> +++ b/content/content_shell.gypi
> @@ -608,7 +608,7 @@
>              'shell/android/shell_library_**loader.h',
>            ],
>            'conditions': [
> -            ['android_build_type==1', {
> +            ['android_webview_build==1', {
>                'ldflags': [
>                  '-lgabi++',  # For rtti
>                ],
> Index: net/net.gyp
> diff --git a/net/net.gyp b/net/net.gyp
> index 98b9496ebcea9ad7404b459bfe85bf**45136e3730..**
> 4716497479a2693299f2410038a9ce**c313ebb16a 100644
> --- a/net/net.gyp
> +++ b/net/net.gyp
> @@ -1269,7 +1269,7 @@
>              },
>            },
>          ],
> -        ['OS=="android" and _toolset=="target" and android_build_type ==
> 0', {
> +        ['OS=="android" and _toolset=="target" and android_webview_build
> == 0', {
>            'dependencies': [
>               'net_java',
>            ],
> Index: skia/skia.gyp
> diff --git a/skia/skia.gyp b/skia/skia.gyp
> index 340e64e0f81121e1e932b362d9912a**5af58567b4..**
> 66531c6818e6a6dc1b9a926fb3ea62**6b71ada7dd 100644
> --- a/skia/skia.gyp
> +++ b/skia/skia.gyp
> @@ -434,7 +434,7 @@
>                  'ext/vector_platform_device_**skia.cc',
>                ],
>              }],
> -            [ '_toolset == "target" and android_build_type == 0', {
> +            [ '_toolset == "target" and android_webview_build == 0', {
>                'defines': [
>                  'HAVE_ENDIAN_H',
>                ],
> @@ -597,7 +597,7 @@
>                'SK_BUILD_FOR_ANDROID_NDK',
>              ],
>              'conditions': [
> -              [ '_toolset == "target" and android_build_type == 0', {
> +              [ '_toolset == "target" and android_webview_build == 0', {
>                  'defines': [
>                    'HAVE_ENDIAN_H',
>                  ],
> Index: third_party/eyesfree/eyesfree.**gyp
> diff --git a/third_party/eyesfree/**eyesfree.gyp b/third_party/eyesfree/**
> eyesfree.gyp
> index 6a8501e35a3ad71b3ef0c2aa41a254**229de668dc..**
> f403ec3c21c0195289e2bbdc9aec12**a8f1631ff8 100644
> --- a/third_party/eyesfree/**eyesfree.gyp
> +++ b/third_party/eyesfree/**eyesfree.gyp
> @@ -3,7 +3,7 @@
>  # found in the LICENSE file.
>  {
>    'conditions': [
> -    ['OS == "android" and android_build_type == 0', {
> +    ['OS == "android" and android_webview_build == 0', {
>        'targets': [
>          {
>            'target_name': 'eyesfree_java',
> Index: ui/shell_dialogs.gypi
> diff --git a/ui/shell_dialogs.gypi b/ui/shell_dialogs.gypi
> index 4f474c5a1c491661730690d9a64543**66506508ae..**
> 2675d73ab8ea0be95e188f47c094ab**c90ef3d831 100644
> --- a/ui/shell_dialogs.gypi
> +++ b/ui/shell_dialogs.gypi
> @@ -70,7 +70,7 @@
>              ],
>            },
>          }],
> -        ['OS=="android" and android_build_type==0', {
> +        ['OS=="android" and android_webview_build==0', {
>            'dependencies': [
>              'ui_java',
>            ],
> Index: ui/ui.gyp
> diff --git a/ui/ui.gyp b/ui/ui.gyp
> index a8489c491a79851feacc64c3385e13**bb7d9436fb..**
> c92781b7937d80a843b6fa8dce726e**118dd5a1bd 100644
> --- a/ui/ui.gyp
> +++ b/ui/ui.gyp
> @@ -819,7 +819,7 @@
>              ],
>            },
>          }],
> -        ['OS=="android" and android_build_type==0', {
> +        ['OS=="android" and android_webview_build==0', {
>            'dependencies': [
>              'ui_java',
>            ],
>
>
>

[Torne, 2013-03-11 17:31:11]

I've expanded the comment on android_webview_build to try and explain this
better; using a different name might be more 'correct' but isn't very
informative to most people as you note.

https://codereview.chromium.org/12730006/diff/1/android_webview/android_webvi...
File android_webview/android_webview.gyp (right):

https://codereview.chromium.org/12730006/diff/1/android_webview/android_webvi...
android_webview/android_webview.gyp:27: ['android_webview_build != 0', {
On 2013/03/11 17:23:29, benm wrote:
> while we're here, would a_w_b == 1 be easier to parse? (i.e get rid of double
> negative)

Done.

https://codereview.chromium.org/12730006/diff/1/build/android/cpufeatures.gypi
File build/android/cpufeatures.gypi (right):

https://codereview.chromium.org/12730006/diff/1/build/android/cpufeatures.gyp...
build/android/cpufeatures.gypi:10: ['android_webview_build != 0', {
On 2013/03/11 17:23:29, benm wrote:
> ditto, awb == 1?

Done.

https://codereview.chromium.org/12730006/diff/1/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/12730006/diff/1/build/common.gypi#newcode998
build/common.gypi:998: 'android_build_type%': 0
On 2013/03/11 17:23:29, benm wrote:
> should we set it to android_webview_build ?

Done.

https://codereview.chromium.org/12730006/diff/1/content/content.gyp
File content/content.gyp (right):

https://codereview.chromium.org/12730006/diff/1/content/content.gyp#newcode33
content/content.gyp:33: 
On 2013/03/11 17:23:29, benm wrote:
> nit: remove \n?

Done.

[benm (inactive), 2013-03-11 17:36:05]

lgtm

[Torne, 2013-03-11 18:01:25]

Rubberstamps please? :)

brettw: base, content, third_party
thakis: skia, ui
rdsmith: net

[Nico, 2013-03-11 19:42:08]

lgtm

https://codereview.chromium.org/12730006/diff/2002/build/android/envsetup_fun...
File build/android/envsetup_functions.sh (right):

https://codereview.chromium.org/12730006/diff/2002/build/android/envsetup_fun...
build/android/envsetup_functions.sh:288: # temporary until all uses of
android_build_type are gone
Can you file a tracking bug and mention that in this comment (and on the BUG=
line of this CL), so that there's a place to track the progress of this
transition?

[brettw, 2013-03-11 21:45:45]

lgtm

[Randy Smith (Not in Mondays), 2013-03-12 05:07:10]

lgtm

[Torne, 2013-03-12 10:20:41]

https://codereview.chromium.org/12730006/diff/2002/build/android/envsetup_fun...
File build/android/envsetup_functions.sh (right):

https://codereview.chromium.org/12730006/diff/2002/build/android/envsetup_fun...
build/android/envsetup_functions.sh:288: # temporary until all uses of
android_build_type are gone
On 2013/03/11 19:42:08, Nico wrote:
> Can you file a tracking bug and mention that in this comment (and on the BUG=
> line of this CL), so that there's a place to track the progress of this
> transition?

Done.

[Torne, 2013-03-12 10:22:41]

Committed patchset #3 manually as r187556 (presubmit successful).