New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 782584 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug



Sign in to add a comment

xcassets are copied to the application bundle

Project Member Reported by sdefresne@chromium.org, Nov 8 2017

Issue description

This is incorrect, they should be passed to actool to be compiled.


 
$ ls -1d out/Debug-iphonesimulator/Chromium.app/*.xcassets/|head -n 20
out/Debug-iphonesimulator/Chromium.app/app_icon_placeholder.xcassets/
out/Debug-iphonesimulator/Chromium.app/autofill_close.xcassets/
out/Debug-iphonesimulator/Chromium.app/autofill_close_pressed.xcassets/
out/Debug-iphonesimulator/Chromium.app/autofill_keyboard_background.xcassets/
out/Debug-iphonesimulator/Chromium.app/autofill_keyboard_background_left.xcassets/
out/Debug-iphonesimulator/Chromium.app/autofill_keyboard_background_right.xcassets/
out/Debug-iphonesimulator/Chromium.app/autofill_left_sep.xcassets/
out/Debug-iphonesimulator/Chromium.app/autofill_left_sep_RTL.xcassets/
out/Debug-iphonesimulator/Chromium.app/autofill_middle_sep.xcassets/
out/Debug-iphonesimulator/Chromium.app/autofill_next.xcassets/
out/Debug-iphonesimulator/Chromium.app/autofill_next_inactive.xcassets/
out/Debug-iphonesimulator/Chromium.app/autofill_next_pressed.xcassets/
out/Debug-iphonesimulator/Chromium.app/autofill_prev.xcassets/
out/Debug-iphonesimulator/Chromium.app/autofill_prev_inactive.xcassets/
out/Debug-iphonesimulator/Chromium.app/autofill_prev_pressed.xcassets/
out/Debug-iphonesimulator/Chromium.app/autofill_right_sep.xcassets/
out/Debug-iphonesimulator/Chromium.app/autofill_right_sep_RTL.xcassets/
out/Debug-iphonesimulator/Chromium.app/bookmark_bar_innershadow.xcassets/
out/Debug-iphonesimulator/Chromium.app/bookmark_bar_shadow.xcassets/
out/Debug-iphonesimulator/Chromium.app/bookmark_black_delete.xcassets/

Components: Build
Labels: M-63
This is also affecting M-63. It means that all images are both present in the application bundle and in the compiled asset catalog (Assets.car).

This will increase the size of the application by roughly 8Mb:
$ du -ch *.xcassets|grep total
7.8M	total

I think this is a regression in gn.
Labels: -ReleaseBlock-Dev ReleaseBlock-Beta
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 8 2017

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

commit a19cb1ed33d46b889585385b39201198f506a384
Author: Sylvain Defresne <sdefresne@chromium.org>
Date: Wed Nov 08 14:21:15 2017

Avoid copying .xcassets in the application bundle.

The refactoring that introduced those templates inadvertently
regressed and caused the content of .xcassets to be copied to
the application bundle thus increasing the size of the bundle
unnecesssarily.

Bug:  782584 
Change-Id: Ib7f39d00347a821467774489ecc222a57a3fe27d
Reviewed-on: https://chromium-review.googlesource.com/758262
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514819}
[modify] https://crrev.com/a19cb1ed33d46b889585385b39201198f506a384/build/config/ios/asset_catalog.gni

Labels: -M-64 Merge-Request-63
Status: Fixed (was: Started)
Project Member

Comment 6 by sheriffbot@chromium.org, Nov 8 2017

Labels: -Merge-Request-63 Merge-Review-63 Hotlist-Merge-Review
This bug requires manual review: Less than 23 days to go before AppStore submit on M63
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: cma...@chromium.org
Can we cherry-pick this in M-63? If we don't the app will have two copies of all resources, wasting disk space on user devices for nothing.
Labels: -Hotlist-Merge-Review -Merge-Review-63 Merge-Approved-63
Not sure if you still want to approve this now that the build was rejected by Apple again.
Labels: -Merge-Approved-63 Merge-Rejected-63
Labels: -Merge-Rejected-63 Merge-Request-63
Project Member

Comment 12 by sheriffbot@chromium.org, Nov 10 2017

Labels: -Merge-Request-63 Merge-Review-63 Hotlist-Merge-Review
This bug requires manual review: Less than 21 days to go before AppStore submit on M63
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Review -Merge-Review-63 Merge-Approved-63
Approving the merge since this is unrelated to the issue I was referring to.
Project Member

Comment 14 by bugdroid1@chromium.org, Nov 13 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/de8511c1146f5a4e6d0dde917d8b6d5ae5aebdab

commit de8511c1146f5a4e6d0dde917d8b6d5ae5aebdab
Author: Sylvain Defresne <sdefresne@chromium.org>
Date: Mon Nov 13 08:50:58 2017

Avoid copying .xcassets in the application bundle.

The refactoring that introduced those templates inadvertently
regressed and caused the content of .xcassets to be copied to
the application bundle thus increasing the size of the bundle
unnecesssarily.

Bug:  782584 
Change-Id: Ib7f39d00347a821467774489ecc222a57a3fe27d
Reviewed-on: https://chromium-review.googlesource.com/758262
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#514819}(cherry picked from commit a19cb1ed33d46b889585385b39201198f506a384)
Reviewed-on: https://chromium-review.googlesource.com/765369
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#457}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/de8511c1146f5a4e6d0dde917d8b6d5ae5aebdab/build/config/ios/asset_catalog.gni

Sign in to add a comment