Split onResourcesLoaded into multiple functions.

Split onResourcesLoaded into multiple functions.

We have so many resources that onResourcesLoaded is exceeding the 64KB limit
Java has on methods. This CL splits onResourcesLoaded based on resource type,
eg: onResourcesLoadedAnim, onResourcesLoadedDrawable, onResourcesLoadedString.

BUG=629768
Committed: https://crrev.com/2c63be44901ccd5fe3af5323ca494959d497628a
Cr-Commit-Position: refs/heads/master@{#407864}

[PEConn, 2016-07-20 15:58:40]

peconn@chromium.org changed reviewers:
+ agrieve@chromium.org, perezju@chromium.org

[PEConn, 2016-07-20 15:58:40]

perezju@ and agrieve@ please take a look.

https://codereview.chromium.org/2163993002/diff/1/build/android/gyp/process_r...
File build/android/gyp/process_resources.py (right):

https://codereview.chromium.org/2163993002/diff/1/build/android/gyp/process_r...
build/android/gyp/process_resources.py:238: private static void
onResourcesLoaded{{ resource_type|title }}(int packageId) {
This exceeds the line length limit but in a template string. I'm not sure if
this is acceptable and if not, what a preferable formatting is.

[PEConn, 2016-07-20 15:59:08]

Description was changed from

==========
Split onResourcesLoaded into multiple functions.

We have so many resources that onResourcesLoaded is exceeding the 64KB limit
Java has on methods. This CL splits onResourcesLoaded based on resource type,
eg: onResourcesLoadedAnim, onResourcesLoadedDrawable, onResourcesLoadedString.

BUG=629768
==========

to

==========
Split onResourcesLoaded into multiple functions.

We have so many resources that onResourcesLoaded is exceeding the 64KB limit
Java has on methods. This CL splits onResourcesLoaded based on resource type,
eg: onResourcesLoadedAnim, onResourcesLoadedDrawable, onResourcesLoadedString.

BUG=629768
==========

[PEConn, 2016-07-20 15:59:08]

peconn@chromium.org changed reviewers:
+ bauerb@chromium.org

[PEConn, 2016-07-20 16:00:46]

Example output:

    public static void onResourcesLoaded(int packageId) {                       
   
        onResourcesLoadedAnim(packageId);                                       
   
        onResourcesLoadedArray(packageId);                                      
   
        onResourcesLoadedAttr(packageId);                                       
   
        onResourcesLoadedBool(packageId);                                       
   
        onResourcesLoadedColor(packageId);                                      
   
        onResourcesLoadedDimen(packageId);                                      
   
        onResourcesLoadedDrawable(packageId);                                   
   
        onResourcesLoadedFloats(packageId);                                     
   
        onResourcesLoadedFraction(packageId);                                   
   
        onResourcesLoadedId(packageId);                                         
   
        onResourcesLoadedInteger(packageId);                                    
   
        onResourcesLoadedInterpolator(packageId);                               
   
        onResourcesLoadedLayout(packageId);                                     
   
        onResourcesLoadedMenu(packageId);                                       
   
        onResourcesLoadedMipmap(packageId);                                     
   
        onResourcesLoadedPlurals(packageId);                                    
   
        onResourcesLoadedRaw(packageId);                                        
   
        onResourcesLoadedString(packageId);                                     
   
        onResourcesLoadedStyle(packageId);                                      
   
        onResourcesLoadedStyleable(packageId);                                  
   
        for(int i = 0; i < styleable.ActionBar.length; ++i) {                   
   
            styleable.ActionBar[i] = (styleable.ActionBar[i] & 0x00ffffff) |
(packageId << 24); 
        }
        ...
    }

    private static void onResourcesLoadedAnim(int packageId) {                  
        anim.abc_fade_in = (anim.abc_fade_in & 0x00ffffff) | (packageId << 24); 
        anim.abc_fade_out = (anim.abc_fade_out & 0x00ffffff) | (packageId <<
24);
        anim.abc_grow_fade_in_from_bottom = (anim.abc_grow_fade_in_from_bottom &
0x00ffffff) | (packageId << 24);
        anim.abc_popup_enter = (anim.abc_popup_enter & 0x00ffffff) | (packageId
<< 24);
        anim.abc_popup_exit = (anim.abc_popup_exit & 0x00ffffff) | (packageId <<
24);
        anim.abc_shrink_fade_out_from_bottom =
(anim.abc_shrink_fade_out_from_bottom & 0x00ffffff) | (packageId << 24);
        anim.abc_slide_in_bottom = (anim.abc_slide_in_bottom & 0x00ffffff) |
(packageId << 24);
        anim.abc_slide_in_top = (anim.abc_slide_in_top & 0x00ffffff) |
(packageId << 24);
        anim.abc_slide_out_bottom = (anim.abc_slide_out_bottom & 0x00ffffff) |
(packageId << 24);
        anim.abc_slide_out_top = (anim.abc_slide_out_top & 0x00ffffff) |
(packageId << 24);
        anim.accelerate_quart = (anim.accelerate_quart & 0x00ffffff) |
(packageId << 24);
        anim.activity_close_exit = (anim.activity_close_exit & 0x00ffffff) |
(packageId << 24);
        anim.activity_open_enter = (anim.activity_open_enter & 0x00ffffff) |
(packageId << 24);
        anim.decelerate_quart = (anim.decelerate_quart & 0x00ffffff) |
(packageId << 24);
        anim.fullscreen_notification_in = (anim.fullscreen_notification_in &
0x00ffffff) | (packageId << 24);
        anim.menu_enter = (anim.menu_enter & 0x00ffffff) | (packageId << 24);   
        anim.menu_exit = (anim.menu_exit & 0x00ffffff) | (packageId << 24);     
        anim.no_anim = (anim.no_anim & 0x00ffffff) | (packageId << 24);         
        anim.snackbar_in = (anim.snackbar_in & 0x00ffffff) | (packageId << 24); 
        anim.tab_switcher_callout_in = (anim.tab_switcher_callout_in &
0x00ffffff) | (packageId << 24);
        anim.tab_switcher_callout_out = (anim.tab_switcher_callout_out &
0x00ffffff) | (packageId << 24);
    }

...

[Bernhard Bauer, 2016-07-20 16:18:22]

LGTM if others are happy.

[PEConn, 2016-07-20 16:29:23]

The CQ bit was checked by peconn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-20 16:29:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-20 16:40:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-20 16:40:19]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[PEConn, 2016-07-20 16:58:26]

https://codereview.chromium.org/2163993002/diff/1/build/android/gyp/process_r...
File build/android/gyp/process_resources.py (right):

https://codereview.chromium.org/2163993002/diff/1/build/android/gyp/process_r...
build/android/gyp/process_resources.py:238: private static void
onResourcesLoaded{{ resource_type|title }}(int packageId) {
On 2016/07/20 15:58:40, PEConn1 wrote:
> This exceeds the line length limit but in a template string. I'm not sure if
> this is acceptable and if not, what a preferable formatting is.

The line length is causing the presubmit to fail.

[Bernhard Bauer, 2016-07-20 17:08:34]

https://codereview.chromium.org/2163993002/diff/1/build/android/gyp/process_r...
File build/android/gyp/process_resources.py (right):

https://codereview.chromium.org/2163993002/diff/1/build/android/gyp/process_r...
build/android/gyp/process_resources.py:238: private static void
onResourcesLoaded{{ resource_type|title }}(int packageId) {
On 2016/07/20 16:58:26, PEConn1 wrote:
> On 2016/07/20 15:58:40, PEConn1 wrote:
> > This exceeds the line length limit but in a template string. I'm not sure if
> > this is acceptable and if not, what a preferable formatting is.
> 
> The line length is causing the presubmit to fail.

This is just a string, right? You could construct the string from shorter
strings somehow.

(Also, this problem seems kind of meta. :D )

[agrieve, 2016-07-20 17:20:27]

lgtm

https://codereview.chromium.org/2163993002/diff/1/build/android/gyp/process_r...
File build/android/gyp/process_resources.py (right):

https://codereview.chromium.org/2163993002/diff/1/build/android/gyp/process_r...
build/android/gyp/process_resources.py:238: private static void
onResourcesLoaded{{ resource_type|title }}(int packageId) {
On 2016/07/20 17:08:33, Bernhard Bauer wrote:
> On 2016/07/20 16:58:26, PEConn1 wrote:
> > On 2016/07/20 15:58:40, PEConn1 wrote:
> > > This exceeds the line length limit but in a template string. I'm not sure
if
> > > this is acceptable and if not, what a preferable formatting is.
> > 
> > The line length is causing the presubmit to fail.
> 
> This is just a string, right? You could construct the string from shorter
> strings somehow.
> 
> (Also, this problem seems kind of meta. :D )

yeah, I'd say choose a shorter variable name for packageId or resource_type and
then move along :P

[agrieve, 2016-07-20 17:21:36]

https://codereview.chromium.org/2163993002/diff/1/build/android/gyp/process_r...
File build/android/gyp/process_resources.py (right):

https://codereview.chromium.org/2163993002/diff/1/build/android/gyp/process_r...
build/android/gyp/process_resources.py:227: onResourcesLoaded{{
resource_type|title }}(packageId);
Actually, please do a add a {# comment #} here pointing out that this differs
from aapt and is required to work around this method size error.

[PEConn, 2016-07-20 17:41:37]

https://codereview.chromium.org/2163993002/diff/1/build/android/gyp/process_r...
File build/android/gyp/process_resources.py (right):

https://codereview.chromium.org/2163993002/diff/1/build/android/gyp/process_r...
build/android/gyp/process_resources.py:238: private static void
onResourcesLoaded{{ resource_type|title }}(int packageId) {
On 2016/07/20 17:20:27, agrieve wrote:
> On 2016/07/20 17:08:33, Bernhard Bauer wrote:
> > On 2016/07/20 16:58:26, PEConn1 wrote:
> > > On 2016/07/20 15:58:40, PEConn1 wrote:
> > > > This exceeds the line length limit but in a template string. I'm not
sure
> if
> > > > this is acceptable and if not, what a preferable formatting is.
> > > 
> > > The line length is causing the presubmit to fail.
> > 
> > This is just a string, right? You could construct the string from shorter
> > strings somehow.
> > 
> > (Also, this problem seems kind of meta. :D )
> 
> yeah, I'd say choose a shorter variable name for packageId or resource_type
and
> then move along :P

Done.

[PEConn, 2016-07-20 17:41:50]

The CQ bit was checked by peconn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-20 17:42:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-20 18:44:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-20 18:44:22]

Dry run: This issue passed the CQ dry run.

[michaelbai, 2016-07-21 01:56:02]

michaelbai@chromium.org changed reviewers:
+ michaelbai@chromium.org

[michaelbai, 2016-07-21 01:56:02]

hold on, we probably don't need this.

[michaelbai, 2016-07-21 01:59:32]

On 2016/07/21 01:56:02, michaelbai wrote:
> hold on, we probably don't need this.

I can't reproduce this issue locally after update the android_system.jar, let's
see if release bot issue is fixed.

[michaelbai, 2016-07-21 05:25:05]

On 2016/07/21 01:59:32, michaelbai wrote:
> On 2016/07/21 01:56:02, michaelbai wrote:
> > hold on, we probably don't need this.
> 
> I can't reproduce this issue locally after update the android_system.jar,
let's
> see if release bot issue is fixed.

Apparently, we don't need this.

[PEConn, 2016-07-21 05:40:31]

On 2016/07/21 05:25:05, michaelbai wrote:
> On 2016/07/21 01:59:32, michaelbai wrote:
> > On 2016/07/21 01:56:02, michaelbai wrote:
> > > hold on, we probably don't need this.
> > 
> > I can't reproduce this issue locally after update the android_system.jar,
> let's
> > see if release bot issue is fixed.
> 
> Apparently, we don't need this.

How does the update solve the issue? Are we just delaying it by reducing the
number of resources?

[Ted C, 2016-07-23 01:06:47]

On 2016/07/21 05:40:31, PEConn1 wrote:
> On 2016/07/21 05:25:05, michaelbai wrote:
> > On 2016/07/21 01:59:32, michaelbai wrote:
> > > On 2016/07/21 01:56:02, michaelbai wrote:
> > > > hold on, we probably don't need this.
> > > 
> > > I can't reproduce this issue locally after update the android_system.jar,
> > let's
> > > see if release bot issue is fixed.
> > 
> > Apparently, we don't need this.
> 
> How does the update solve the issue? Are we just delaying it by reducing the
> number of resources?

We hit this issue again when we tried to roll to the 24.1.1 support library,
so we might need this after all.  Or we might find the support library was
incorrectly including 63K resources.  Any easy way to see how many we are
at right now to see how much breathing room we have?

[michaelbai, 2016-07-23 01:16:00]

On 2016/07/23 01:06:47, Ted C wrote:
> On 2016/07/21 05:40:31, PEConn1 wrote:
> > On 2016/07/21 05:25:05, michaelbai wrote:
> > > On 2016/07/21 01:59:32, michaelbai wrote:
> > > > On 2016/07/21 01:56:02, michaelbai wrote:
> > > > > hold on, we probably don't need this.
> > > > 
> > > > I can't reproduce this issue locally after update the
android_system.jar,
> > > let's
> > > > see if release bot issue is fixed.
> > > 
> > > Apparently, we don't need this.
> > 
> > How does the update solve the issue? Are we just delaying it by reducing the
> > number of resources?
> 
> We hit this issue again when we tried to roll to the 24.1.1 support library,
> so we might need this after all.  Or we might find the support library was
> incorrectly including 63K resources.  Any easy way to see how many we are
> at right now to see how much breathing room we have?

The 24.1.1 is for NYC mr1 or NYC? If it is for NYC, we probably should update
the android_system.jar.

[perezju, 2016-07-25 09:24:19]

removing myself as reviewer, I think others can provide more valuable input

[perezju, 2016-07-25 09:24:36]

perezju@chromium.org changed reviewers:
- perezju@chromium.org

[PEConn, 2016-07-26 15:12:05]

On 2016/07/23 01:06:47, Ted C wrote:
> On 2016/07/21 05:40:31, PEConn1 wrote:
> > On 2016/07/21 05:25:05, michaelbai wrote:
> > > On 2016/07/21 01:59:32, michaelbai wrote:
> > > > On 2016/07/21 01:56:02, michaelbai wrote:
> > > > > hold on, we probably don't need this.
> > > > 
> > > > I can't reproduce this issue locally after update the
android_system.jar,
> > > let's
> > > > see if release bot issue is fixed.
> > > 
> > > Apparently, we don't need this.
> > 
> > How does the update solve the issue? Are we just delaying it by reducing the
> > number of resources?
> 
> We hit this issue again when we tried to roll to the 24.1.1 support library,
> so we might need this after all.  Or we might find the support library was
> incorrectly including 63K resources.  Any easy way to see how many we are
> at right now to see how much breathing room we have?

So I believe the following procedure will tell us how many resources we are at
right now:

> cd //out/Release
> jar xf gen/chrome/android/chrome_java_resources.srcjar
org/chromium/chrome/R.java
> javac org/chromium/chrome/R.java
> javap -c org.chromium.chrome.R | tail -n 5
    59753: ishl
    59754: ior
    59755: putstatic     #4179              // Field
org/chromium/chrome/R$xml.website_preferences:I
    59758: return
}

The Java decompiler gives the offset of each line of bytecode, so the method
size is the number you see before return (plus the size of a return, 2).

I did this with a build from these parameters:
gn gen out_dist/Release --args='target_os="android" use_goma=true
use_signing_keys=true is_debug=false is_official_build=true'

The method was 59760 bytes long which is 59760/65536 = 91% of the limit.

[PEConn, 2016-07-26 15:14:09]

On 2016/07/26 15:12:05, PEConn1 wrote:
> On 2016/07/23 01:06:47, Ted C wrote:
> > On 2016/07/21 05:40:31, PEConn1 wrote:
> > > On 2016/07/21 05:25:05, michaelbai wrote:
> > > > On 2016/07/21 01:59:32, michaelbai wrote:
> > > > > On 2016/07/21 01:56:02, michaelbai wrote:
> > > > > > hold on, we probably don't need this.
> > > > > 
> > > > > I can't reproduce this issue locally after update the
> android_system.jar,
> > > > let's
> > > > > see if release bot issue is fixed.
> > > > 
> > > > Apparently, we don't need this.
> > > 
> > > How does the update solve the issue? Are we just delaying it by reducing
the
> > > number of resources?
> > 
> > We hit this issue again when we tried to roll to the 24.1.1 support library,
> > so we might need this after all.  Or we might find the support library was
> > incorrectly including 63K resources.  Any easy way to see how many we are
> > at right now to see how much breathing room we have?
> 
> So I believe the following procedure will tell us how many resources we are at
> right now:
> 
> > cd //out/Release
> > jar xf gen/chrome/android/chrome_java_resources.srcjar
> org/chromium/chrome/R.java
> > javac org/chromium/chrome/R.java
> > javap -c org.chromium.chrome.R | tail -n 5
>     59753: ishl
>     59754: ior
>     59755: putstatic     #4179              // Field
> org/chromium/chrome/R$xml.website_preferences:I
>     59758: return
> }
> 
> The Java decompiler gives the offset of each line of bytecode, so the method
> size is the number you see before return (plus the size of a return, 2).
> 
> I did this with a build from these parameters:
> gn gen out_dist/Release --args='target_os="android" use_goma=true
> use_signing_keys=true is_debug=false is_official_build=true'
> 
> The method was 59760 bytes long which is 59760/65536 = 91% of the limit.

(Both lines that start with org/chromium should be on the previous line, the
line breaking messed with the formatting)

[Ian Wen, 2016-07-26 17:24:35]

The CQ bit was checked by ianwen@chromium.org

[Ian Wen, 2016-07-26 17:24:35]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org,
agrieve@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/2163993002/#ps20001
(title: "Took the easy way out.")

[commit-bot: I haz the power, 2016-07-26 17:24:46]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ian Wen, 2016-07-26 17:29:59]

The CQ bit was unchecked by ianwen@chromium.org

[PEConn, 2016-07-26 17:35:15]

The CQ bit was checked by peconn@chromium.org

[PEConn, 2016-07-26 17:35:15]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org,
agrieve@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/2163993002/#ps40001
(title: "Moved ugly name to template variable")

[commit-bot: I haz the power, 2016-07-26 17:35:41]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[PEConn, 2016-07-26 17:36:01]

On 2016/07/26 17:24:46, commit-bot: I haz the power wrote:
> CQ is trying da patch. Follow status at
>  
>
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

Apparently the failure crops up when trying to build monochrome so I'm
committing the fix.

[commit-bot: I haz the power, 2016-07-26 17:46:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-26 17:46:02]

Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[PEConn, 2016-07-26 17:47:35]

The CQ bit was checked by peconn@chromium.org

[PEConn, 2016-07-26 17:47:36]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org,
agrieve@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/2163993002/#ps60001
(title: "Fixed stupid bug")

[commit-bot: I haz the power, 2016-07-26 17:48:01]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-26 18:41:59]

Description was changed from

==========
Split onResourcesLoaded into multiple functions.

We have so many resources that onResourcesLoaded is exceeding the 64KB limit
Java has on methods. This CL splits onResourcesLoaded based on resource type,
eg: onResourcesLoadedAnim, onResourcesLoadedDrawable, onResourcesLoadedString.

BUG=629768
==========

to

==========
Split onResourcesLoaded into multiple functions.

We have so many resources that onResourcesLoaded is exceeding the 64KB limit
Java has on methods. This CL splits onResourcesLoaded based on resource type,
eg: onResourcesLoadedAnim, onResourcesLoadedDrawable, onResourcesLoadedString.

BUG=629768
==========

[commit-bot: I haz the power, 2016-07-26 18:42:01]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-07-26 18:46:18]

Description was changed from

==========
Split onResourcesLoaded into multiple functions.

We have so many resources that onResourcesLoaded is exceeding the 64KB limit
Java has on methods. This CL splits onResourcesLoaded based on resource type,
eg: onResourcesLoadedAnim, onResourcesLoadedDrawable, onResourcesLoadedString.

BUG=629768
==========

to

==========
Split onResourcesLoaded into multiple functions.

We have so many resources that onResourcesLoaded is exceeding the 64KB limit
Java has on methods. This CL splits onResourcesLoaded based on resource type,
eg: onResourcesLoadedAnim, onResourcesLoadedDrawable, onResourcesLoadedString.

BUG=629768

Committed: https://crrev.com/2c63be44901ccd5fe3af5323ca494959d497628a
Cr-Commit-Position: refs/heads/master@{#407864}
==========

[commit-bot: I haz the power, 2016-07-26 18:46:19]

Patchset 4 (id:??) landed as
https://crrev.com/2c63be44901ccd5fe3af5323ca494959d497628a
Cr-Commit-Position: refs/heads/master@{#407864}