From 17a313b4c735893ff7789791fa2aa27922062e5e Mon Sep 17 00:00:00 2001 From: Manuel Martin Date: Fri, 24 Apr 2020 20:30:22 +0200 Subject: [PATCH] Fixes #3241 Fixes #3240 Download fixes (#3242) * Remove unnecessary cursor loop * Fixed typo * Report all pending downloads number and arithmetic exceptions * Fix deleting the downloaded file * Only show Tray notifications if tray is visible * Remove unused string parameter * Switch to empty list instantly when removing all downloads * Better diff handling --- .../mozilla/vrbrowser/VRBrowserActivity.java | 9 ++-- .../vrbrowser/downloads/DownloadsManager.java | 52 +++++++------------ .../ui/adapters/DownloadsAdapter.java | 10 ++-- .../ui/views/library/DownloadsView.java | 15 +++--- .../vrbrowser/ui/widgets/TrayWidget.java | 36 ++++++++----- .../vrbrowser/ui/widgets/WindowWidget.java | 18 +++---- .../mozilla/vrbrowser/ui/widgets/Windows.java | 2 +- .../ui/widgets/dialogs/CrashDialogWidget.java | 2 +- .../ui/widgets/dialogs/PermissionWidget.java | 2 +- .../widgets/dialogs/PromptDialogWidget.java | 6 +-- .../widgets/dialogs/RestartDialogWidget.java | 2 +- .../widgets/dialogs/SignOutDialogWidget.java | 2 +- .../ui/widgets/dialogs/VoiceSearchWidget.java | 2 +- .../ui/widgets/dialogs/WhatsNewWidget.java | 2 +- app/src/main/res/layout/tray.xml | 2 +- 15 files changed, 80 insertions(+), 82 deletions(-) diff --git a/app/src/common/shared/org/mozilla/vrbrowser/VRBrowserActivity.java b/app/src/common/shared/org/mozilla/vrbrowser/VRBrowserActivity.java index 283fc7550..9c6dfe219 100644 --- a/app/src/common/shared/org/mozilla/vrbrowser/VRBrowserActivity.java +++ b/app/src/common/shared/org/mozilla/vrbrowser/VRBrowserActivity.java @@ -729,7 +729,7 @@ public void onBackPressed() { new String[]{ getString(R.string.exit_confirm_dialog_button_cancel), getString(R.string.exit_confirm_dialog_button_quit), - }, index -> { + }, (index, isChecked) -> { if (index == PromptDialogWidget.POSITIVE) { VRBrowserActivity.super.onBackPressed(); } @@ -1150,7 +1150,7 @@ private void haltActivity(final int aReason) { mWindows.getFocusedWindow().showAlert( getString(R.string.not_entitled_title), getString(R.string.not_entitled_message, getString(R.string.app_name)), - index -> finish()); + (index, isChecked) -> finish()); } }); } @@ -1176,7 +1176,10 @@ private void handlePoorPerformance() { } window.getSession().loadHomePage(); final String[] buttons = {getString(R.string.ok_button), getString(R.string.performance_unblock_page)}; - window.showConfirmPrompt(getString(R.string.performance_title), getString(R.string.performance_message), buttons, index -> { + window.showConfirmPrompt(getString(R.string.performance_title), + getString(R.string.performance_message), + buttons, + (index, isChecked) -> { if (index == PromptDialogWidget.NEGATIVE) { mPoorPerformanceWhiteList.add(originalUri); window.getSession().loadUri(originalUri); diff --git a/app/src/common/shared/org/mozilla/vrbrowser/downloads/DownloadsManager.java b/app/src/common/shared/org/mozilla/vrbrowser/downloads/DownloadsManager.java index 636dc3442..1ea771c94 100644 --- a/app/src/common/shared/org/mozilla/vrbrowser/downloads/DownloadsManager.java +++ b/app/src/common/shared/org/mozilla/vrbrowser/downloads/DownloadsManager.java @@ -117,37 +117,30 @@ private String getOutputPathForJob(@NonNull DownloadJob job) { return null; } - public void removeDownload(long downloadId) { - mDownloadManager.remove(downloadId); - notifyDownloadsUpdate(); - } - - public void removeAllDownloads() { - if (getDownloads().size() > 0) { - mDownloadManager.remove(getDownloads().stream().mapToLong(Download::getId).toArray()); - notifyDownloadsUpdate(); - } - } - - public void clearDownload(long downloadId) { + public void removeDownload(long downloadId, boolean deleteFiles) { Download download = getDownload(downloadId); if (download != null) { - File file = new File(UrlUtils.stripProtocol(download.getOutputFile())); - if (file.exists()) { - File newFile = new File(UrlUtils.stripProtocol(download.getOutputFile().concat(".bak"))); - file.renameTo(newFile); + if (!deleteFiles) { + File file = new File(UrlUtils.stripProtocol(download.getOutputFile())); + if (file.exists()) { + File newFile = new File(UrlUtils.stripProtocol(download.getOutputFile().concat(".bak"))); + file.renameTo(newFile); + mDownloadManager.remove(downloadId); + newFile.renameTo(file); + + } else { + mDownloadManager.remove(downloadId); + } + + } else { mDownloadManager.remove(downloadId); - newFile.renameTo(file); } } notifyDownloadsUpdate(); } - public void clearAllDownloads() { - getDownloads().forEach(download -> { - clearDownload(download.getId()); - }); - notifyDownloadsUpdate(); + public void removeAllDownloads(boolean deleteFiles) { + getDownloads().forEach(download -> removeDownload(download.getId(), deleteFiles)); } @Nullable @@ -210,17 +203,8 @@ private void notifyDownloadError(@NonNull String error, @NonNull String file) { mListeners.forEach(listener -> listener.onDownloadError(error, file)); } - private Runnable mDownloadUpdateTask = new Runnable() { - @Override - public void run() { - DownloadManager.Query query = new DownloadManager.Query(); - Cursor c = mDownloadManager.query(query); - - while (c.moveToNext()) { - mMainHandler.post(() -> notifyDownloadsUpdate()); - } - c.close(); - } + private Runnable mDownloadUpdateTask = () -> { + mMainHandler.post(this::notifyDownloadsUpdate); }; } diff --git a/app/src/common/shared/org/mozilla/vrbrowser/ui/adapters/DownloadsAdapter.java b/app/src/common/shared/org/mozilla/vrbrowser/ui/adapters/DownloadsAdapter.java index 01ba9be02..42b592e44 100644 --- a/app/src/common/shared/org/mozilla/vrbrowser/ui/adapters/DownloadsAdapter.java +++ b/app/src/common/shared/org/mozilla/vrbrowser/ui/adapters/DownloadsAdapter.java @@ -77,18 +77,16 @@ public int getNewListSize() { @Override public boolean areItemsTheSame(int oldItemPosition, int newItemPosition) { - return mDownloadsList.get(oldItemPosition).getUri().equals(downloadsList.get(newItemPosition).getUri()); + return mDownloadsList.get(oldItemPosition).getId() == downloadsList.get(newItemPosition).getId(); } @Override public boolean areContentsTheSame(int oldItemPosition, int newItemPosition) { Download newDownloadItem = downloadsList.get(newItemPosition); Download oldDownloadItem = mDownloadsList.get(oldItemPosition); - return newDownloadItem.getLastModified() == oldDownloadItem.getLastModified() - && Objects.equals(newDownloadItem.getTitle(), oldDownloadItem.getTitle()) - && Objects.equals(newDownloadItem.getDescription(), oldDownloadItem.getDescription()) - && Objects.equals(newDownloadItem.getOutputFile(), oldDownloadItem.getOutputFile()) - && Objects.equals(newDownloadItem.getUri(), oldDownloadItem.getUri()); + return newDownloadItem.getProgress() == oldDownloadItem.getProgress() + && newDownloadItem.getStatus() == oldDownloadItem.getStatus() + && newDownloadItem.getFilename().equals(oldDownloadItem.getFilename()); } }); diff --git a/app/src/common/shared/org/mozilla/vrbrowser/ui/views/library/DownloadsView.java b/app/src/common/shared/org/mozilla/vrbrowser/ui/views/library/DownloadsView.java index d0a3076c1..5e06bb03c 100644 --- a/app/src/common/shared/org/mozilla/vrbrowser/ui/views/library/DownloadsView.java +++ b/app/src/common/shared/org/mozilla/vrbrowser/ui/views/library/DownloadsView.java @@ -161,9 +161,9 @@ public void onDelete(@NonNull View view, @NonNull Download item) { getContext().getString(R.string.download_delete_confirm_delete) }, getContext().getString(R.string.download_delete_file_confirm_checkbox), - index -> { + (index, isChecked) -> { if (index == PromptDialogWidget.POSITIVE) { - mDownloadsManager.removeDownload(item.getId()); + mDownloadsManager.removeDownload(item.getId(), isChecked); } } ); @@ -207,9 +207,10 @@ public void onDeleteDownloads(@NonNull View view) { getContext().getString(R.string.download_delete_confirm_delete) }, getContext().getString(R.string.download_delete_all_confirm_checkbox), - index -> { + (index, isChecked) -> { if (index == PromptDialogWidget.POSITIVE) { - mDownloadsManager.clearAllDownloads(); + mViewModel.setIsEmpty(true); + post(() -> mDownloadsManager.removeAllDownloads(isChecked)); } } ); @@ -293,9 +294,9 @@ public void onDelete(DownloadsContextMenuWidget.DownloadsContextMenuItem item) { getContext().getString(R.string.download_delete_confirm_delete) }, getContext().getString(R.string.download_delete_file_confirm_checkbox), - index -> { + (index, isChecked) -> { if (index == PromptDialogWidget.POSITIVE) { - mDownloadsManager.removeDownload(item.getDownloadsId()); + mDownloadsManager.removeDownload(item.getDownloadsId(), isChecked); } } ); @@ -380,7 +381,7 @@ public void onDownloadsUpdate(@NonNull List downloads) { public void onDownloadError(@NonNull String error, @NonNull String filename) { Log.e(LOGTAG, error); mWidgetManager.getFocusedWindow().showAlert( - getContext().getString(R.string.download_error_title, filename), + getContext().getString(R.string.download_error_title), error, null ); diff --git a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/TrayWidget.java b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/TrayWidget.java index 0a2211550..861fe1e3c 100644 --- a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/TrayWidget.java +++ b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/TrayWidget.java @@ -499,13 +499,15 @@ private void showNotification(int notificationId, UIButton button, int stringRes } private void showNotification(int notificationId, UIButton button, String string) { - NotificationManager.Notification notification = new NotificationManager.Builder(this) - .withView(button) - .withDensity(R.dimen.tray_tooltip_density) - .withString(string) - .withPosition(NotificationManager.Notification.TOP) - .withZTranslation(25.0f).build(); - NotificationManager.show(notificationId, notification); + if (isVisible()) { + NotificationManager.Notification notification = new NotificationManager.Builder(this) + .withView(button) + .withDensity(R.dimen.tray_tooltip_density) + .withString(string) + .withPosition(NotificationManager.Notification.TOP) + .withZTranslation(25.0f).build(); + NotificationManager.show(notificationId, notification); + } } private void hideNotifications() { @@ -528,16 +530,26 @@ public void onBookmarkAdded() { @Override public void onDownloadsUpdate(@NonNull List downloads) { - long inProgressNum = downloads.stream().filter(item -> item.getStatus() == Download.RUNNING).count(); + long inProgressNum = downloads.stream().filter(item -> + item.getStatus() == Download.RUNNING || + item.getStatus() == Download.PAUSED || + item.getStatus() == Download.PENDING).count(); mTrayViewModel.setDownloadsNumber((int)inProgressNum); if (inProgressNum == 0) { mBinding.downloadsButton.setLevel(0); } else { - long size = downloads.stream().filter(item -> item.getStatus() == Download.RUNNING).mapToLong(Download::getSizeBytes).sum(); - long downloaded = downloads.stream().filter(item -> item.getStatus() == Download.RUNNING).mapToLong(Download::getDownloadedBytes).sum(); - long percent = downloaded*100/size; - mBinding.downloadsButton.setLevel((int)percent*100); + long size = downloads.stream() + .filter(item -> item.getStatus() == Download.RUNNING) + .mapToLong(Download::getSizeBytes) + .sum(); + long downloaded = downloads.stream().filter(item -> item.getStatus() == Download.RUNNING) + .mapToLong(Download::getDownloadedBytes) + .sum(); + if (size > 0) { + long percent = downloaded*100/size; + mBinding.downloadsButton.setLevel((int)percent*100); + } } } diff --git a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/WindowWidget.java b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/WindowWidget.java index 47d30466e..ef62d36f2 100644 --- a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/WindowWidget.java +++ b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/WindowWidget.java @@ -1265,10 +1265,10 @@ public void showAlert(String title, @NonNull String msg, @Nullable PromptDialogW } mAlertDialog.setTitle(title); mAlertDialog.setBody(msg); - mAlertDialog.setButtonsDelegate(index -> { + mAlertDialog.setButtonsDelegate((index, isChecked) -> { mAlertDialog.hide(REMOVE_WIDGET); if (callback != null) { - callback.onButtonClicked(index); + callback.onButtonClicked(index, isChecked); } mAlertDialog.releaseWidget(); mAlertDialog = null; @@ -1345,10 +1345,10 @@ public void showConfirmPrompt(@DrawableRes int icon, mConfirmDialog.setTitle(title); mConfirmDialog.setBody(msg); mConfirmDialog.setButtons(btnMsg); - mConfirmDialog.setButtonsDelegate(index -> { + mConfirmDialog.setButtonsDelegate((index, isChecked) -> { mConfirmDialog.hide(REMOVE_WIDGET); if (callback != null) { - callback.onButtonClicked(index); + callback.onButtonClicked(index, isChecked); } mConfirmDialog.releaseWidget(); mConfirmDialog = null; @@ -1371,10 +1371,10 @@ public void showDialog(@NonNull String title, @StringRes int description, @NonN mAppDialog.setTitle(title); mAppDialog.setBody(description); mAppDialog.setButtons(btnMsg); - mAppDialog.setButtonsDelegate(index -> { + mAppDialog.setButtonsDelegate((index, isChecked) -> { mAppDialog.hide(REMOVE_WIDGET); if (buttonsCallback != null) { - buttonsCallback.onButtonClicked(index); + buttonsCallback.onButtonClicked(index, isChecked); } mAppDialog.releaseWidget(); }); @@ -1399,7 +1399,7 @@ public void showFirstTimeDrmDialog(@NonNull Runnable callback) { getContext().getString(R.string.drm_first_use_do_not_allow), getContext().getString(R.string.drm_first_use_allow), }, - index -> { + (index, isChecked) -> { // We remove the prefs listener before the first DRM update to avoid reloading the session mPrefs.unregisterOnSharedPreferenceChangeListener(this); SettingsStore.getInstance(getContext()).setDrmContentPlaybackEnabled(index == PromptDialogWidget.POSITIVE); @@ -1541,7 +1541,7 @@ public void startDownload(@NonNull DownloadJob downloadJob, boolean showConfirmD new String[]{ getResources().getString(R.string.download_confirm_cancel), getResources().getString(R.string.download_confirm_download)}, - index -> { + (index, isChecked) -> { if (index == PromptDialogWidget.POSITIVE) { mDownloadsManager.startDownload(downloadJob); } @@ -1649,7 +1649,7 @@ public void onExternalResponse(@NonNull GeckoSession geckoSession, @NonNull Geck new String[]{ getResources().getString(R.string.download_open_file_unsupported_cancel), getResources().getString(R.string.download_open_file_unsupported_open) - }, index -> { + }, (index, isChecked) -> { if (index == PromptDialogWidget.POSITIVE) { Uri contentUri = FileProvider.getUriForFile( getContext(), diff --git a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/Windows.java b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/Windows.java index bf6a4f950..154dfc007 100644 --- a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/Windows.java +++ b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/Windows.java @@ -1360,7 +1360,7 @@ public void onTabsReceived(@NonNull List aTabs) { mNoInternetDialog.setDescriptionVisible(false); mNoInternetDialog.setTitle(R.string.no_internet_title); mNoInternetDialog.setBody(R.string.no_internet_message); - mNoInternetDialog.setButtonsDelegate(index -> { + mNoInternetDialog.setButtonsDelegate((index, isChecked) -> { mNoInternetDialog.hide(UIWidget.REMOVE_WIDGET); mNoInternetDialog.releaseWidget(); mNoInternetDialog = null; diff --git a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/CrashDialogWidget.java b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/CrashDialogWidget.java index 5af8a44ea..08f18e835 100644 --- a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/CrashDialogWidget.java +++ b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/CrashDialogWidget.java @@ -48,7 +48,7 @@ public void updateUI() { R.string.do_not_sent_button, R.string.send_data_button }); - setButtonsDelegate(index -> { + setButtonsDelegate((index, isChecked) -> { if (index == PromptDialogWidget.NEGATIVE) { if (mFiles != null) { SystemUtils.clearCrashFiles(getContext(), mFiles); diff --git a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/PermissionWidget.java b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/PermissionWidget.java index 706100edf..8d2c8ca21 100644 --- a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/PermissionWidget.java +++ b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/PermissionWidget.java @@ -43,7 +43,7 @@ public void updateUI() { R.string.permission_reject, R.string.permission_allow }); - setButtonsDelegate(index -> { + setButtonsDelegate((index, isChecked) -> { if (index == PromptDialogWidget.NEGATIVE) { // Do not allow handlePermissionResult(false); diff --git a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/PromptDialogWidget.java b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/PromptDialogWidget.java index df83a1f86..30970f2ea 100644 --- a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/PromptDialogWidget.java +++ b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/PromptDialogWidget.java @@ -24,7 +24,7 @@ public class PromptDialogWidget extends UIDialog { public interface Delegate { - void onButtonClicked(int index); + void onButtonClicked(int index, boolean isChecked); default void onDismiss() {} } @@ -54,12 +54,12 @@ public void updateUI() { mBinding.leftButton.setOnClickListener(v -> { if (mAppDialogDelegate != null) { - mAppDialogDelegate.onButtonClicked(NEGATIVE); + mAppDialogDelegate.onButtonClicked(NEGATIVE, isChecked()); } }); mBinding.rightButton.setOnClickListener(v -> { if (mAppDialogDelegate != null) { - mAppDialogDelegate.onButtonClicked(POSITIVE); + mAppDialogDelegate.onButtonClicked(POSITIVE, isChecked()); } }); } diff --git a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/RestartDialogWidget.java b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/RestartDialogWidget.java index d7f2d749e..49fd81f88 100644 --- a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/RestartDialogWidget.java +++ b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/RestartDialogWidget.java @@ -25,7 +25,7 @@ public void updateUI() { R.string.restart_later_dialog_button, R.string.restart_now_dialog_button }); - setButtonsDelegate(index -> { + setButtonsDelegate((index, isChecked) -> { if (index == PromptDialogWidget.NEGATIVE) { onDismiss(); diff --git a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/SignOutDialogWidget.java b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/SignOutDialogWidget.java index b994f75a7..aa3400a64 100644 --- a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/SignOutDialogWidget.java +++ b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/SignOutDialogWidget.java @@ -47,7 +47,7 @@ public void updateUI() { R.string.fxa_signout_confirmation_signout, R.string.fxa_signout_confirmation_cancel }); - setButtonsDelegate(index -> { + setButtonsDelegate((index, isChecked) -> { if (index == PromptDialogWidget.NEGATIVE) { try { Objects.requireNonNull(mAccounts.logoutAsync()).thenAcceptAsync(unit -> { diff --git a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/VoiceSearchWidget.java b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/VoiceSearchWidget.java index a63e0a750..f11f8515e 100644 --- a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/VoiceSearchWidget.java +++ b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/VoiceSearchWidget.java @@ -296,7 +296,7 @@ public void show(@ShowFlags int aShowFlags) { new int[]{ R.string.voice_samples_collect_dialog_do_not_allow, R.string.voice_samples_collect_dialog_allow}, - index -> { + (index, isChecked) -> { SettingsStore.getInstance(getContext()).setSpeechDataCollectionReviewed(true); if (index == PromptDialogWidget.POSITIVE) { SettingsStore.getInstance(getContext()).setSpeechDataCollectionEnabled(true); diff --git a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/WhatsNewWidget.java b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/WhatsNewWidget.java index d46114bf2..5bdb74860 100644 --- a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/WhatsNewWidget.java +++ b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/dialogs/WhatsNewWidget.java @@ -46,7 +46,7 @@ public void updateUI() { R.string.whats_new_button_start_browsing, R.string.whats_new_button_sign_in }); - setButtonsDelegate(index -> { + setButtonsDelegate((index, isChecked) -> { if (index == PromptDialogWidget.NEGATIVE) { onDismiss(); diff --git a/app/src/main/res/layout/tray.xml b/app/src/main/res/layout/tray.xml index 07265b27b..5e5a797d1 100644 --- a/app/src/main/res/layout/tray.xml +++ b/app/src/main/res/layout/tray.xml @@ -102,7 +102,7 @@ app:autoSizeMaxTextSize="10sp" app:autoSizeStepGranularity="2sp" android:background="@drawable/downloads_badge" - android:text="@{(traymodel.downloadsNumber < 100) ? String.valueOf(traymodel.downloadsNumber) : @string/ellipsis" + android:text="@{(traymodel.downloadsNumber < 100) ? String.valueOf(traymodel.downloadsNumber) : @string/ellipsis}" visibleGone="@{traymodel.downloadsNumber > 0}"/>