New issue
Advanced search Search tips

Issue 791078 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Monochrome: Rewrite R.java IDs for webview only

Project Member Reported by agrieve@chromium.org, Dec 1 2017

Issue description

This is a follow-up to  bug 669137 .

Right now, when Monochrome is loaded as a webview, we're re-writing all R constants. We really need to rewrite only the R constants that webview actually uses.

I propose that we use the R.txt from system_webview_apk as a whitelist for resource IDs that should be made non-final & rewritten in onResourceLoaded(). All other resources should be marked as final.

This should reduce code size a bit, and also improve load time a bit.
 

Comment 1 by torne@chromium.org, Dec 1 2017

aapt writes the R class, though, no? I assumed we can't have some of them be final and some of them not as a result; is there a way to do this?

(though agreed that we can just not generate the rewriter code for the ones that don't need to be rewritten and that will still be beneficial)
For some reason that predates me joining the project, we have our own template for writing R.java files :P

https://cs.chromium.org/chromium/src/build/android/gyp/process_resources.py?rcl=6713d85d1567baaa3def753ed8e9bf8ee29185e2&l=342

Comment 3 by torne@chromium.org, Dec 1 2017

The comment says we have so many resources that the populate method ends up being too big, which is horrifying. :)
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 19 2017

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

commit 04a7ee63332a8496e0d44e40d3033c03447064a7
Author: Mohamed Heikal <mheikal@google.com>
Date: Tue Dec 19 17:40:36 2017

Android: Pass whitelist of shared resources for public Monochrome

Allows monochrome_public_apk to specify a whitelist of resources
that are used by the webview portion of Monochrome. This change only
marks those as non-final and changes their package id at runtime
(the rest of the resources are marked as final).

Bug:  791078 
Change-Id: Id32858c014cabd7d259e00cf603d2d949b690b5e
Reviewed-on: https://chromium-review.googlesource.com/815235
Commit-Queue: Mohamed Heikal <mheikal@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525075}
[modify] https://crrev.com/04a7ee63332a8496e0d44e40d3033c03447064a7/build/android/gyp/process_resources.py
[modify] https://crrev.com/04a7ee63332a8496e0d44e40d3033c03447064a7/build/config/android/internal_rules.gni
[modify] https://crrev.com/04a7ee63332a8496e0d44e40d3033c03447064a7/build/config/android/rules.gni
[modify] https://crrev.com/04a7ee63332a8496e0d44e40d3033c03447064a7/chrome/android/BUILD.gn

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 19 2017

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

commit 5e7dcbcb49b37cf53d85232d05828c8a9492d295
Author: John Budorick <jbudorick@chromium.org>
Date: Tue Dec 19 23:50:27 2017

Revert "Android: Pass whitelist of shared resources for public Monochrome"

This reverts commit 04a7ee63332a8496e0d44e40d3033c03447064a7.

Reason for revert: breaks downstream builders, https://bugs.chromium.org/p/chromium/issues/detail?id=796374

Original change's description:
> Android: Pass whitelist of shared resources for public Monochrome
> 
> Allows monochrome_public_apk to specify a whitelist of resources
> that are used by the webview portion of Monochrome. This change only
> marks those as non-final and changes their package id at runtime
> (the rest of the resources are marked as final).
> 
> Bug:  791078 
> Change-Id: Id32858c014cabd7d259e00cf603d2d949b690b5e
> Reviewed-on: https://chromium-review.googlesource.com/815235
> Commit-Queue: Mohamed Heikal <mheikal@chromium.org>
> Reviewed-by: agrieve <agrieve@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#525075}

TBR=agrieve@chromium.org,mheikal@chromium.org

Change-Id: Ie7c1905ba9cacf8d08ccac7f729b893d077a112b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  791078 
Reviewed-on: https://chromium-review.googlesource.com/835209
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: John Budorick <jbudorick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525186}
[modify] https://crrev.com/5e7dcbcb49b37cf53d85232d05828c8a9492d295/build/android/gyp/process_resources.py
[modify] https://crrev.com/5e7dcbcb49b37cf53d85232d05828c8a9492d295/build/config/android/internal_rules.gni
[modify] https://crrev.com/5e7dcbcb49b37cf53d85232d05828c8a9492d295/build/config/android/rules.gni
[modify] https://crrev.com/5e7dcbcb49b37cf53d85232d05828c8a9492d295/chrome/android/BUILD.gn

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 20 2017

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

commit 3106e184c6570acfcce4baac6e887e7a6bcae794
Author: Mohamed Heikal <mheikal@google.com>
Date: Wed Dec 20 21:58:07 2017

Reland "Android: Pass whitelist of shared resources for public Monochrome"

This reverts commit 5e7dcbcb49b37cf53d85232d05828c8a9492d295.

Reason for reland: Fixed bug when system_webview_apk is not defined

Bug:  791078 
Change-Id: Ib5c0ee60bef2eb79c6136ebfafc10880820e6115
Reviewed-on: https://chromium-review.googlesource.com/837007
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Mohamed Heikal <mheikal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525480}
[modify] https://crrev.com/3106e184c6570acfcce4baac6e887e7a6bcae794/build/android/gyp/process_resources.py
[modify] https://crrev.com/3106e184c6570acfcce4baac6e887e7a6bcae794/build/config/android/internal_rules.gni
[modify] https://crrev.com/3106e184c6570acfcce4baac6e887e7a6bcae794/build/config/android/rules.gni
[modify] https://crrev.com/3106e184c6570acfcce4baac6e887e7a6bcae794/chrome/android/BUILD.gn

Status: Fixed (was: Assigned)
Project Member

Comment 8 by bugdroid1@chromium.org, Jan 19 2018

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

commit 9991bb8cd9ab9e60ed36ed44395346378bd668e8
Author: Eric Stevenson <estevenson@chromium.org>
Date: Fri Jan 19 13:16:03 2018

Android: Cleanup Monochrome shared resource whitelist usage.

This CL unifies the upstream and downstream usage of the whitelist and
removes obsolete arguments.

Bug:  791078 , 762130
Change-Id: I77a022218a7b8a333f252169d8b074c8757cee1f
Reviewed-on: https://chromium-review.googlesource.com/875205
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Eric Stevenson <estevenson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530497}
[modify] https://crrev.com/9991bb8cd9ab9e60ed36ed44395346378bd668e8/build/config/android/internal_rules.gni
[modify] https://crrev.com/9991bb8cd9ab9e60ed36ed44395346378bd668e8/build/config/android/rules.gni

Project Member

Comment 9 by bugdroid1@chromium.org, Jan 19 2018

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/72982e5cd521b5daacee84a07402856c840bfda2

commit 72982e5cd521b5daacee84a07402856c840bfda2
Author: Eric Stevenson <estevenson@google.com>
Date: Fri Jan 19 14:40:11 2018

Project Member

Comment 10 by bugdroid1@chromium.org, Jan 29 2018

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/ae1384a16873517de7424dbef3bcc2e44e83f227

commit ae1384a16873517de7424dbef3bcc2e44e83f227
Author: Eric Stevenson <estevenson@google.com>
Date: Mon Jan 29 15:03:46 2018

Project Member

Comment 11 by bugdroid1@chromium.org, Mar 6 2018

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/96bb2009f71ccb922c33ecaa8f0157d1452e74ea

commit 96bb2009f71ccb922c33ecaa8f0157d1452e74ea
Author: Mohamed Heikal <mheikal@google.com>
Date: Tue Mar 06 17:21:35 2018

Project Member

Comment 12 by bugdroid1@chromium.org, Mar 6 2018

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/fecb9ca6bb0f2ad661c3e239494f50ada72889bf

commit fecb9ca6bb0f2ad661c3e239494f50ada72889bf
Author: Mohamed Heikal <mheikal@google.com>
Date: Tue Mar 06 19:21:37 2018

 Issue 692554  has been merged into this issue.

Sign in to add a comment