New issue
Advanced search Search tips

Issue 899574 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 22
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

play_core support library consuming 44kb of dex in Chrome.apk

Project Member Reported by agrieve@chromium.org, Oct 29

Issue description

New in M71:
https://storage.googleapis.com/chrome-supersize/viewer.html?load_url=milestones%2Farm%2FMonochrome.apk%2Freport_70.0.3538.17_71.0.3574.0.ndjson&include=play_core&exclude=assets%2Funwind_cfi&diff_mode=on

We should figure out a way to win back this size for apks that don't use bundles (Chrome.apk).

My guess as to the best way is to use a proguard rule like:
-assumenosideeffects class org.chromium.components.module_installerModuleInstaller {
  *;
}

And include the rule for non-bundle apk targets.
 
Status: Started (was: Assigned)
Proguard didn't work out due to some GPU code failing. Not sure why. New plan is to revert the ModuleInstaller CLs for M71 and fix it properly for M72.
Labels: CommitLog-Audit-Violation Merge-Without-Approval
Here's a summary of the rules that were executed: 
 - OnlyMergeApprovedChange: Rule Failed -- Revision 396bfd307c865a4332529f1279ddc9ee23cac4f3 was merged to refs/branch-heads/3578 branch with no merge approval from a TPM! 
Please explain why this change was merged to the branch!
 - AcknowledgeMerge: Notification Required -- 
Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/396bfd307c865a4332529f1279ddc9ee23cac4f3

Commit: 396bfd307c865a4332529f1279ddc9ee23cac4f3
Author: tiborg@chromium.org
Commiter: tiborg@chromium.org
Date: 2018-11-20 22:54:26 +0000 UTC

Revert "[feature modules] Allow disk writes for SplitCompat.install"

This reverts commit 4397d75c3bd54382c67ef875b6c69fd3115922a2.

Reason for revert: only needed for bundles but we don't build bundles
for M72.

Original change's description:
> [feature modules] Allow disk writes for SplitCompat.install
>
> Bug:  888602 
> Change-Id: I9ac5818460da30057a42fff007dbcceb52b6f6a2
> Reviewed-on: https://chromium-review.googlesource.com/1244599
> Reviewed-by: agrieve <agrieve@chromium.org>
> Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#594302}

TBR=agrieve@chromium.org

Bug:  899574 
Change-Id: I87363d49be2c0657c41bbf97004c19ed86980018
Reviewed-on: https://chromium-review.googlesource.com/c/1344907
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#779}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 20

Labels: merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/396bfd307c865a4332529f1279ddc9ee23cac4f3

commit 396bfd307c865a4332529f1279ddc9ee23cac4f3
Author: Tibor Goldschwendt <tiborg@chromium.org>
Date: Tue Nov 20 22:54:26 2018

Revert "[feature modules] Allow disk writes for SplitCompat.install"

This reverts commit 4397d75c3bd54382c67ef875b6c69fd3115922a2.

Reason for revert: only needed for bundles but we don't build bundles
for M72.

Original change's description:
> [feature modules] Allow disk writes for SplitCompat.install
>
> Bug:  888602 
> Change-Id: I9ac5818460da30057a42fff007dbcceb52b6f6a2
> Reviewed-on: https://chromium-review.googlesource.com/1244599
> Reviewed-by: agrieve <agrieve@chromium.org>
> Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#594302}

TBR=agrieve@chromium.org

Bug:  899574 
Change-Id: I87363d49be2c0657c41bbf97004c19ed86980018
Reviewed-on: https://chromium-review.googlesource.com/c/1344907
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#779}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/396bfd307c865a4332529f1279ddc9ee23cac4f3/components/module_installer/android/java/src/org/chromium/components/module_installer/ModuleInstaller.java

Project Member

Comment 6 by bugdroid1@chromium.org, Nov 20

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

commit d9002403e5a956fd161f0092b619bcfd7bb48435
Author: Tibor Goldschwendt <tiborg@chromium.org>
Date: Tue Nov 20 22:55:54 2018

Revert "[ar] On-demand install AR module, add ModuleInstaller class"

This reverts commit aada02dfce485d6754e6e1a6b24d561cd164a50e.

Reason for revert: only needed for bundles but we don't build bundles
for M72.

Original change's description:
> [ar] On-demand install AR module, add ModuleInstaller class
>
> The ModuleInstaller goes into the new component module_installer, which
> hosts code necessary to install dynamic feature modules. It is used by
> Chrome to install the AR module (and other modules in the future). See
> go/chromevr-dfm-design-proposal for more information.
>
> Bug: 863068,  862690 
> Change-Id: Ib7fb7451fbd9ce08cd57e96f923a81f7327a8ddb
> Reviewed-on: https://chromium-review.googlesource.com/1228308
> Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
> Reviewed-by: Cait Phillips <caitkp@chromium.org>
> Reviewed-by: Ted Choc <tedchoc@chromium.org>
> Reviewed-by: agrieve <agrieve@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#593190}

TBR=caitkp@chromium.org,tedchoc@chromium.org,agrieve@chromium.org

Bug:  899574 
Change-Id: Id3fbafdbdfdbd3d4058409dff9eccf51944ebc46
Reviewed-on: https://chromium-review.googlesource.com/c/1345243
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#780}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/d9002403e5a956fd161f0092b619bcfd7bb48435/chrome/android/BUILD.gn
[modify] https://crrev.com/d9002403e5a956fd161f0092b619bcfd7bb48435/chrome/android/java/DEPS
[modify] https://crrev.com/d9002403e5a956fd161f0092b619bcfd7bb48435/chrome/android/java/src/org/chromium/chrome/browser/vr/ArCoreJavaUtils.java
[modify] https://crrev.com/d9002403e5a956fd161f0092b619bcfd7bb48435/chrome/android/java/strings/android_chrome_strings.grd
[modify] https://crrev.com/d9002403e5a956fd161f0092b619bcfd7bb48435/chrome/android/modules/ar/AndroidManifest.xml
[modify] https://crrev.com/d9002403e5a956fd161f0092b619bcfd7bb48435/chrome/browser/android/vr/arcore_device/arcore_device.cc
[modify] https://crrev.com/d9002403e5a956fd161f0092b619bcfd7bb48435/chrome/browser/android/vr/arcore_device/arcore_device.h
[modify] https://crrev.com/d9002403e5a956fd161f0092b619bcfd7bb48435/chrome/browser/android/vr/arcore_device/arcore_java_utils.cc
[modify] https://crrev.com/d9002403e5a956fd161f0092b619bcfd7bb48435/chrome/browser/android/vr/arcore_device/arcore_java_utils.h
[modify] https://crrev.com/d9002403e5a956fd161f0092b619bcfd7bb48435/chrome/browser/android/vr/arcore_device/arcore_shim.cc
[modify] https://crrev.com/d9002403e5a956fd161f0092b619bcfd7bb48435/chrome/browser/android/vr/arcore_device/arcore_shim.h
[delete] https://crrev.com/396bfd307c865a4332529f1279ddc9ee23cac4f3/components/module_installer/OWNERS
[delete] https://crrev.com/396bfd307c865a4332529f1279ddc9ee23cac4f3/components/module_installer/android/BUILD.gn
[delete] https://crrev.com/396bfd307c865a4332529f1279ddc9ee23cac4f3/components/module_installer/android/java/src/org/chromium/components/module_installer/ModuleInstaller.java

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

Commit: d9002403e5a956fd161f0092b619bcfd7bb48435
Author: tiborg@chromium.org
Commiter: tiborg@chromium.org
Date: 2018-11-20 22:55:54 +0000 UTC

Revert "[ar] On-demand install AR module, add ModuleInstaller class"

This reverts commit aada02dfce485d6754e6e1a6b24d561cd164a50e.

Reason for revert: only needed for bundles but we don't build bundles
for M72.

Original change's description:
> [ar] On-demand install AR module, add ModuleInstaller class
>
> The ModuleInstaller goes into the new component module_installer, which
> hosts code necessary to install dynamic feature modules. It is used by
> Chrome to install the AR module (and other modules in the future). See
> go/chromevr-dfm-design-proposal for more information.
>
> Bug: 863068,  862690 
> Change-Id: Ib7fb7451fbd9ce08cd57e96f923a81f7327a8ddb
> Reviewed-on: https://chromium-review.googlesource.com/1228308
> Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
> Reviewed-by: Cait Phillips <caitkp@chromium.org>
> Reviewed-by: Ted Choc <tedchoc@chromium.org>
> Reviewed-by: agrieve <agrieve@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#593190}

TBR=caitkp@chromium.org,tedchoc@chromium.org,agrieve@chromium.org

Bug:  899574 
Change-Id: Id3fbafdbdfdbd3d4058409dff9eccf51944ebc46
Reviewed-on: https://chromium-review.googlesource.com/c/1345243
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#780}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Here's a summary of the rules that were executed: 
 - OnlyMergeApprovedChange: Rule Failed -- Revision d9002403e5a956fd161f0092b619bcfd7bb48435 was merged to refs/branch-heads/3578 branch with no merge approval from a TPM! 
Please explain why this change was merged to the branch!
 - AcknowledgeMerge: Notification Required -- 
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 22

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

commit 8f6e4855530de2386f0983afd5b3f60a45a86dc3
Author: Tibor Goldschwendt <tiborg@chromium.org>
Date: Thu Nov 22 15:52:42 2018

Remove Play Core SDK and Module Installer from APKs

Remove to save some binary size since this code is not usable by APKs.

Bug:  899574 
Change-Id: Ia0a857ac58185f3ee83f38bba27cfed8cc9d6b29
Reviewed-on: https://chromium-review.googlesource.com/c/1321160
Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610422}
[modify] https://crrev.com/8f6e4855530de2386f0983afd5b3f60a45a86dc3/chrome/android/BUILD.gn
[modify] https://crrev.com/8f6e4855530de2386f0983afd5b3f60a45a86dc3/chrome/android/chrome_public_apk_tmpl.gni
[modify] https://crrev.com/8f6e4855530de2386f0983afd5b3f60a45a86dc3/chrome/test/BUILD.gn
[modify] https://crrev.com/8f6e4855530de2386f0983afd5b3f60a45a86dc3/components/module_installer/android/BUILD.gn
[add] https://crrev.com/8f6e4855530de2386f0983afd5b3f60a45a86dc3/components/module_installer/android/java/src-common/org/chromium/components/module_installer/OnModuleInstallFinishedListener.java
[rename] https://crrev.com/8f6e4855530de2386f0983afd5b3f60a45a86dc3/components/module_installer/android/java/src-impl/org/chromium/components/module_installer/FakeModuleInstallerBackend.java
[rename] https://crrev.com/8f6e4855530de2386f0983afd5b3f60a45a86dc3/components/module_installer/android/java/src-impl/org/chromium/components/module_installer/ModuleInstaller.java
[rename] https://crrev.com/8f6e4855530de2386f0983afd5b3f60a45a86dc3/components/module_installer/android/java/src-impl/org/chromium/components/module_installer/ModuleInstallerBackend.java
[rename] https://crrev.com/8f6e4855530de2386f0983afd5b3f60a45a86dc3/components/module_installer/android/java/src-impl/org/chromium/components/module_installer/PlayCoreModuleInstallerBackend.java
[add] https://crrev.com/8f6e4855530de2386f0983afd5b3f60a45a86dc3/components/module_installer/android/java/src-stub/org/chromium/components/module_installer/ModuleInstaller.java

Status: Fixed (was: Started)
Fixed for M72, too.

Sign in to add a comment