Support undoing offline page deletion

chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkItemsAdapter.java

 // 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.enhancedbookmarks;
 
 import android.content.Context;
 import android.support.v7.widget.RecyclerView;
 import android.support.v7.widget.RecyclerView.ViewHolder;
 import android.view.LayoutInflater;
 import android.view.View;
 import android.view.ViewGroup;
 
 import org.chromium.base.annotations.SuppressFBWarnings;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.BookmarksBridge.BookmarkItem;
 import org.chromium.chrome.browser.BookmarksBridge.BookmarkModelObserver;
 import org.chromium.chrome.browser.enhancedbookmarks.EnhancedBookmarkManager.UIState;
 import org.chromium.chrome.browser.enhancedbookmarks.EnhancedBookmarkPromoHeader.PromoHeaderShowingChangeListener;
 import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
+import org.chromium.chrome.browser.offlinepages.OfflinePageBridge.OfflinePageModelObserver;
 import org.chromium.chrome.browser.offlinepages.OfflinePageFreeUpSpaceCallback;
 import org.chromium.chrome.browser.offlinepages.OfflinePageFreeUpSpaceDialog;
 import org.chromium.chrome.browser.offlinepages.OfflinePageStorageSpaceHeader;
 import org.chromium.components.bookmarks.BookmarkId;
 
 import java.util.ArrayList;
 import java.util.List;
 
 /**
  * BaseAdapter for EnhancedBookmarkItemsContainer. It manages bookmarks to list there.
@@ skipping 39 matching lines @@
                 }
             }
         }
 
         @Override
         public void bookmarkModelChanged() {
             mDelegate.notifyStateChange(EnhancedBookmarkItemsAdapter.this);
         }
     };
 
+    private OfflinePageModelObserver mOfflinePageModelObserver;
+
     EnhancedBookmarkItemsAdapter(Context context) {
         mContext = context;
 
         mSections = new ArrayList<>();
         mSections.add(mPromoHeaderSection);
         mSections.add(mOfflineStorageSection);
         mSections.add(mFolderDividerSection);
         mSections.add(mFolderSection);
         mSections.add(mBookmarkDividerSection);
         mSections.add(mBookmarkSection);
@@ skipping 175 matching lines @@
     // EnhancedBookmarkUIObserver implementations.
 
     @Override
     public void onEnhancedBookmarkDelegateInitialized(EnhancedBookmarkDelegate delegate) {
         mDelegate = delegate;
         mDelegate.addUIObserver(this);
         mDelegate.getModel().addObserver(mBookmarkModelObserver);
         mPromoHeaderManager = new EnhancedBookmarkPromoHeader(mContext, this);
         OfflinePageBridge offlinePageBridge = mDelegate.getModel().getOfflinePageBridge();
         if (offlinePageBridge != null) {
+            mOfflinePageModelObserver = new OfflinePageModelObserver() {
+                @Override
+                public void offlinePageModelChanged() {
+                    mDelegate.notifyStateChange(EnhancedBookmarkItemsAdapter.this);

[Ian Wen, 2015/10/14 23:30:01]

1. Is offline model change capable of removing/adding bookmarks? If not, I would
use notifyDataSetChanged() here.

EBDelegate#notifyStateChange() will let bookmark model regenerate the list of
bookmarks to show. Instead, notifyDataSetChange() only tells recycler view to
relayout all items.

2. It seems offlinePageModelChanged() might be called after offlinepage model is
loaded. If we iniatialize the offline page during EB's initialization, we might
end up having to refresh this adapter twice (once for EB's loading, once for
offline page). Are there better designs to prevent this refresh-twice problem?

[jianli, 2015/10/14 23:40:48]

We need to call notifyStateChange, since relayout provided by
notifyDataSetChange is not enough because the removed items will reappear here.
OfflinePageModelChanged will only be fired after the model is loaded and there
is a request to add/remove/update offline page items, mostly due to user action.
So it means that we do need to refresh the UI per the user action for most of
the cases. In the rare case of bookmark sync, this is likely but probably we
don't want to handle it.

[Ian Wen, 2015/10/15 00:02:16]

Hmm I saw another CL that adds delete signals to the model observer. If delete
is handled specifically, are there other reasons that we want to use
notifyStateChange()?

[jianli, 2015/10/15 00:05:26]

Yes, we need it for undoing the bookmark deletion that will also bring the
offline page back.

+                }
+            };
+            offlinePageBridge.addObserver(mOfflinePageModelObserver);
+
             mOfflineStorageHeader = new OfflinePageStorageSpaceHeader(
                     mContext, offlinePageBridge, new OfflinePageFreeUpSpaceCallback() {
                         @Override
                         public void onFreeUpSpaceDone() {
                             refreshOfflinePagesFilterView();
                             mDelegate.getSnackbarManager().showSnackbar(
                                     OfflinePageFreeUpSpaceDialog.createStorageClearedSnackbar(
                                             mContext));
                         }
 
                         @Override
                         public void onFreeUpSpaceCancelled() {
                             // No need to refresh, as result outcome should be the same here.
                         }
                     });
         }
     }
 
     @Override
     public void onDestroy() {
         mDelegate.removeUIObserver(this);
         mDelegate.getModel().removeObserver(mBookmarkModelObserver);
         mPromoHeaderManager.destroy();
-        if (mOfflineStorageHeader != null) {
+
+        OfflinePageBridge offlinePageBridge = mDelegate.getModel().getOfflinePageBridge();
+        if (offlinePageBridge != null) {
+            offlinePageBridge.removeObserver(mOfflinePageModelObserver);
             mOfflineStorageHeader.destroy();
         }
     }
 
     @Override
     public void onAllBookmarksStateSet() {
         setBookmarks(null, mDelegate.getModel().getAllBookmarkIDsOrderedByCreationDate());
     }
 
     @Override
@@ skipping 35 matching lines @@
             }
         }
     }
 
     private void refreshOfflinePagesFilterView() {
         if (mDelegate.getCurrentState() != UIState.STATE_FILTER) return;
         setBookmarks(null,
                 mDelegate.getModel().getBookmarkIDsByFilter(EnhancedBookmarkFilter.OFFLINE_PAGES));
     }
 }