New issue
Advanced search Search tips

Issue 864878 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 19
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Unable to use enable_resource_whitelist_generation in local build

Project Member Reported by ntfschr@chromium.org, Jul 18

Issue description

I'm writing a change which requires modifying Chrome/WebView resources. In the past, I've been told to add `enable_resource_whitelist_generation = true` to my args.gn, to use the updated resource whitelist for WebView.

When I try to flip this to true, I get the following output:

$ gn gen out/Default
ERROR at //tools/grit/grit_rule.gni:95:1: Assertion failed.
assert(!enable_resource_whitelist_generation || is_android,
^-----
resource whitelist generation only works on android
See //net/BUILD.gn:16:1: whence it was imported.
import("//tools/grit/grit_rule.gni")
^----------------------------------
See //third_party/blink/renderer/platform/BUILD.gn:191:5: which caused the file to be included.
    "//net",
    ^------

---

Minimal GN args:

target_os = "android"
target_cpu = "arm64"
# is_official_build = true # it repros with or without this
enable_resource_whitelist_generation = true

---

Root cause is that is_android is sometimes false (despite target_os = android). I added some print() statements in grit_rule.gni and found this file is being run 3 times, and is_android == false only on the last time the file is executed (for this experiment, enable_resource_whitelist_generation = false).

To add to the mystery: this arg used to work for me (I think I last used it ~1 year ago), but doesn't seem to work anymore.

CC'ing some folks who may know why I'm hitting this assertion (or, who could recommend a better workflow for locally building resources into WebView).
 
You can't set it to true, because that enables it while doing the host build as well. You might be able to set it to "is_android"? I think it's only an argument so you can set it to false.

I don't know why you would need to set it, though - my parsing of what this does is that without turning it on (by is_official_build), the resources just aren't filtered, but maybe I'm mistaken?

We probably shouldn't be evaluating anything that uses this during the host build anyway though?
This bit someone else recently as well:
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/wSZf-EvPEFM

I'll switch it to not be a GN arg. It was only that way because a long time ago you did need to enable it manually for it to work.
Owner: agrieve@chromium.org
Status: Assigned (was: Untriaged)
Passing to agrieve@ as he started http://crrev/c/1142193 already.
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/90205fffe287a2bb9c4dbb288b8d8872ff2cee22

commit 90205fffe287a2bb9c4dbb288b8d8872ff2cee22
Author: Andrew Grieve <agrieve@chromium.org>
Date: Wed Sep 05 01:36:28 2018

Android: Make enable_resource_whitelist_generation GN arg work when setting to true

Also:
 * Default the arg to true for all release builds that contain debug info.
   (most bots set strip_debug_info=true though)
 * Add better failure messages for when no debug info is present.

Bug:  864878 
Change-Id: Ied849d6e24a672b896c802be6df6c0958ecabfe0
Reviewed-on: https://chromium-review.googlesource.com/1142193
Commit-Queue: agrieve <agrieve@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588734}
[modify] https://crrev.com/90205fffe287a2bb9c4dbb288b8d8872ff2cee22/build/config/android/config.gni
[modify] https://crrev.com/90205fffe287a2bb9c4dbb288b8d8872ff2cee22/build/config/compiler/BUILD.gn
[modify] https://crrev.com/90205fffe287a2bb9c4dbb288b8d8872ff2cee22/build/config/compiler/compiler.gni
[modify] https://crrev.com/90205fffe287a2bb9c4dbb288b8d8872ff2cee22/build/toolchain/android/BUILD.gn
[modify] https://crrev.com/90205fffe287a2bb9c4dbb288b8d8872ff2cee22/build/toolchain/gcc_toolchain.gni
[modify] https://crrev.com/90205fffe287a2bb9c4dbb288b8d8872ff2cee22/tools/grit/grit/format/data_pack.py
[modify] https://crrev.com/90205fffe287a2bb9c4dbb288b8d8872ff2cee22/tools/grit/grit_rule.gni
[modify] https://crrev.com/90205fffe287a2bb9c4dbb288b8d8872ff2cee22/tools/resources/generate_resource_whitelist.py

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3a9bbf84099f2b522a599a8456a134bbd104c968

commit 3a9bbf84099f2b522a599a8456a134bbd104c968
Author: Bernhard Bauer <bauerb@chromium.org>
Date: Wed Sep 05 13:12:51 2018

Revert "Android: Make enable_resource_whitelist_generation GN arg work when setting to true"

This reverts commit 90205fffe287a2bb9c4dbb288b8d8872ff2cee22.

Reason for revert: Breaks cronet build; see e.g. https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8936264155658998032/+/steps/generate_build_files/0/stdout

Original change's description:
> Android: Make enable_resource_whitelist_generation GN arg work when setting to true
> 
> Also:
>  * Default the arg to true for all release builds that contain debug info.
>    (most bots set strip_debug_info=true though)
>  * Add better failure messages for when no debug info is present.
> 
> Bug:  864878 
> Change-Id: Ied849d6e24a672b896c802be6df6c0958ecabfe0
> Reviewed-on: https://chromium-review.googlesource.com/1142193
> Commit-Queue: agrieve <agrieve@chromium.org>
> Reviewed-by: Richard Coles <torne@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#588734}

TBR=torne@chromium.org,pcc@chromium.org,agrieve@chromium.org,lizeb@chromium.org

Change-Id: Ia7ca1c880cb1f2d5b8eda7f458cb0a3b2b8133ba
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  864878 
Reviewed-on: https://chromium-review.googlesource.com/1206334
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Bernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588844}
[modify] https://crrev.com/3a9bbf84099f2b522a599a8456a134bbd104c968/build/config/android/config.gni
[modify] https://crrev.com/3a9bbf84099f2b522a599a8456a134bbd104c968/build/config/compiler/BUILD.gn
[modify] https://crrev.com/3a9bbf84099f2b522a599a8456a134bbd104c968/build/config/compiler/compiler.gni
[modify] https://crrev.com/3a9bbf84099f2b522a599a8456a134bbd104c968/build/toolchain/android/BUILD.gn
[modify] https://crrev.com/3a9bbf84099f2b522a599a8456a134bbd104c968/build/toolchain/gcc_toolchain.gni
[modify] https://crrev.com/3a9bbf84099f2b522a599a8456a134bbd104c968/tools/grit/grit/format/data_pack.py
[modify] https://crrev.com/3a9bbf84099f2b522a599a8456a134bbd104c968/tools/grit/grit_rule.gni
[modify] https://crrev.com/3a9bbf84099f2b522a599a8456a134bbd104c968/tools/resources/generate_resource_whitelist.py

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 13

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cc6ba940be2d6fa4074e085978f58d513ccaf2d8

commit cc6ba940be2d6fa4074e085978f58d513ccaf2d8
Author: Andrew Grieve <agrieve@chromium.org>
Date: Thu Sep 13 16:21:23 2018

Reland: Android: Make enable_resource_whitelist_generation GN arg work when setting to true

Also:
 * Default the arg to true for all release builds that contain debug info.
   (most bots set strip_debug_info=true though)
 * Add better failure messages for when no debug info is present.

This reverts commit 3a9bbf84099f2b522a599a8456a134bbd104c968.

Reason for reland: Has fixes for:
 * !is_debug && is_component_build
 * cronet bots (which set is_official=true and strip debug info)

TBR=agrieve

Bug:  864878 
Change-Id: I99966ee4e12a0277951616159c844d4d94acaf1f
Reviewed-on: https://chromium-review.googlesource.com/1212347
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591036}
[modify] https://crrev.com/cc6ba940be2d6fa4074e085978f58d513ccaf2d8/build/config/android/config.gni
[modify] https://crrev.com/cc6ba940be2d6fa4074e085978f58d513ccaf2d8/build/config/compiler/BUILD.gn
[modify] https://crrev.com/cc6ba940be2d6fa4074e085978f58d513ccaf2d8/build/config/compiler/compiler.gni
[modify] https://crrev.com/cc6ba940be2d6fa4074e085978f58d513ccaf2d8/build/toolchain/android/BUILD.gn
[modify] https://crrev.com/cc6ba940be2d6fa4074e085978f58d513ccaf2d8/build/toolchain/gcc_toolchain.gni
[modify] https://crrev.com/cc6ba940be2d6fa4074e085978f58d513ccaf2d8/tools/grit/grit/format/data_pack.py
[modify] https://crrev.com/cc6ba940be2d6fa4074e085978f58d513ccaf2d8/tools/grit/grit_rule.gni
[modify] https://crrev.com/cc6ba940be2d6fa4074e085978f58d513ccaf2d8/tools/mb/mb_config.pyl
[modify] https://crrev.com/cc6ba940be2d6fa4074e085978f58d513ccaf2d8/tools/resources/generate_resource_whitelist.py

Status: Fixed (was: Assigned)

Sign in to add a comment