New issue
Advanced search Search tips

Issue 878730 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Some CrOS tests fail trying to load 200% version of IDR_WEBSTORE_ICON if its message ID is changed

Project Member Reported by engedy@chromium.org, Aug 29

Issue description

The original version of crrev.com/c/1193944 introduced a couple of image assets that caused the message ID assigned to IDR_WEBSTORE_ICON to change from 10505 to 10515. This caused cros_vm_sanity_test on chromeos-amd64-generic-rel to time out [1] due to Chrome crashing [2] with:

   [2737:2737:0828/114826.473857:FATAL:resource_bundle.cc(123)] Check failed: false. Unable to load image with id 10515, scale=2

It's quite unclear to me what's happening.
 
 (1) What exactly is `cros_vm_sanity_test` doing to trigger loading this resource? E.g. chrome://apps uses it, but I failed to reproduce the crash thee with SCALE_FACTOR_200P disabled. Are the tests force enabling 200% mode?

 (2) We are trying to load an image with scale_factor = 2, yet standard high-DPI support does not seem to be enabled on CrOS according [3] and [4]. Is there some special magic going on here?

 (3) How did this ever work with the old message ID?

For references, see:

[1] https://ci.chromium.org/p/chromium/builders/luci.chromium.try/chromeos-amd64-generic-rel/76756

[2] https://isolateserver.appspot.com/browse?namespace=default-gzip&digest=36873fea20303b22607e48344098bec00e582b74&as=chrome_20180828-114804

[3] https://cs.chromium.org/chromium/src/ui/base/ui_features.gni?rcl=3f9ae10daf7fb6558346ea21a9bf5dc4ae0e2850&l=23

[4] https://cs.chromium.org/chromium/src/ui/base/resource/resource_bundle.cc?rcl=3f9ae10daf7fb6558346ea21a9bf5dc4ae0e2850&l=769
 
Components: Infra>Client>Chrome
cros_vm_sanity_test is a smoke check of the browser. It launches chrome, logs in, executes some javascript. I think it loads the chrome://about page too.

As for the rest of your questions... I can't really speak to them (bit out of my purview), but I might have an idea as to what's happening here:

- In the failures you saw in the CL, the compiler bots package up test binaries and ship them to the test bots. The full list of files included is:
https://isolateserver.appspot.com/browse?namespace=default-gzip&hash=b4590d3b04d67aade7dc9bdda3b0f34a64a93c0a
- That list includes chrome_100_percent.pak, but not chrome_200_percent.pak.
- We deploy that list to the default chrome dir on the cros VM (/opt/google/chrome/).
- That dir comes shipped with a known-good deployment of chrome thats ~a few days old... and when we deploy your CL's version of chrome, we overwrite what's already there.
- So we ended up with a deployment of chrome that had a up-to-date version of chrome_100_percent.pak... but a much older chrome_200_percent.pak


That, I think, is the cause of the crashes you saw... but I'm not 100% sure. (What do the chrome_X_percent.pak files contain? I'm assuming pngs? Maybe the exact pngs chrome is failing to load here?) Does that sound right? If so, there's a couple bugs here:

- Add chrome_200_percent.pak to the shipped test dependency files. The list of files is generated via a test target's GN deps... so it's strange that they'd be missing. I'll track it down and figure out why they aren't present.
- Deploy chrome on the CrOS VM to a clean dir/stop trying to overwrite the default dir. Kind of a no-brainer now that I think of it... a bit surprising it took me this long to realize. Fortunately, I think that's a pretty straightforward fix. I'll get to that too.
Owner: bpastene@chromium.org
Status: Assigned (was: Available)
Ben, thanks for investigating! Regarding `chrome_200_percent.pak`, looks like we intentionally do not generate it on CrOS, see:

https://cs.chromium.org/chromium/src/chrome/chrome_paks.gni?rcl=d3ccf6b5b0b0dd14e2fe459881496f563e2dedb2&l=220
We generate it (or at least we do on HiDPI devices... which I think is all CrOS devices), we just don't add it to the data. Uploaded https://chromium-review.googlesource.com/c/chromium/src/+/1197826 which should fix that.

Also uploaded https://chromium-review.googlesource.com/c/chromiumos/chromite/+/1198502 to prevent us from reusing chrome artifacts from a previous deployment/test.

Once those land, we should no longer be susceptible to the errors you were seeing earlier.
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 5

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

commit 58b56083c8d1cb617cc215bbf5f1530198e271c8
Author: Ben Pastene <bpastene@chromium.org>
Date: Wed Sep 05 20:20:24 2018

Add chrome_200_percent.pak to a chrome_binary's data on HiDPI platforms.

The pak file gets generated when building chrome for HiDPI:
https://codesearch.chromium.org/chromium/src/chrome/chrome_paks.gni?rcl=9a8cfdf0edde3f7d61d08b5890a94c038a15f3fb&l=220

But it doesn't show up in the target's data. Consequently, it's missing
from tests' isolate. This should fix that.

Bug: 878730
Change-Id: I1e95c41a18cfe88a83235b42605c53829f9a344f
Reviewed-on: https://chromium-review.googlesource.com/1197826
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Ben Pastene <bpastene@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588986}
[modify] https://crrev.com/58b56083c8d1cb617cc215bbf5f1530198e271c8/chrome/BUILD.gn

Cc: achuith@chromium.org

Sign in to add a comment