Refactorying gule/BUILD.gn to make merging easy.

android_webview/glue/BUILD.gn

Index: android_webview/glue/BUILD.gn
diff --git a/android_webview/glue/BUILD.gn b/android_webview/glue/BUILD.gn
index 59302f7783514bd882f205b2df158cc97555046e..3434e7b2351e3aa6ced8941bd85a8f4cee15c4d3 100644
--- a/android_webview/glue/BUILD.gn
+++ b/android_webview/glue/BUILD.gn
@@ -7,12 +7,23 @@ import("//build/config/android/rules.gni")
 import("generate_resource_rewriter.gni")
 import("glue.gni")
 
-webview_framework_ijar =
-    "$root_out_dir/lib.java/webview/frameworks.interface.jar"
+webview_framework_ijar = "frameworks.interface.jar"
+webview_public_framework_ijar = "frameworks.public.interface.jar"
 
-generate_interface_jar("framework_ijar") {
-  input_jar = webview_framework_jar
-  output_jar = webview_framework_ijar
+# Mark webview_framework_ijar is used, otherwise it isn't used if there
+# is no internal code.
+assert(webview_framework_ijar != "")
+
+if (defined(webview_internal_framework_jar)) {
+  generate_interface_jar("framework_ijar") {
+    input_jar = webview_internal_framework_jar
+    output_jar = "$target_out_dir/$webview_framework_ijar"
+  }
+}
+
+generate_interface_jar("public_framework_ijar") {
+  input_jar = webview_public_framework_jar
+  output_jar = "$target_out_dir/$webview_public_framework_ijar"
 }
 
 generate_resource_rewriter("glue_resource_rewriter") {
@@ -21,35 +32,42 @@ generate_resource_rewriter("glue_resource_rewriter") {
   package_name = "com.android.webview.chromium"
 }
 
-if (!defined(use_webview_internal_framework) ||
-    !use_webview_internal_framework) {
-  android_library("glue") {
-    # Change deps? please modify glue_library_deps variable.
-    deps = glue_library_deps
-    srcjar_deps = [ ":glue_resource_rewriter" ]
-
-    alternative_android_sdk_ijar = webview_framework_ijar
-    alternative_android_sdk_ijar_dep = "//android_webview/glue:framework_ijar"
-
-    java_files = [
-      "java/src/com/android/webview/chromium/ContentSettingsAdapter.java",
-      "java/src/com/android/webview/chromium/CookieManagerAdapter.java",
-      "java/src/com/android/webview/chromium/DrawGLFunctor.java",
-      "java/src/com/android/webview/chromium/GeolocationPermissionsAdapter.java",
-      "java/src/com/android/webview/chromium/GraphicsUtils.java",
-      "java/src/com/android/webview/chromium/LicenseActivity.java",
-      "java/src/com/android/webview/chromium/LicenseContentProvider.java",
-      "java/src/com/android/webview/chromium/ResourcesContextWrapperFactory.java",
-      "java/src/com/android/webview/chromium/WebBackForwardListChromium.java",
-      "java/src/com/android/webview/chromium/WebHistoryItemChromium.java",
-      "java/src/com/android/webview/chromium/WebIconDatabaseAdapter.java",
-      "java/src/com/android/webview/chromium/WebMessagePortAdapter.java",
-      "java/src/com/android/webview/chromium/WebStorageAdapter.java",
-      "java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java",
-      "java/src/com/android/webview/chromium/WebViewChromium.java",
-      "java/src/com/android/webview/chromium/WebViewContentsClientAdapter.java",
-      "java/src/com/android/webview/chromium/WebViewDatabaseAdapter.java",
-      "java/src/com/android/webview/chromium/WebViewDelegateFactory.java",
-    ]
+android_library("glue") {
+  # Change deps? please modify glue_library_deps variable.
+  deps = glue_library_deps
+  srcjar_deps = [ ":glue_resource_rewriter" ]
+
+  _target_dir = get_label_info(target_name, "label_no_toolchain")
+  if (_target_dir == "//android_webview/glue/glue:glue") {

[agrieve, 2016/04/16 00:14:59]

Wouldn't this always evaluate to true? Are you planning on symlinking this file
downstream or something? Certainly comment-worthy.

Also - it would make more sense to check ":$target_name" ==
"//android_webview/glue:glue"

[Torne, 2016/04/18 13:22:12]

There is a duplicate copy of android_webview/glue downstream which we use to
depend on new Android APIs not yet released to the public SDK. Once each Android
release comes out we expect to be able to just push all the downstream changes
to the public chromium repo, i.e. this is just a temporary fork each release.

So, we need a way for this build file to make sense in both locations, to avoid
the downstream version being permanently forked.

I'm not sure this way is particularly great, but also not sure what else to
do...

[michaelbai, 2016/04/18 16:59:07]

We have a copy of this file in downstream, this is used to detect the target is
downstream or upstream, then make sure they are compiled with internal or public
framework jar respectively.

$target_name is short name,  it will always be 'glue' in this case.

[agrieve, 2016/04/18 17:40:56]

I just mean that it's strange to look for a target that doesn't exist.

"//android_webview/glue/glue:glue" does not exist. The target is:
"//android_webview/glue:glue"

The reason get_label_info is returning the former is because it's interpreting
"glue" as a relative directory. You should instead pass ":glue"

Maybe simpler to do one of:
is_upstream = (target_out_dir == "//out-gn/Debug/obj/android_webview/glue")
or
is_upstream = (rebase_path(".", "//android_webview/glue") == ".")

It might also make sense to put this is_upstream at the top of the file where
your newly added comment about it is.

[michaelbai, 2016/04/18 18:28:55]

Thanks, learning!

+    alternative_android_sdk_ijar_dep = ":public_framework_ijar"
+    _ijar = webview_public_framework_ijar
+  } else {
+    assert(_target_dir == "//clank/android_webview/glue/glue:glue",
+           "target_dir is " + _target_dir)
+    assert(defined(webview_internal_framework_jar))
+    alternative_android_sdk_ijar_dep = ":framework_ijar"
+    _ijar = webview_framework_ijar
   }
+  _ijar_dir = get_label_info(alternative_android_sdk_ijar_dep, "target_out_dir")
+  alternative_android_sdk_ijar = "$_ijar_dir/$_ijar"
+  java_files = [
+    "java/src/com/android/webview/chromium/ContentSettingsAdapter.java",
+    "java/src/com/android/webview/chromium/CookieManagerAdapter.java",
+    "java/src/com/android/webview/chromium/DrawGLFunctor.java",
+    "java/src/com/android/webview/chromium/GeolocationPermissionsAdapter.java",
+    "java/src/com/android/webview/chromium/GraphicsUtils.java",
+    "java/src/com/android/webview/chromium/LicenseActivity.java",
+    "java/src/com/android/webview/chromium/LicenseContentProvider.java",
+    "java/src/com/android/webview/chromium/ResourcesContextWrapperFactory.java",
+    "java/src/com/android/webview/chromium/WebBackForwardListChromium.java",
+    "java/src/com/android/webview/chromium/WebHistoryItemChromium.java",
+    "java/src/com/android/webview/chromium/WebIconDatabaseAdapter.java",
+    "java/src/com/android/webview/chromium/WebMessagePortAdapter.java",
+    "java/src/com/android/webview/chromium/WebStorageAdapter.java",
+    "java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java",
+    "java/src/com/android/webview/chromium/WebViewChromium.java",
+    "java/src/com/android/webview/chromium/WebViewContentsClientAdapter.java",
+    "java/src/com/android/webview/chromium/WebViewDatabaseAdapter.java",
+    "java/src/com/android/webview/chromium/WebViewDelegateFactory.java",
+  ]
 }