[Offline pages] Adding capability to free up space used by Offline pages

chrome/android/java/src/org/chromium/chrome/browser/offline_pages/OfflinePageFreeUpSpaceDialog.java

Index: chrome/android/java/src/org/chromium/chrome/browser/offline_pages/OfflinePageFreeUpSpaceDialog.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/offline_pages/OfflinePageFreeUpSpaceDialog.java b/chrome/android/java/src/org/chromium/chrome/browser/offline_pages/OfflinePageFreeUpSpaceDialog.java
new file mode 100644
index 0000000000000000000000000000000000000000..075844b6a966a44dffe0bf40edbf60100ce23e4f
--- /dev/null
+++ b/chrome/android/java/src/org/chromium/chrome/browser/offline_pages/OfflinePageFreeUpSpaceDialog.java
@@ -0,0 +1,105 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.offline_pages;
+
+import android.app.Dialog;
+import android.app.DialogFragment;
+import android.content.DialogInterface;
+import android.os.Bundle;
+import android.support.v7.app.AlertDialog;
+import android.text.format.Formatter;
+
+import org.chromium.chrome.R;
+import org.chromium.chrome.browser.offline_pages.OfflinePageBridge.DeletePageCallback;
+import org.chromium.components.bookmarks.BookmarkId;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Shows a "Free up space" dialog to clean up storage for offline pages.
+ */
+public class OfflinePageFreeUpSpaceDialog
+        extends DialogFragment implements DialogInterface.OnClickListener {
+    private OfflinePageBridge mOfflinePageBridge;
+    private List<OfflinePageItem> mOfflinePagesToDelete;
+    private FreeUpSpaceCallback mCallback;
+
+    /**
+     * Interface for informing the caller that the clean up operation is completed.
+     */
+    public interface FreeUpSpaceCallback {
+        /** Called when clean up operation is completed, or the dialog is cancelled. */
+        void onCleanUpSpaceDone();

[newt (away), 2015/08/24 22:37:08]

Use "free up" or "clean up" consistently. Don't mix them as that reduces
predictability.

[fgorski, 2015/08/25 18:25:25]

Done.

+    }
+
+    public static OfflinePageFreeUpSpaceDialog newInstance(

[newt (away), 2015/08/24 22:37:08]

public methods need javadoc

[fgorski, 2015/08/25 18:25:25]

Done. For some reason javadoc was misplaced.

[newt (away), 2015/08/27 03:13:27]

Oh weird. Better now :)

+            OfflinePageBridge offlinePageBridge, FreeUpSpaceCallback callback) {
+        assert offlinePageBridge != null;
+        OfflinePageFreeUpSpaceDialog dialog = new OfflinePageFreeUpSpaceDialog();
+        dialog.mOfflinePageBridge = offlinePageBridge;
+        dialog.mCallback = callback;
+        return dialog;
+    }
+
+    /**
+     * Creates the dialog. If the passed bridge instance needs to be destroyed after the dialog
+     * is finished, it should be taken care of in the callback implementation.
+     *
+     * @param offlinePageBridge An object to access offline page functionality.
+     * @param callback An object that will be called when the dialog finishes. Can be null.
+     * @see FreeUpSpaceCallback
+     */
+
+    @Override
+    public Dialog onCreateDialog(Bundle savedInstanceState) {
+        if (savedInstanceState != null) dismiss();
+
+        mOfflinePagesToDelete = mOfflinePageBridge.getPagesToCleanUp();
+        AlertDialog.Builder builder =
+                new AlertDialog.Builder(getActivity(), R.style.AlertDialogTheme)
+                        .setTitle(R.string.offline_pages_free_up_space_title)
+                        .setPositiveButton(R.string.delete, this)
+                        .setNegativeButton(R.string.cancel, this)
+                        .setMessage(getString(R.string.offline_pages_free_up_space_message,
+                                mOfflinePagesToDelete.size(),
+                                Formatter.formatFileSize(getActivity(), getTotalSize())));
+        return builder.create();
+    }
+
+    @Override
+    public void onClick(DialogInterface dialog, int id) {
+        if (id != AlertDialog.BUTTON_POSITIVE) {

[newt (away), 2015/08/24 22:37:08]

why "!=" ?? It seems clearer to write "if (id == AlertDialog.BUTTON_NEGATIVE)"

[fgorski, 2015/08/25 18:25:25]

Done.

+            dialog.cancel();
+            if (mCallback != null) mCallback.onCleanUpSpaceDone();

[newt (away), 2015/08/24 22:37:08]

Calling onCleanUpSpaceDone() doesn't seem right, since the space wasn't actually
cleaned up. If you want to differentiate between the cleaned-up cases and the
canceled case, you should add another method "onCanceled()" to
FreeUpSpaceCallback; otherwise, rename "onCleanUpSpaceDone()" to
"onDialogDismissed()" or something like that.

[fgorski, 2015/08/25 18:25:25]

Done.

+            return;
+        }
+
+        mOfflinePageBridge.deletePages(getBookmarkIdsToDelete(), new DeletePageCallback() {
+            @Override
+            public void onDeletePageDone(int deletePageResult) {
+                if (mCallback != null) mCallback.onCleanUpSpaceDone();
+            }
+        });
+    }
+
+    /** @return A list of Bookmark IDs for which the offline pages will be deleted. */

[newt (away), 2015/08/24 22:37:08]

nit: no need for "@return" for single-line javadocs. You can just write "Returns
a list..."

https://google.github.io/styleguide/javaguide.html#s7.2-summary-fragment

[fgorski, 2015/08/25 18:25:25]

Done.

+    private List<BookmarkId> getBookmarkIdsToDelete() {
+        List<BookmarkId> bookmarkIds = new ArrayList<BookmarkId>();
+        for (OfflinePageItem offlinePage : mOfflinePagesToDelete) {
+            bookmarkIds.add(offlinePage.getBookmarkId());
+        }
+        return bookmarkIds;
+    }
+
+    /** @return A total size of offline pages that will be deleted. */
+    private long getTotalSize() {
+        long totalSize = 0;
+        for (OfflinePageItem offlinePage : mOfflinePagesToDelete) {
+            totalSize += offlinePage.getFileSize();
+        }
+        return totalSize;
+    }
+}