Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Financial Connections] Eagerly finish activities if they're started with invalid args. #5891

Merged

Conversation

carlosmuvi-stripe
Copy link
Collaborator

@carlosmuvi-stripe carlosmuvi-stripe commented Nov 30, 2022

Summary

Adds viewModelLazy mavericks extension that prevents Mavericks ViewModel eagerly in onCreate.

BANKCON-5821

Testing

  • Added tests
  • Modified tests
  • Manually verified

@github-actions
Copy link
Contributor

github-actions bot commented Nov 30, 2022

Diffuse output:

OLD: financial-connections-example-release-base.apk (signature: none)
NEW: financial-connections-example-release-pr.apk (signature: none)

          │            compressed            │          uncompressed          
          ├───────────┬───────────┬──────────┼───────────┬───────────┬────────
 APK      │ old       │ new       │ diff     │ old       │ new       │ diff   
──────────┼───────────┼───────────┼──────────┼───────────┼───────────┼────────
      dex │     4 MiB │     4 MiB │ -1.4 KiB │  10.1 MiB │  10.1 MiB │ -2 KiB 
     arsc │   1.5 MiB │   1.5 MiB │      0 B │   1.5 MiB │   1.5 MiB │    0 B 
 manifest │   3.2 KiB │   3.2 KiB │      0 B │  14.6 KiB │  14.6 KiB │    0 B 
      res │ 630.3 KiB │ 630.3 KiB │      0 B │ 958.3 KiB │ 958.3 KiB │    0 B 
    asset │  17.1 KiB │  17.1 KiB │    -51 B │  21.4 KiB │  21.3 KiB │  -51 B 
    other │  78.4 KiB │  78.4 KiB │      0 B │ 157.7 KiB │ 157.7 KiB │    0 B 
──────────┼───────────┼───────────┼──────────┼───────────┼───────────┼────────
    total │   6.2 MiB │   6.2 MiB │ -1.4 KiB │  12.7 MiB │  12.7 MiB │ -2 KiB 

         │         raw          │              unique               
         ├───────┬───────┬──────┼───────┬───────┬───────────────────
 DEX     │ old   │ new   │ diff │ old   │ new   │ diff              
─────────┼───────┼───────┼──────┼───────┼───────┼───────────────────
   files │     2 │     2 │    0 │       │       │                   
 strings │ 59139 │ 59074 │  -65 │ 56825 │ 56836 │ +11 (+88 -77)     
   types │ 16391 │ 16349 │  -42 │ 15303 │ 15305 │  +2 (+63 -61)     
 classes │ 13461 │ 13463 │   +2 │ 13461 │ 13463 │  +2 (+51 -49)     
 methods │ 71965 │ 71904 │  -61 │ 69891 │ 69898 │  +7 (+2509 -2502) 
  fields │ 55974 │ 55951 │  -23 │ 54934 │ 54931 │  -3 (+2535 -2538) 

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  288 │  288 │  0   
 entries │ 5227 │ 5227 │  0
APK
      compressed      │    uncompressed     │                                
───────────┬──────────┼──────────┬──────────┤                                
 size      │ diff     │ size     │ diff     │ path                           
───────────┼──────────┼──────────┼──────────┼────────────────────────────────
 440.4 KiB │ -2.9 KiB │  1.2 MiB │ -5.8 KiB │ ∆ classes2.dex                 
   3.6 MiB │ +1.5 KiB │  8.9 MiB │ +3.8 KiB │ ∆ classes.dex                  
   7.4 KiB │    -45 B │  7.2 KiB │    -45 B │ ∆ assets/dexopt/baseline.prof  
     807 B │     -6 B │    675 B │     -6 B │ ∆ assets/dexopt/baseline.profm 
───────────┼──────────┼──────────┼──────────┼────────────────────────────────
     4 MiB │ -1.4 KiB │ 10.1 MiB │   -2 KiB │ (total)
DEX
STRINGS:

   old   │ new   │ diff          
  ───────┼───────┼───────────────
   56825 │ 56836 │ +11 (+88 -77) 
  + _keyFactory
  + _this_viewModelLazy
  + ()TVM;
  + >()
  + La2/d0_a;
  + La2/d0;
  + La2/o_a;
  + Lcom/stripe/android/financialconnections/FinancialConnectionsSheetActivity_special__inlined_viewModelLazy_default_1;
  + Lcom/stripe/android/financialconnections/ui/FinancialConnectionsSheetNativeActivity_special__inlined_viewModelLazy_default_1;
  + Lcom/stripe/android/financialconnections/utils/MavericksExtensionsKt_argsOrNull_1;
  + Lcom/stripe/android/financialconnections/utils/MavericksExtensionsKt_viewModelLazy_1;
  + Lcom/stripe/android/financialconnections/utils/MavericksExtensionsKt_viewModelLazy_2;
  + Lcom/stripe/android/financialconnections/utils/MavericksExtensionsKt;
  + Lg8/c;
  + Lg8/c<
  + Lj0/a2_a;
  + Lj0/a2_b;
  + Lj0/a2_c;
  + Lj0/a2_d;
  + Lj0/a2_e;
  + Lj0/a2_f;
  + Lj0/a3<
  + Lj0/d3;
  + Lj0/f2_b;
  + Lj0/g2_a;
  + Lj0/l2_a;
  + Lj0/m2<
  + Lj0/o1<
  + Lj0/p1_a;
  + Lj0/q2_a;
  + Lj0/s2_a;
  + Lj0/s2_a<
  + Lj0/s2_b;
  + Lj0/t1<
  + Lj0/t2<
  + Lj0/w1<
  + Lj0/w2_b_a;
  + Lj0/x2_a;
  + Lj0/x2_b;
  + Lk1/c0_a_a;
  + Lk1/c0_a_b;
  + Lk1/c0_a;
  + Lk1/c0_a<
  + Lk1/c0_b;
  + Lk1/f0_a;
  + Lk1/f0_b;
  + Lk1/i0;
  + Lk1/t_a;
  + Llb/f_a;
  + Lm9/j_a;
  + Lm9/j;
  + Ln7/a;
  + Lr7/i_a;
  + Lr7/j<
  + Lr7/l<
  + Lr7/n_a;
  + Lr7/p_a;
  + Lr7/r_a;
  + Lr7/t_a;
  + Lr7/w;
  + Lt8/y;
  + Lu2/b0_a;
  + Lu2/x_a;
  + Lu2/x_b;
  + Lw1/b0;
  + TVM;>;TT;
  + VM:
  + [La2/r;
  + [Lj0/a2_d;
  + [Lj0/o1;
  + [Lj0/w1;
  + [Lk1/c0_a;
  + [Lk1/k;
  + [Lk1/n;
  + [Lk1/r;
  + [Lk2/z;
  + [Lr7/h;
  + [Lr7/l;
  + [Lu2/z;
  + argsOrNull
  + getArgs()Lcom/stripe/android/financialconnections/launcher/FinancialConnectionsSheetActivityArgs;
  + getArgs()Lcom/stripe/android/financialconnections/launcher/FinancialConnectionsSheetNativeActivityArgs;
  + getRead
  + setRead
  + viewModelLazy
  + viewModelLazy_default
  + ~~R8{backend:dex,compilation-mode:release,has-checksums:false,min-api:21,pg-map-id:c2c3ecf,r8-mode:compatibility,version:3.3.83}
  + Lorg/bouncycastle/jce/provider/a;
  
  - _this_viewModel
  - La2/c0_a;
  - La2/n_a;
  - Landroidx/activity/s;
  - Lcom/airbnb/mvrx/lifecycleAwareLazy;
  - Lcom/airbnb/mvrx/lifecycleAwareLazy<
  - Lcom/stripe/android/financialconnections/FinancialConnectionsSheetActivity_special__inlined_viewModel_default_1;
  - Lcom/stripe/android/financialconnections/FinancialConnectionsSheetState_Companion;
  - Lcom/stripe/android/financialconnections/ui/FinancialConnectionsSheetNativeActivity_special__inlined_viewModel_default_1;
  - Li4/v2;
  - Lj/i;
  - Lj0/e2_a;
  - Lj0/e2_b;
  - Lj0/k2_a;
  - Lj0/l2<
  - Lj0/n1_a;
  - Lj0/n1<
  - Lj0/p2_a;
  - Lj0/r2_a;
  - Lj0/r2_a<
  - Lj0/r2_b;
  - Lj0/r2<
  - Lj0/s1<
  - Lj0/u1<
  - Lj0/v2_a;
  - Lj0/v2_b_a;
  - Lj0/v2_b;
  - Lj0/z1_a;
  - Lj0/z1_b;
  - Lj0/z1_c;
  - Lj0/z1_d;
  - Lj0/z1_e;
  - Lj0/z1_f;
  - Lj0/z2<
  - Lj3/m;
  - Lk1/b0_a_a;
  - Lk1/b0_a_b;
  - Lk1/b0_a;
  - Lk1/b0_a<
  - Lk1/b0_b;
  - Lk1/e0_a;
  - Lk1/e0_b;
  - Lk1/s_a;
  - Lk2/a0;
  - Llb/h_a;
  - Llb/o;
  - Lm9/h_a;
  - Lorg/bouncycastle/jcajce/provider/asymmetric/a;
  - Lr7/g<
  - Lr7/h_a;
  - Lr7/k<
  - Lr7/m_a;
  - Lr7/o_a;
  - Lr7/q_a;
  - Lr7/s_a;
  - Lu2/c0_a;
  - Lu2/c0;
  - Lu2/y_a;
  - Lu2/y_b;
  - Lu8/m;
  - Lw2/m;
  - [La2/q;
  - [Lj0/n1;
  - [Lj0/v1;
  - [Lj0/z1_d;
  - [Lk1/b0_a;
  - [Lk1/j;
  - [Lk1/m;
  - [Lk1/q;
  - [Lk2/a0;
  - [Lr7/g;
  - [Lr7/k;
  - [Lu2/a0;
  - emptyArgs
  - isMainThread
  - owner.lifecycle.currentState
  - ~~R8{backend:dex,compilation-mode:release,has-checksums:false,min-api:21,pg-map-id:ce96802,r8-mode:compatibility,version:3.3.83}
  

TYPES:

   old   │ new   │ diff         
  ───────┼───────┼──────────────
   15303 │ 15305 │ +2 (+63 -61) 
  + La2/d0_a;
  + La2/d0;
  + La2/o_a;
  + Lcom/stripe/android/financialconnections/FinancialConnectionsSheetActivity_special__inlined_viewModelLazy_default_1;
  + Lcom/stripe/android/financialconnections/ui/FinancialConnectionsSheetNativeActivity_special__inlined_viewModelLazy_default_1;
  + Lcom/stripe/android/financialconnections/utils/MavericksExtensionsKt_argsOrNull_1;
  + Lcom/stripe/android/financialconnections/utils/MavericksExtensionsKt_viewModelLazy_1;
  + Lcom/stripe/android/financialconnections/utils/MavericksExtensionsKt_viewModelLazy_2;
  + Lcom/stripe/android/financialconnections/utils/MavericksExtensionsKt;
  + Lg8/c;
  + Lj0/a2_a;
  
...✂

@carlosmuvi-stripe carlosmuvi-stripe marked this pull request as ready for review December 1, 2022 00:20
@carlosmuvi-stripe
Copy link
Collaborator Author

@jaynewstrom-stripe Removed the nullable viewModel thanks to your suggestions!

Copy link
Collaborator

@jaynewstrom-stripe jaynewstrom-stripe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, happy to throw the green check if you don't think it's worth the test change.

import com.google.common.truth.Truth.assertThat
import org.junit.Test

internal class FinancialConnectionsSheetActivityTest {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we move this to a unit test (via robolectric)? Seems unnecessary to run a test like this on an emulator.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@carlosmuvi-stripe carlosmuvi-stripe enabled auto-merge (squash) December 1, 2022 19:56
@carlosmuvi-stripe carlosmuvi-stripe merged commit 70fcc5c into master Dec 1, 2022
@carlosmuvi-stripe carlosmuvi-stripe deleted the carlosmuvi/eagerly-finish-activity-no-args branch December 1, 2022 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants