Skip to content

Initialize global context before MainApp.onCreate() - #3907

Merged
tobiasKaminsky merged 1 commit into
masterfrom
ezaquarii/initialize-main-app-context-before-content-providers-start
Apr 17, 2019
Merged

Initialize global context before MainApp.onCreate()#3907
tobiasKaminsky merged 1 commit into
masterfrom
ezaquarii/initialize-main-app-context-before-content-providers-start

Conversation

@ezaquarii

Copy link
Copy Markdown
Collaborator

Signed-off-by: Chris Narkiewicz hello@ezaquarii.com

@ezaquarii

Copy link
Copy Markdown
CollaboratorAuthor

This could fix #3901 and possibly some other issues.

@mario

Copy link
Copy Markdown
Contributor

Can I ask why not a module and then provideContext which can be injected everywhere to get application-wide context?

* Temporary hack
*/
private static void initGlobalContext(Context context) {
mContext = context;

@ezaquariiezaquariiApr 15, 2019

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

getApplicationContext() is null, as the app context is set somewhere between Application.attachBaseContext() and Application.onCreate(). However, because there is only 1 instance of MainApp, there is no leak, so no worries.

Once the base context is attached, the context is basically functional, with some obscure exceptions around Looper and whatnot. Those should be fully initialized before ContentProvider.onCreate() kicks in.

@ezaquarii
ezaquariiforce-pushed the ezaquarii/initialize-main-app-context-before-content-providers-start branch 2 times, most recently from c252efc to 0c0e485CompareApril 15, 2019 21:30
@nextcloudnextcloud deleted a comment Apr 15, 2019
@ezaquarii

ezaquarii commented Apr 15, 2019

Copy link
Copy Markdown
CollaboratorAuthor

Can I ask why not a module and then provideContext which can be injected everywhere to get application-wide context?

It is already done.

Contextcontext(Applicationapplication) {

However, large portions of the code still call to MainApp.getAppContext() either directly or indirectly via other helpers and it will be a long march to fix this.

The best we can do in this situation is to provide them a valid reference. This can't be worse than NPE. :)

@ezaquarii
ezaquariiforce-pushed the ezaquarii/initialize-main-app-context-before-content-providers-start branch 2 times, most recently from db8c7c1 to 1fe7e2eCompareApril 15, 2019 21:50
@nextcloudnextcloud deleted a comment Apr 15, 2019
@ezaquarii
ezaquariiforce-pushed the ezaquarii/initialize-main-app-context-before-content-providers-start branch from 1fe7e2e to c78b434CompareApril 16, 2019 04:28
@codecov

codecovBot commented Apr 16, 2019

Copy link
Copy Markdown

Codecov Report

Merging #3907 into master will decrease coverage by 0.05%.
The diff coverage is 100%.

@@ Coverage Diff @@## master #3907 +/- ##
===========================================
- Coverage 6.57% 6.51% -0.06% 
Complexity 1 1 ===========================================
Files 320 320 Lines 30814 30816 +2 Branches 4403 4403 ===========================================
- Hits 2027 2009 -18 - Misses 28492 28511 +19 - Partials 295 296 +1
Impacted FilesCoverage ΔComplexity Δ
src/main/java/com/owncloud/android/MainApp.java45.7% <100%> (+0.02%)0 <0> (ø)⬇️
...services/firebase/NCFirebaseInstanceIDService.java0% <0%> (-87.5%)0% <0%> (ø)
.../services/firebase/NCFirebaseMessagingService.java0% <0%> (-7.15%)0% <0%> (ø)
...xtcloud/client/preferences/AppPreferencesImpl.java29.86% <0%> (-4.17%)0% <0%> (ø)
...in/java/com/owncloud/android/datamodel/OCFile.java57.99% <0%> (-1.37%)0% <0%> (ø)
...cloud/android/datamodel/ArbitraryDataProvider.java11.53% <0%> (-1.29%)0% <0%> (ø)
.../third_parties/daveKoeller/AlphanumComparator.java82.14% <0%> (-1.2%)0% <0%> (ø)

@codecov

codecovBot commented Apr 16, 2019

Copy link
Copy Markdown

Codecov Report

❗ No coverage uploaded for pull request base (master@02cbafe). Click here to learn what that means.
The diff coverage is 100%.

@@ Coverage Diff @@## master #3907 +/- ##
========================================
Coverage ? 6.58% Complexity ? 1 ========================================
Files ? 320 Lines ? 30878 Branches ? 4411 ========================================
Hits ? 2032 Misses ? 28550 Partials ? 296
Impacted FilesCoverage ΔComplexity Δ
src/main/java/com/owncloud/android/MainApp.java46.04% <100%> (ø)0 <0> (?)

@ezaquarii
ezaquariiforce-pushed the ezaquarii/initialize-main-app-context-before-content-providers-start branch 2 times, most recently from b7a08b6 to 9a9d1f0CompareApril 16, 2019 14:37
Initialize global context just after attaching base
context.
ContentProvider depend on MainApp.getAppContext(),
but ContentProvider.onCreate() is called *before*
Application.onCreate().
This hack should improve stability of ContentProviders
at process bring-up time until we address this
with a proper DI.
Signed-off-by: Chris Narkiewicz <hello@ezaquarii.com>
@ezaquarii
ezaquariiforce-pushed the ezaquarii/initialize-main-app-context-before-content-providers-start branch from 9a9d1f0 to 2a08cb4CompareApril 16, 2019 18:07
@nextcloud-android-bot

Copy link
Copy Markdown
Collaborator

Codacy

300

Lint

TypemasterPR
Warnings5757
Errors00

SpotBugs (new)

Warning TypeNumber
Bad practice Warnings25
Correctness Warnings80
Internationalization Warnings15
Malicious code vulnerability Warnings4
Multithreaded correctness Warnings9
Performance Warnings124
Security Warnings47
Dodgy code Warnings132
Total436

SpotBugs (master)

Warning TypeNumber
Bad practice Warnings25
Correctness Warnings80
Internationalization Warnings15
Malicious code vulnerability Warnings4
Multithreaded correctness Warnings9
Performance Warnings124
Security Warnings47
Dodgy code Warnings132
Total436

@tobiasKaminsky
tobiasKaminsky merged commit 28f80cd into masterApr 17, 2019
@tobiasKaminsky
tobiasKaminsky deleted the ezaquarii/initialize-main-app-context-before-content-providers-start branch April 17, 2019 11:57
nextcloud-android-bot pushed a commit that referenced this pull request May 2, 2019
71f3370 Drone: update FindBugs results to reflect reduced error/warning count [skip ci]
0f50088 Merge pull request #3916 from jmue/cleanup/string_compare
7bb07a0 Update wording for store text (#3903)
4c72f3f Merge pull request #3923 from nextcloud/editOnShareLink
81ed32f allow edit on link share on a file
7d01ff7 - fix IT tests - re-organized build.gradle a bit
a9afff7 Merge pull request #3929 from nextcloud/dependabot/gradle/org.powermock-powermock-core-2.0.2
71c6d99 bump all other powermock parts to 2.0.2
0421d26 Bump powermock-core from 2.0.0 to 2.0.2
1d28d1b Update with new wording from frontpage
de3d3de Merge pull request #3924 from nextcloud/warnOnStable
69dd243 warn for wrong library branch on stable
2379b49 Merge pull request #3921 from nextcloud/dependabot/gradle/com.android.tools.build-gradle-3.4.0
c2730e8 Bump gradle from 3.3.2 to 3.4.0
b69169f unify empty string compare
8bb654b fix test & review comments
d8b9d10 unify empty string compare
2f23a30 remove unused variable (#3920)
92df32e Merge pull request #3918 from nextcloud/codacyCleanup
091c003 Drone: update FindBugs results to reflect reduced error/warning count [skip ci]
15ec487 Drone: update FindBugs results to reflect reduced error/warning count [skip ci]
da3c497 Merge commit '334155e707975e740a0a13b33d9b7bbedf40e9bc'
b22f517 Merge pull request #3915 from jmue/cleanup/ocfile
c7c6a98 use constant
e6604c8 use constant
caa2a91 Merge pull request #3912 from nextcloud/libraryUsage
334155e some cleanups
28f80cd Merge pull request #3907 from nextcloud/ezaquarii/initialize-main-app-context-before-content-providers-start
2afb934 Drone: update FindBugs results to reflect reduced error/warning count [skip ci]
8be1157 Merge pull request #3899 from jmue/cleanup/uriutils
52020a9 direct usage of library project
4062d40 Merge pull request #3908 from nextcloud/missingInjectable
2a08cb4 Initialize global context before MainApp.onCreate()
f02f482 Missing Injectable lead to half-stored account
02cbafe Merge pull request #3765 from nextcloud/showSharees
dc7a2c0 show sharees
03e04e5 Merge pull request #3889 from nextcloud/checkUserId
d65e94b Update wording for store text
585a3d0 Drone: update FindBugs results to reflect reduced error/warning count [skip ci]
533debf - removed useNextcloudUserAgent -> we now use this everytime - removed any oAuth and saml stuff, as we rely on weblogin flow
d544fa5 Merge pull request #3897 from nextcloud/ezaquarii/move-current-account-getter-to-user-account-manager
5b4ffba Merge pull request #3900 from nextcloud/codacyOnMaster
2757244 remove unused code
428a8bb show codacy count only on master
6b44074 Drone: update FindBugs results to reflect reduced error/warning count [skip ci]
c62e89b Merge commit '442394f949ff2de86bcccae172d2e9d997ab2ef8'
442394f Migrate current account getter from AccountUtils to UserAccountManager
182b65e daily dev 20190414
Sign up for freeto 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.

5 participants

@ezaquarii@mario@nextcloud-android-bot@AndyScherzinger@tobiasKaminsky