From daa3e1e3b04493906ef5b41ad62c0f7f8686cd07 Mon Sep 17 00:00:00 2001 From: Sergio Villar Senin Date: Tue, 19 Nov 2024 13:01:32 +0100 Subject: [PATCH 1/4] Use StrictMode for debug builds StrictMode is most commonly used to catch accidental disk or network access on the application's main thread, where UI operations are received and animations take place. Keeping disk and network operations off the main thread makes for much smoother, more responsive applications[1]. [1] https://developer.android.com/reference/android/os/StrictMode --- .../igalia/wolvic/VRBrowserApplication.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/app/src/common/shared/com/igalia/wolvic/VRBrowserApplication.java b/app/src/common/shared/com/igalia/wolvic/VRBrowserApplication.java index 2c183ae4aa..c21a4d658a 100644 --- a/app/src/common/shared/com/igalia/wolvic/VRBrowserApplication.java +++ b/app/src/common/shared/com/igalia/wolvic/VRBrowserApplication.java @@ -9,6 +9,7 @@ import android.app.Application; import android.content.Context; import android.content.res.Configuration; +import android.os.StrictMode; import androidx.annotation.NonNull; @@ -91,6 +92,23 @@ public void onConfigurationChanged(Configuration newConfig) { super.onConfigurationChanged(newConfig); } + @Override + public void onCreate() { + super.onCreate(); + // TODO: eventually add .penaltyDeath() to the policies once we have fixed all the issues. + if (BuildConfig.DEBUG) { + StrictMode.setThreadPolicy(new StrictMode.ThreadPolicy.Builder() + .detectAll() + .penaltyLog() + .build()); + + StrictMode.setVmPolicy(new StrictMode.VmPolicy.Builder() + .detectAll() + .penaltyLog() + .build()); + } + } + public Services getServices() { return mServices; } From a086c2ac31253975df808484ae0dc8c888adb60a Mon Sep 17 00:00:00 2001 From: Sergio Villar Senin Date: Tue, 19 Nov 2024 13:34:45 +0100 Subject: [PATCH 2/4] Do not write SharedPreferences to disk in the main thread SettingsStore used to call editor.commit() to apply the value of a setting and write it to disk. That should not be done in the main thread though as it's a blocking operation. It's better to use apply() which atomically modifies the SharedPreferences object in memory and then uses a background thread to write it to disk. It's safe to replace commit() by apply() as long as the return value is not used, which is the case. This fixes reports like: D StrictMode policy violation; ~duration=3 ms: android.os.strictmode.DiskWriteViolation at android.os.StrictMode$AndroidBlockGuardPolicy.onWriteToDisk(StrictMode.java:1615) at libcore.io.BlockGuardOs.write(BlockGuardOs.java:446) at libcore.io.ForwardingOs.write(ForwardingOs.java:951) at libcore.io.IoBridge.write(IoBridge.java:649) at java.io.FileOutputStream.write(FileOutputStream.java:401) at com.android.internal.util.FastXmlSerializer.flushBytes(FastXmlSerializer.java:253) at com.android.internal.util.FastXmlSerializer.flush(FastXmlSerializer.java:274) at com.android.internal.util.FastXmlSerializer.endDocument(FastXmlSerializer.java:219) at com.android.internal.util.XmlSerializerWrapper.endDocument(XmlSerializerWrapper.java:68) at com.android.internal.util.XmlUtils.writeMapXml(XmlUtils.java:408) at android.app.SharedPreferencesImpl.writeToFile(SharedPreferencesImpl.java:803) at android.app.SharedPreferencesImpl.access$900(SharedPreferencesImpl.java:59) at android.app.SharedPreferencesImpl$2.run(SharedPreferencesImpl.java:672) at android.app.SharedPreferencesImpl.enqueueDiskWrite(SharedPreferencesImpl.java:691) at android.app.SharedPreferencesImpl.access$100(SharedPreferencesImpl.java:59) at android.app.SharedPreferencesImpl$EditorImpl.commit(SharedPreferencesImpl.java:604) at com.igalia.wolvic.browser.SettingsStore.resetCrashRestartCount(SettingsStore.java:766) at com.igalia.wolvic.VRBrowserActivity.lambda$onCreate$0$com-igalia-wolvic-VRBrowserActivity(VRBrowserActivity.java:296) at com.igalia.wolvic.VRBrowserActivity$$ExternalSyntheticLambda3.run(D8$$SyntheticClass:0) at android.os.Handler.handleCallback(Handler.java:938) at android.os.Handler.dispatchMessage(Handler.java:99) at android.os.Looper.loopOnce(Looper.java:214) at android.os.Looper.loop(Looper.java:304) at android.app.ActivityThread.main(ActivityThread.java:7918) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1010) --- .../igalia/wolvic/browser/SettingsStore.java | 140 +++++++++--------- 1 file changed, 70 insertions(+), 70 deletions(-) diff --git a/app/src/common/shared/com/igalia/wolvic/browser/SettingsStore.java b/app/src/common/shared/com/igalia/wolvic/browser/SettingsStore.java index cb609ba28d..6dbba2acc0 100644 --- a/app/src/common/shared/com/igalia/wolvic/browser/SettingsStore.java +++ b/app/src/common/shared/com/igalia/wolvic/browser/SettingsStore.java @@ -199,7 +199,7 @@ private void update() { String json = response.getBody().string(StandardCharsets.UTF_8); SharedPreferences.Editor editor = mPrefs.edit(); editor.putString(mContext.getString(R.string.settings_key_remote_props), json); - editor.commit(); + editor.apply(); mSettingsViewModel.setProps(json); } @@ -217,7 +217,7 @@ public boolean isCrashReportingEnabled() { public void setCrashReportingEnabled(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_crash), isEnabled); - editor.commit(); + editor.apply(); } public boolean isTelemetryEnabled() { @@ -234,7 +234,7 @@ public boolean isTelemetryEnabled() { public void setTelemetryEnabled(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_telemetry), isEnabled); - editor.commit(); + editor.apply(); // We send after enabling in case of opting-in if (isEnabled) { @@ -258,13 +258,13 @@ public boolean isTelemetryPingUpdateSent() { public void setTelemetryPingUpdateSent(boolean isSent) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_telemetry_status_update_sent), isSent); - editor.commit(); + editor.apply(); } public void setGeolocationData(String aGeolocationData) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putString(mContext.getString(R.string.settings_key_geolocation_data), aGeolocationData); - editor.commit(); + editor.apply(); } public String getGeolocationData() { @@ -279,7 +279,7 @@ public boolean isRemoteDebuggingEnabled() { public void setRemoteDebuggingEnabled(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_remote_debugging), isEnabled); - editor.commit(); + editor.apply(); } @@ -295,7 +295,7 @@ public boolean isDrmContentPlaybackSet() { public void setDrmContentPlaybackEnabled(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_drm_playback), isEnabled); - editor.commit(); + editor.apply(); mSettingsViewModel.setIsDrmEnabled(isEnabled); } @@ -308,7 +308,7 @@ public int getTrackingProtectionLevel() { public void setTrackingProtectionLevel(int level) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putInt(mContext.getString(R.string.settings_key_tracking_protection_level), level); - editor.commit(); + editor.apply(); mSettingsViewModel.setIsTrackingProtectionEnabled(level != WContentBlocking.EtpLevel.NONE); } @@ -321,7 +321,7 @@ public boolean isSystemRootCAEnabled() { public void setSystemRootCAEnabled(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_system_root_ca), isEnabled); - editor.commit(); + editor.apply(); } public static boolean shouldStartWithPassthrougEnabled() { @@ -336,7 +336,7 @@ public boolean isStartWithPassthroughEnabled() { public void setStartWithPassthroughEnabled(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_start_with_passthrough), isEnabled); - editor.commit(); + editor.apply(); } public boolean isLatinAutoCompleteEnabled() { @@ -347,7 +347,7 @@ public boolean isLatinAutoCompleteEnabled() { public void setLatinAutoComplete(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_latin_auto_complete), isEnabled); - editor.commit(); + editor.apply(); } public boolean isHeadLockEnabled() { @@ -358,7 +358,7 @@ public boolean isHeadLockEnabled() { public void setHeadLockEnabled(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_head_lock), isEnabled); - editor.commit(); + editor.apply(); } public boolean isWindowMovementEnabled() { @@ -369,7 +369,7 @@ public boolean isWindowMovementEnabled() { public void setWindowMovementEnabled(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_window_movement), isEnabled); - editor.commit(); + editor.apply(); mSettingsViewModel.setWindowMovementEnabled(isEnabled); } @@ -382,7 +382,7 @@ public boolean isEnvironmentOverrideEnabled() { public void setEnvironmentOverrideEnabled(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_environment_override), isEnabled); - editor.commit(); + editor.apply(); } public boolean isUIHardwareAccelerationEnabled() { @@ -401,7 +401,7 @@ public boolean isUIHardwareAccelerationEnabled() { public void setUIHardwareAccelerationEnabled(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_ui_hardware_acceleration), isEnabled); - editor.commit(); + editor.apply(); } public boolean isPerformanceMonitorEnabled() { @@ -412,7 +412,7 @@ public boolean isPerformanceMonitorEnabled() { public void setPerformanceMonitorEnabled(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_performance_monitor), isEnabled); - editor.commit(); + editor.apply(); } @FloatRange(from = 0, to = 1) @@ -423,7 +423,7 @@ public float getWindowDistance() { public void setWindowDistance(@FloatRange(from = 0, to = 1) float distance) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putFloat(mContext.getString(R.string.settings_key_window_distance), distance); - editor.commit(); + editor.apply(); } public int getUaMode() { @@ -439,7 +439,7 @@ public void setUaMode(int mode) { } SharedPreferences.Editor editor = mPrefs.edit(); editor.putInt(mContext.getString(R.string.settings_key_user_agent_version), checkedMode); - editor.commit(); + editor.apply(); } public int getInputMode() { @@ -450,7 +450,7 @@ public int getInputMode() { public void setInputMode(int aTouchMode) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putInt(mContext.getString(R.string.settings_key_input_mode), aTouchMode); - editor.commit(); + editor.apply(); } public String getHomepage() { @@ -462,7 +462,7 @@ public String getHomepage() { public void setHomepage(String aHomepage) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putString(mContext.getString(R.string.settings_key_homepage), aHomepage); - editor.commit(); + editor.apply(); } public float getDisplayDensity() { @@ -473,7 +473,7 @@ public float getDisplayDensity() { public void setDisplayDensity(float aDensity) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putFloat(mContext.getString(R.string.settings_key_display_density), aDensity); - editor.commit(); + editor.apply(); } public int getWindowWidth() { @@ -496,7 +496,7 @@ public String getDeviceName() { public void setDeviceName(String aDeviceName) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putString(mContext.getString(R.string.settings_key_device_name), aDeviceName); - editor.commit(); + editor.apply(); } public int getDisplayDpi() { @@ -511,7 +511,7 @@ public void setDisplayDpi(int aDpi) { } SharedPreferences.Editor editor = mPrefs.edit(); editor.putInt(mContext.getString(R.string.settings_key_display_dpi), aDpi); - editor.commit(); + editor.apply(); } public int getMaxWindowWidth() { @@ -529,7 +529,7 @@ public String getEnvironment() { public void setEnvironment(String aEnv) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putString(mContext.getString(R.string.settings_key_env), aEnv); - editor.commit(); + editor.apply(); } public int getPointerColor() { @@ -540,7 +540,7 @@ public int getPointerColor() { public void setPointerColor(int color) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putInt(mContext.getString(R.string.settings_key_pointer_color), color); - editor.commit(); + editor.apply(); } public int getScrollDirection() { @@ -554,7 +554,7 @@ public void setScrollDirection(int aScrollDirection) { mCachedScrollDirection = aScrollDirection; SharedPreferences.Editor editor = mPrefs.edit(); editor.putInt(mContext.getString(R.string.settings_key_scroll_direction), aScrollDirection); - editor.commit(); + editor.apply(); } @@ -569,7 +569,7 @@ public int getMSAALevel() { public void setMSAALevel(int level) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putInt(mContext.getString(R.string.settings_key_msaa), level); - editor.commit(); + editor.apply(); } public boolean getLayersEnabled() { @@ -592,7 +592,7 @@ public boolean isAudioEnabled() { public void setAudioEnabled(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_audio), isEnabled); - editor.commit(); + editor.apply(); } public String getVoiceSearchService() { @@ -603,7 +603,7 @@ public String getVoiceSearchService() { public void setVoiceSearchService(String service) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putString(mContext.getString(R.string.settings_key_voice_search_service), service); - editor.commit(); + editor.apply(); } public String getVoiceSearchLocale() { @@ -615,7 +615,7 @@ public String getVoiceSearchLocale() { public void setVoiceSearchLocale(String language) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putString(mContext.getString(R.string.settings_key_voice_search_language), language); - editor.commit(); + editor.apply(); } public String getDisplayLocale() { @@ -627,7 +627,7 @@ public String getDisplayLocale() { public void setDisplayLocale(String language) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putString(mContext.getString(R.string.settings_key_display_language), language); - editor.commit(); + editor.apply(); } public ArrayList getContentLocales() { @@ -654,7 +654,7 @@ public void setContentLocales(List languages) { JSONArray json = new JSONArray(languages); SharedPreferences.Editor editor = mPrefs.edit(); editor.putString(mContext.getString(R.string.settings_key_content_languages), json.toString()); - editor.commit(); + editor.apply(); } public float getCylinderDensity() { @@ -664,7 +664,7 @@ public float getCylinderDensity() { public void setCylinderDensity(float aDensity) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putFloat(mContext.getString(R.string.settings_key_cylinder_density), aDensity); - editor.commit(); + editor.apply(); } public boolean isCurvedModeEnabled() { @@ -678,7 +678,7 @@ public float getHapticPulseDuration() { public void setHapticPulseDuration(float aPulseDuration) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putFloat(mContext.getString(R.string.settings_key_haptic_pulse_duration), aPulseDuration); - editor.commit(); + editor.apply(); } public float getHapticPulseIntensity() { @@ -688,13 +688,13 @@ public float getHapticPulseIntensity() { public void setHapticPulseIntensity(float aPulseIntensity) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putFloat(mContext.getString(R.string.settings_key_haptic_pulse_intensity), aPulseIntensity); - editor.commit(); + editor.apply(); } public void setHapticFeedbackEnabled(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_haptic_feedback_enabled), isEnabled); - editor.commit(); + editor.apply(); } public boolean isHapticFeedbackEnabled() { @@ -704,7 +704,7 @@ public boolean isHapticFeedbackEnabled() { public void setPointerMode(@WidgetManagerDelegate.PointerMode int pointerMode) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putInt(mContext.getString(R.string.settings_key_pointer_mode), pointerMode); - editor.commit(); + editor.apply(); } public @WidgetManagerDelegate.PointerMode int getPointerMode() { @@ -719,13 +719,13 @@ public boolean isCenterWindows() { public void setCenterWindows(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_center_windows), isEnabled); - editor.commit(); + editor.apply(); } public void setSelectedKeyboard(Locale aLocale) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putString(mContext.getString(R.string.settings_key_keyboard_locale), aLocale.toLanguageTag()); - editor.commit(); + editor.apply(); } public Locale getKeyboardLocale() { @@ -745,7 +745,7 @@ public synchronized long getCrashRestartCount() { SharedPreferences.Editor editor = mPrefs.edit(); editor.putLong(mContext.getString(R.string.settings_key_crash_restart_count), count); editor.putLong(mContext.getString(R.string.settings_key_crash_restart_count_timestamp), -1); - editor.commit(); + editor.apply(); } } return count; @@ -757,13 +757,13 @@ public synchronized void incrementCrashRestartCount() { count++; editor.putLong(mContext.getString(R.string.settings_key_crash_restart_count), count); editor.putLong(mContext.getString(R.string.settings_key_crash_restart_count_timestamp), System.currentTimeMillis()); - editor.commit(); + editor.apply(); } public synchronized void resetCrashRestartCount() { SharedPreferences.Editor editor = mPrefs.edit(); editor.putLong(mContext.getString(R.string.settings_key_crash_restart_count), 0); - editor.commit(); + editor.apply(); } public boolean isSpeechDataCollectionEnabled() { @@ -774,7 +774,7 @@ public boolean isSpeechDataCollectionEnabled() { public void setSpeechDataCollectionEnabled(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_speech_data_collection), isEnabled); - editor.commit(); + editor.apply(); } public boolean isNotificationsEnabled() { @@ -785,7 +785,7 @@ public boolean isNotificationsEnabled() { public void setNotificationsEnabled(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_notifications), isEnabled); - editor.commit(); + editor.apply(); } public boolean isSpeechDataCollectionReviewed() { @@ -796,7 +796,7 @@ public boolean isSpeechDataCollectionReviewed() { public void setSpeechDataCollectionReviewed(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_speech_data_collection_reviewed), isEnabled); - editor.commit(); + editor.apply(); } public boolean isDebugLoggingEnabled() { @@ -806,7 +806,7 @@ public boolean isDebugLoggingEnabled() { public void setDebugLoggingEnabled(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_debug_logging), isEnabled); - editor.commit(); + editor.apply(); } public boolean isAutoplayEnabled() { @@ -816,13 +816,13 @@ public boolean isAutoplayEnabled() { public void setAutoplayEnabled(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_autoplay), isEnabled); - editor.commit(); + editor.apply(); } public void setPid(int aPid) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putInt(mContext.getString(R.string.settings_key_pid), aPid); - editor.commit(); + editor.apply(); } public int getPid() { @@ -836,7 +836,7 @@ public boolean isPopUpsBlockingEnabled() { public void setPopUpsBlockingEnabled(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_pop_up_blocking), isEnabled); - editor.commit(); + editor.apply(); mSettingsViewModel.setIsPopUpBlockingEnabled(isEnabled); } @@ -848,7 +848,7 @@ public boolean isWebXREnabled() { public void setWebXREnabled(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_webxr), isEnabled); - editor.commit(); + editor.apply(); mSettingsViewModel.setIsWebXREnabled(isEnabled); } @@ -856,7 +856,7 @@ public void setWebXREnabled(boolean isEnabled) { public void setWhatsNewDisplayed(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_whats_new_displayed), isEnabled); - editor.commit(); + editor.apply(); } public boolean isWhatsNewDisplayed() { @@ -874,7 +874,7 @@ public void setFxALastSync(@NonNull String email, long timestamp) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putString(mContext.getString(R.string.settings_key_fxa_last_sync), jsonObject.toString()); - editor.commit(); + editor.apply(); } catch (Exception e) { Log.d(LOGTAG, e.getMessage()); @@ -906,7 +906,7 @@ public long getFxALastSync(@NonNull String email) { public void setRestoreTabsEnabled(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_restore_tabs), isEnabled); - editor.commit(); + editor.apply(); } public boolean isRestoreTabsEnabled() { @@ -916,7 +916,7 @@ public boolean isRestoreTabsEnabled() { public void setBypassCacheOnReload(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_bypass_cache_on_reload), isEnabled); - editor.commit(); + editor.apply(); } public boolean isBypassCacheOnReloadEnabled() { @@ -926,7 +926,7 @@ public boolean isBypassCacheOnReloadEnabled() { public void setDownloadsSortingOrder(@SortingContextMenuWidget.Order int order) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putInt(mContext.getString(R.string.settings_key_downloads_sorting_order), order); - editor.commit(); + editor.apply(); } public @Storage int getDownloadsSortingOrder() { @@ -936,7 +936,7 @@ public void setDownloadsSortingOrder(@SortingContextMenuWidget.Order int order) public void setRemotePropsVersionName(String versionName) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putString(mContext.getString(R.string.settings_key_remote_props_version_name), versionName); - editor.commit(); + editor.apply(); mSettingsViewModel.setPropsVersionName(versionName); } @@ -948,7 +948,7 @@ public String getRemotePropsVersionName() { public void setAutocompleteEnabled(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_autocomplete), isEnabled); - editor.commit(); + editor.apply(); } public boolean isAutocompleteEnabled() { @@ -966,13 +966,13 @@ public void setSearchEngineId(@Nullable String searchEngineId) { } else { editor.remove(mContext.getString(R.string.settings_key_search_engine_id)); } - editor.commit(); + editor.apply(); } public void setWebGLOutOfProcess(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_webgl_out_of_process), isEnabled); - editor.commit(); + editor.apply(); } public boolean isWebGLOutOfProcess() { @@ -982,7 +982,7 @@ public boolean isWebGLOutOfProcess() { public void setLocalAddonAllowed(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_local_addon_allowed), isEnabled); - editor.commit(); + editor.apply(); } public boolean isLocalAddonAllowed() { @@ -996,7 +996,7 @@ public int getPrefsLastResetVersionCode() { public void setPrefsLastResetVersionCode(int versionCode) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putInt(mContext.getString(R.string.settings_key_prefs_last_reset_version_code), versionCode); - editor.commit(); + editor.apply(); } @Nullable @@ -1018,13 +1018,13 @@ public Map getRemoteProperties() { public void setRemoteProperties(@Nullable String json) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putString(mContext.getString(R.string.settings_key_remote_props), json); - editor.commit(); + editor.apply(); } public void recordPasswordsEncryptionKeyGenerated() { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_passwords_encryption_key_generated), true); - editor.commit(); + editor.apply(); } public boolean isPasswordsEncryptionKeyGenerated() { @@ -1034,7 +1034,7 @@ public boolean isPasswordsEncryptionKeyGenerated() { public void setAutoFillEnabled(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_autofill_enabled), isEnabled); - editor.commit(); + editor.apply(); } public boolean isAutoFillEnabled() { @@ -1044,7 +1044,7 @@ public boolean isAutoFillEnabled() { public void setLoginAutocompleteEnabled(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_login_autocomplete_enabled), isEnabled); - editor.commit(); + editor.apply(); } public boolean isLoginAutocompleteEnabled() { @@ -1054,7 +1054,7 @@ public boolean isLoginAutocompleteEnabled() { public void setLoginSyncEnabled(boolean isEnabled) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_login_sync_enabled), isEnabled); - editor.commit(); + editor.apply(); } public boolean isLoginSyncEnabled() { @@ -1068,7 +1068,7 @@ public void setTabAfterRestore(@Nullable String uri) { } else { editor.remove(mContext.getString(R.string.settings_key_tab_after_restore)); } - editor.commit(); + editor.apply(); } public String getTabAfterRestore() { @@ -1078,7 +1078,7 @@ public String getTabAfterRestore() { public void setTermsServiceAccepted(boolean isAccepted) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_terms_service_accepted), isAccepted); - editor.commit(); + editor.apply(); } public boolean isTermsServiceAccepted() { @@ -1088,7 +1088,7 @@ public boolean isTermsServiceAccepted() { public void setPrivacyPolicyAccepted(boolean isAccepted) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putBoolean(mContext.getString(R.string.settings_key_privacy_policy_accepted), isAccepted); - editor.commit(); + editor.apply(); } public boolean isPrivacyPolicyAccepted() { @@ -1098,7 +1098,7 @@ public boolean isPrivacyPolicyAccepted() { public void setWebAppsData(String aWebAppsData) { SharedPreferences.Editor editor = mPrefs.edit(); editor.putString(mContext.getString(R.string.settings_key_web_apps_data), aWebAppsData); - editor.commit(); + editor.apply(); } public String getWebAppsData() { From d98d576cf1160144344d573f42c5cfb6779faf06 Mon Sep 17 00:00:00 2001 From: Sergio Villar Senin Date: Tue, 19 Nov 2024 14:00:14 +0100 Subject: [PATCH 3/4] Do not query the list of downloads in the main thread Quering the list of downloads might involve SQL queries that can write to a file. That's a blocking operation that should not happen in the main thread. Instead get the diskIO executor in the constructor and use it to run the query. Note that notifications to listeners should still happen in the main thread. This fixes this report: D StrictMode policy violation; ~duration=38 ms: android.os.strictmode.DiskWriteViolation at android.os.StrictMode$AndroidBlockGuardPolicy.onWriteToDisk(StrictMode.java:1615) at android.database.sqlite.SQLiteConnection.applyBlockGuardPolicy(SQLiteConnection.java:1201) at android.database.sqlite.SQLiteConnection.executeForString(SQLiteConnection.java:794) at android.database.sqlite.SQLiteConnection.setJournalMode(SQLiteConnection.java:407) at android.database.sqlite.SQLiteConnection.setWalModeFromConfiguration(SQLiteConnection.java:347) at android.database.sqlite.SQLiteConnection.open(SQLiteConnection.java:260) at android.database.sqlite.SQLiteConnection.open(SQLiteConnection.java:205) at android.database.sqlite.SQLiteConnectionPool.openConnectionLocked(SQLiteConnectionPool.java:505) at android.database.sqlite.SQLiteConnectionPool.tryAcquireNonPrimaryConnectionLocked(SQLiteConnectionPool.java:989) at android.database.sqlite.SQLiteConnectionPool.waitForConnection(SQLiteConnectionPool.java:695) at android.database.sqlite.SQLiteConnectionPool.acquireConnection(SQLiteConnectionPool.java:380) at android.database.sqlite.SQLiteSession.acquireConnection(SQLiteSession.java:896) at android.database.sqlite.SQLiteSession.prepare(SQLiteSession.java:588) at android.database.sqlite.SQLiteDatabase.validateSql(SQLiteDatabase.java:1945) at android.database.sqlite.SQLiteQueryBuilder.query(SQLiteQueryBuilder.java:573) at android.database.sqlite.SQLiteQueryBuilder.query(SQLiteQueryBuilder.java:463) at com.android.providers.downloads.DownloadProvider.query(DownloadProvider.java:1270) at android.content.ContentProvider.query(ContentProvider.java:1408) at android.content.ContentProvider.query(ContentProvider.java:1504) at android.content.ContentProvider$Transport.query(ContentProvider.java:272) at android.content.ContentProviderNative.onTransact(ContentProviderNative.java:107) at android.os.Binder.execTransactInternal(Binder.java:1179) at android.os.Binder.execTransact(Binder.java:1143) at android.os.StrictMode.readAndHandleBinderCallViolations(StrictMode.java:2497) at android.os.Parcel.readExceptionCode(Parcel.java:2371) at android.database.DatabaseUtils.readExceptionFromParcel(DatabaseUtils.java:139) at android.content.ContentProviderProxy.query(ContentProviderNative.java:481) at android.content.ContentResolver.query(ContentResolver.java:1219) at android.content.ContentResolver.query(ContentResolver.java:1151) at android.content.ContentResolver.query(ContentResolver.java:1107) at android.app.DownloadManager$Query.runQuery(DownloadManager.java:1019) at android.app.DownloadManager.query(DownloadManager.java:1165) at android.app.DownloadManager.query(DownloadManager.java:1160) at com.igalia.wolvic.downloads.DownloadsManager.getDownloads(DownloadsManager.java:281) at com.igalia.wolvic.downloads.DownloadsManager.notifyDownloadsUpdate(DownloadsManager.java:313) at com.igalia.wolvic.downloads.DownloadsManager.$r8$lambda$0HiP7vssHY38l68BiKxs3V29Nug(Unknown Source:0) at com.igalia.wolvic.downloads.DownloadsManager$$ExternalSyntheticLambda0.run(D8$$SyntheticClass:0) at android.os.Handler.handleCallback(Handler.java:938) at android.os.Handler.dispatchMessage(Handler.java:99) at android.os.Looper.loopOnce(Looper.java:214) at android.os.Looper.loop(Looper.java:304) at android.app.ActivityThread.main(ActivityThread.java:7918) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1010) --- .../wolvic/downloads/DownloadsManager.java | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/app/src/common/shared/com/igalia/wolvic/downloads/DownloadsManager.java b/app/src/common/shared/com/igalia/wolvic/downloads/DownloadsManager.java index 04330122af..1884967512 100644 --- a/app/src/common/shared/com/igalia/wolvic/downloads/DownloadsManager.java +++ b/app/src/common/shared/com/igalia/wolvic/downloads/DownloadsManager.java @@ -19,6 +19,7 @@ import androidx.annotation.Nullable; import com.igalia.wolvic.R; +import com.igalia.wolvic.VRBrowserApplication; import com.igalia.wolvic.utils.StringUtils; import com.igalia.wolvic.utils.UrlUtils; @@ -29,6 +30,7 @@ import java.io.OutputStream; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.Executor; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; @@ -51,6 +53,7 @@ default void onDownloadError(@NonNull String error, @NonNull String file) {} private DownloadManager mDownloadManager; private ScheduledThreadPoolExecutor mExecutor; private ScheduledFuture mFuture; + private Executor mDiskExecutor; public DownloadsManager(@NonNull Context context) { mMainHandler = new Handler(Looper.getMainLooper()); @@ -58,6 +61,7 @@ public DownloadsManager(@NonNull Context context) { mListeners = new ArrayList<>(); mDownloadManager = (DownloadManager) mContext.getSystemService(Context.DOWNLOAD_SERVICE); mExecutor = new ScheduledThreadPoolExecutor(1); + mDiskExecutor = ((VRBrowserApplication) mContext.getApplicationContext()).getExecutors().diskIO(); } public void init() { @@ -310,13 +314,17 @@ public void onReceive(Context context, Intent intent) { }; private void notifyDownloadsUpdate() { - List downloads = getDownloads(); - int filter = Download.RUNNING | Download.PAUSED | Download.PENDING; - boolean activeDownloads = downloads.stream().filter(d -> (d.getStatus() & filter) != 0).count() > 0; - mListeners.forEach(listener -> listener.onDownloadsUpdate(downloads)); - if (!activeDownloads) { - stopUpdates(); - } + mDiskExecutor.execute(() -> { + List downloads = getDownloads(); + int filter = Download.RUNNING | Download.PAUSED | Download.PENDING; + boolean activeDownloads = downloads.stream().filter(d -> (d.getStatus() & filter) != 0).count() > 0; + mMainHandler.post(() -> { + mListeners.forEach(listener -> listener.onDownloadsUpdate(downloads)); + }); + if (!activeDownloads) { + stopUpdates(); + } + }); } private void notifyDownloadCompleted(@NonNull long downloadId) { From 13eb3149f027b0ffa66a782db5c098ae47323780 Mon Sep 17 00:00:00 2001 From: Sergio Villar Senin Date: Tue, 19 Nov 2024 14:11:31 +0100 Subject: [PATCH 4/4] Do not remove downloads on main thread Removing a download involves on disk reading/writing operations. Those are blocking operations that should not happen in the main thread. Use the recently added disk executor to carry on the task. This fixes this report: D StrictMode policy violation; ~duration=102 ms: android.os.strictmode.DiskReadViolation at android.os.StrictMode$AndroidBlockGuardPolicy.onReadFromDisk(StrictMode.java:1659) at libcore.io.BlockGuardOs.access(BlockGuardOs.java:74) at libcore.io.ForwardingOs.access(ForwardingOs.java:131) at android.app.ActivityThread$AndroidOs.access(ActivityThread.java:7795) at java.io.UnixFileSystem.checkAccess(UnixFileSystem.java:281) at java.io.File.exists(File.java:813) at android.app.ContextImpl.ensurePrivateDirExists(ContextImpl.java:751) at android.app.ContextImpl.ensurePrivateDirExists(ContextImpl.java:742) at android.app.ContextImpl.getFilesDir(ContextImpl.java:787) at com.android.providers.downloads.Helpers.isFilenameValid(Helpers.java:665) at com.android.providers.downloads.Helpers.isFilenameValid(Helpers.java:527) at com.android.providers.downloads.DownloadProvider.delete(DownloadProvider.java:1629) at android.content.ContentProvider.delete(ContentProvider.java:1785) at android.content.ContentProvider$Transport.delete(ContentProvider.java:430) at android.content.ContentProviderNative.onTransact(ContentProviderNative.java:215) at android.os.Binder.execTransactInternal(Binder.java:1179) at android.os.Binder.execTransact(Binder.java:1143) at android.os.StrictMode.readAndHandleBinderCallViolations(StrictMode.java:2497) at android.os.Parcel.readExceptionCode(Parcel.java:2371) at android.database.DatabaseUtils.readExceptionFromParcel(DatabaseUtils.java:139) at android.content.ContentProviderProxy.delete(ContentProviderNative.java:629) at android.content.ContentResolver.delete(ContentResolver.java:2333) at android.content.ContentResolver.delete(ContentResolver.java:2299) at android.app.DownloadManager.markRowDeleted(DownloadManager.java:1138) at android.app.DownloadManager.remove(DownloadManager.java:1150) at com.igalia.wolvic.downloads.DownloadsManager.removeDownload(DownloadsManager.java:250) at com.igalia.wolvic.utils.EnvironmentsManager$1.onUnzipFinish(EnvironmentsManager.java:193) at com.igalia.wolvic.utils.zip.UnzipResultReceiver.lambda$onReceiveResult$0(UnzipResultReceiver.java:59) at com.igalia.wolvic.utils.zip.UnzipResultReceiver$$ExternalSyntheticLambda0.accept(D8$$SyntheticClass:0) at java.util.ArrayList.forEach(ArrayList.java:1262) at com.igalia.wolvic.utils.zip.UnzipResultReceiver.onReceiveResult(UnzipResultReceiver.java:50) at android.os.ResultReceiver$MyRunnable.run(ResultReceiver.java:50) at android.os.Handler.handleCallback(Handler.java:938) at android.os.Handler.dispatchMessage(Handler.java:99) at android.os.Looper.loopOnce(Looper.java:214) at android.os.Looper.loop(Looper.java:304) at android.app.ActivityThread.main(ActivityThread.java:7918) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1010) --- .../igalia/wolvic/downloads/DownloadsManager.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/app/src/common/shared/com/igalia/wolvic/downloads/DownloadsManager.java b/app/src/common/shared/com/igalia/wolvic/downloads/DownloadsManager.java index 1884967512..a03b3ae00b 100644 --- a/app/src/common/shared/com/igalia/wolvic/downloads/DownloadsManager.java +++ b/app/src/common/shared/com/igalia/wolvic/downloads/DownloadsManager.java @@ -64,6 +64,10 @@ public DownloadsManager(@NonNull Context context) { mDiskExecutor = ((VRBrowserApplication) mContext.getApplicationContext()).getExecutors().diskIO(); } + private void removeDownloadOnDiskIO(long id) { + mDiskExecutor.execute(() -> { mDownloadManager.remove(id); }); + } + public void init() { IntentFilter filter = new IntentFilter(DownloadManager.ACTION_DOWNLOAD_COMPLETE); if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { @@ -75,7 +79,7 @@ public void init() { downloads.forEach(download -> { File downloadedFile = download.getOutputFile(); if (mDownloadManager != null && (downloadedFile == null || !downloadedFile.exists())) { - mDownloadManager.remove(download.getId()); + removeDownloadOnDiskIO(download.getId()); } }); } @@ -235,19 +239,19 @@ public void removeDownload(long downloadId, boolean deleteFiles) { File newFile = new File(file.getAbsolutePath().concat(".bak")); file.renameTo(newFile); if (mDownloadManager != null) { - mDownloadManager.remove(downloadId); + removeDownloadOnDiskIO(downloadId); } newFile.renameTo(file); } else { if (mDownloadManager != null) { - mDownloadManager.remove(downloadId); + removeDownloadOnDiskIO(downloadId); } } } else { if (mDownloadManager != null) { - mDownloadManager.remove(downloadId); + removeDownloadOnDiskIO(downloadId); } } }