|
|
DescriptionIn test code, allow any 100% data pack to provide fallbacks.
A test is allowed to request a 200% resource and fallback to a 100%
resource. Currently only the last-added 100% data pack provides
fallbacks, which may only be a small supplement.
Instead, scan through all 100% data packs. But only when
InitSharedInstanceWithPakPath() has invoked
ResourceBundle::LoadTestResources().
BUG=636995
Committed: https://crrev.com/16fe440227ce9e085117faa4c4b7af197de88602
Cr-Commit-Position: refs/heads/master@{#427272}
Patch Set 1 #
Total comments: 2
Patch Set 2 : separate loop #
Total comments: 1
Messages
Total messages: 26 (16 generated)
The CQ bit was checked by tapted@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== In test code, only allow the first 100% data pack to provide fallbacks. There may be multiple .pak files that provide 100% resources. A test is allowed to request a 200% resource and fallback to a 100% resource. But if a test requires supplementary resources they currently replace the fallback mechanism. The test fallback mechanism is provided by ResourceBundle::is_test_resources_ which is set in InitSharedInstanceWithPakPath(), so only .pak files loaded there should participate in the fallback mechanism. BUG=636995 ========== to ========== In test code, only allow the first 100% data pack to provide fallbacks. There may be multiple .pak files that provide 100% resources. A test is allowed to request a 200% resource and fallback to a 100% resource. But if a test requires supplementary resources they currently clobber the fallback mechanism. The test fallback mechanism is provided by ResourceBundle::is_test_resources_ which is set in InitSharedInstanceWithPakPath(), so only .pak files loaded there should participate in the fallback mechanism. BUG=636995 ==========
tapted@chromium.org changed reviewers: + oshima@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Hi oshima, please take a look
https://codereview.chromium.org/2406763002/diff/1/ui/base/resource/resource_b... File ui/base/resource/resource_bundle.cc (right): https://codereview.chromium.org/2406763002/diff/1/ui/base/resource/resource_b... ui/base/resource/resource_bundle.cc:835: // may provide test fallbacks. if this is for tets, can you also check is_test_resources_ as well?
Patchset #2 (id:20001) has been deleted
Description was changed from ========== In test code, only allow the first 100% data pack to provide fallbacks. There may be multiple .pak files that provide 100% resources. A test is allowed to request a 200% resource and fallback to a 100% resource. But if a test requires supplementary resources they currently clobber the fallback mechanism. The test fallback mechanism is provided by ResourceBundle::is_test_resources_ which is set in InitSharedInstanceWithPakPath(), so only .pak files loaded there should participate in the fallback mechanism. BUG=636995 ========== to ========== In test code, allow any 100% data pack to provide fallbacks. A test is allowed to request a 200% resource and fallback to a 100% resource. Currently only the last-added 100% data pack provides fallbacks, which may only be a small supplement. Instead, scan through all 100% data packs. But only when InitSharedInstanceWithPakPath() has invoked ResourceBundle::LoadTestResources(). BUG=636995 ==========
The CQ bit was checked by tapted@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
oshima, PTAL. Thanks! https://chromiumcodereview.appspot.com/2406763002/diff/1/ui/base/resource/res... File ui/base/resource/resource_bundle.cc (right): https://chromiumcodereview.appspot.com/2406763002/diff/1/ui/base/resource/res... ui/base/resource/resource_bundle.cc:835: // may provide test fallbacks. On 2016/10/12 04:22:25, oshima wrote: > if this is for tets, can you also check is_test_resources_ as well? Done. Actually, separated out the `is_test_resources_` logic to a separate loop - neater, and also more flexible.
lgtm https://codereview.chromium.org/2406763002/diff/40001/ui/base/resource/resour... File ui/base/resource/resource_bundle.h (right): https://codereview.chromium.org/2406763002/diff/40001/ui/base/resource/resour... ui/base/resource/resource_bundle.h:358: // |scale_factor| will be set to SCALE_FACTOR_NONE. Thank you for catching this. Looks like I forgot to update this when impl has changed.
lgtm
tapted@chromium.org changed reviewers: + sky@chromium.org
+sky for /ui OWNERS - Thanks!
LGTM
The CQ bit was checked by tapted@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== In test code, allow any 100% data pack to provide fallbacks. A test is allowed to request a 200% resource and fallback to a 100% resource. Currently only the last-added 100% data pack provides fallbacks, which may only be a small supplement. Instead, scan through all 100% data packs. But only when InitSharedInstanceWithPakPath() has invoked ResourceBundle::LoadTestResources(). BUG=636995 ========== to ========== In test code, allow any 100% data pack to provide fallbacks. A test is allowed to request a 200% resource and fallback to a 100% resource. Currently only the last-added 100% data pack provides fallbacks, which may only be a small supplement. Instead, scan through all 100% data packs. But only when InitSharedInstanceWithPakPath() has invoked ResourceBundle::LoadTestResources(). BUG=636995 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== In test code, allow any 100% data pack to provide fallbacks. A test is allowed to request a 200% resource and fallback to a 100% resource. Currently only the last-added 100% data pack provides fallbacks, which may only be a small supplement. Instead, scan through all 100% data packs. But only when InitSharedInstanceWithPakPath() has invoked ResourceBundle::LoadTestResources(). BUG=636995 ========== to ========== In test code, allow any 100% data pack to provide fallbacks. A test is allowed to request a 200% resource and fallback to a 100% resource. Currently only the last-added 100% data pack provides fallbacks, which may only be a small supplement. Instead, scan through all 100% data packs. But only when InitSharedInstanceWithPakPath() has invoked ResourceBundle::LoadTestResources(). BUG=636995 Committed: https://crrev.com/16fe440227ce9e085117faa4c4b7af197de88602 Cr-Commit-Position: refs/heads/master@{#427272} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/16fe440227ce9e085117faa4c4b7af197de88602 Cr-Commit-Position: refs/heads/master@{#427272} |