Merge pull request #326 from cliqz-oss/dev

Tab deletion logic moved to TabsManager

app/src/main/java/acr/browser/lightning/activity/BrowserActivity.java

@@ -100,6 +100,7 @@ import acr.browser.lightning.constant.HistoryPage;
 import acr.browser.lightning.controller.UIController;
 import acr.browser.lightning.database.BookmarkManager;
 import acr.browser.lightning.database.HistoryDatabase;
+import acr.browser.lightning.database.HistoryItem;
 import acr.browser.lightning.dialog.LightningDialogBuilder;
 import acr.browser.lightning.fragment.BookmarksFragment;
 import acr.browser.lightning.fragment.TabsFragment;
@@ -667,6 +668,7 @@ public abstract class BrowserActivity extends ThemableBrowserActivity implements
     @Override
     public boolean onOptionsItemSelected(MenuItem item) {
         final LightningView currentView = mTabsManager.getCurrentTab();
+        final String currentUrl = currentView != null ? currentView.getUrl() : null;
         // Handle action buttons
         switch (item.getItemId()) {
             case android.R.id.home:
@@ -692,11 +694,11 @@ public abstract class BrowserActivity extends ThemableBrowserActivity implements
                 overridePendingTransition(R.anim.slide_up_in, R.anim.fade_out_scale);
                 return true;
             case R.id.action_share:
-                if (currentView != null && !UrlUtils.isSpecialUrl(currentView.getUrl())) {
+                if (currentUrl != null && !UrlUtils.isSpecialUrl(currentUrl)) {
                     Intent shareIntent = new Intent(Intent.ACTION_SEND);
                     shareIntent.setType("text/plain");
                     shareIntent.putExtra(Intent.EXTRA_SUBJECT, currentView.getTitle());
-                    shareIntent.putExtra(Intent.EXTRA_TEXT, currentView.getUrl());
+                    shareIntent.putExtra(Intent.EXTRA_TEXT, currentUrl);
                     startActivity(Intent.createChooser(shareIntent, getResources().getString(R.string.dialog_title_share)));
                 }
                 return true;
@@ -704,9 +706,9 @@ public abstract class BrowserActivity extends ThemableBrowserActivity implements
                 openBookmarks();
                 return true;
             case R.id.action_copy:
-                if (currentView != null && !UrlUtils.isSpecialUrl(currentView.getUrl())) {
+                if (currentUrl != null && !UrlUtils.isSpecialUrl(currentUrl)) {
                     ClipboardManager clipboard = (ClipboardManager) getSystemService(CLIPBOARD_SERVICE);
-                    ClipData clip = ClipData.newPlainText("label", currentView.getUrl());
+                    ClipData clip = ClipData.newPlainText("label", currentUrl);
                     clipboard.setPrimaryClip(clip);
                     Utils.showSnackbar(this, R.string.message_link_copied);
                 }
@@ -718,18 +720,17 @@ public abstract class BrowserActivity extends ThemableBrowserActivity implements
                 openHistory();
                 return true;
             case R.id.action_add_bookmark:
-                if (currentView != null && !UrlUtils.isSpecialUrl(currentView.getUrl())) {
-                    mEventBus.post(new BrowserEvents.AddBookmark(currentView.getTitle(),
-                            currentView.getUrl()));
+                if (currentUrl != null && !UrlUtils.isSpecialUrl(currentUrl)) {
+                    addBookmark(currentView.getTitle(), currentUrl);
                 }
                 return true;
             case R.id.action_find:
                 findInPage();
                 return true;
             case R.id.action_reading_mode:
-                if (currentView != null) {
+                if (currentUrl != null) {
                     Intent read = new Intent(this, ReadingActivity.class);
-                    read.putExtra(Constants.LOAD_READING_URL, currentView.getUrl());
+                    read.putExtra(Constants.LOAD_READING_URL, currentUrl);
                     startActivity(read);
                 }
                 return true;
@@ -738,6 +739,27 @@ public abstract class BrowserActivity extends ThemableBrowserActivity implements
         }
     }
 
+    // By using a manager, adds a bookmark and notifies third parties about that
+    private void addBookmark(final String title, final String url) {
+        final HistoryItem item = !mBookmarkManager.isBookmark(url)
+                ? new HistoryItem(url, title)
+                : null;
+        if (item != null && mBookmarkManager.addBookmark(item)) {
+            mSearchAdapter.refreshBookmarks();
+            mEventBus.post(new BrowserEvents.BookmarkAdded(title, url));
+        }
+    }
+
+    private void deleteBookmark(final String title, final String url) {
+        final HistoryItem item = mBookmarkManager.isBookmark(url)
+                ? new HistoryItem(url, title)
+                : null;
+        if (item != null && mBookmarkManager.deleteBookmark(item)) {
+            mSearchAdapter.refreshBookmarks();
+            mEventBus.post(new BrowserEvents.CurrentPageUrl(url));
+        }
+    }
+
     /**
      * method that shows a dialog asking what string the user wishes to search
      * for. It highlights the text entered.
@@ -817,8 +839,14 @@ public abstract class BrowserActivity extends ThemableBrowserActivity implements
      */
     private synchronized void showTab(final int position) {
         final LightningView currentView = mTabsManager.getCurrentTab();
-        final WebView currentWebView = currentView != null ? currentView.getWebView() : null;
         final LightningView newView = mTabsManager.switchToTab(position);
+        switchTabs(currentView, newView);
+    }
+
+    // This is commodity to breack the flow between regular tab management and the CLIQZ's search
+    // interface.
+    private void switchTabs(final LightningView currentView, final LightningView newView) {
+        final WebView currentWebView = currentView != null ? currentView.getWebView() : null;
         final WebView newWebView = newView != null ? newView.getWebView() : null;
         if (newView == null || newWebView == null) {
             return;
@@ -1001,14 +1029,11 @@ public abstract class BrowserActivity extends ThemableBrowserActivity implements
 
     private synchronized void deleteTab(int position) {
         final LightningView tabToDelete = mTabsManager.getTabAtPosition(position);
-        final LightningView currentTab = mTabsManager.getCurrentTab();
 
         if (tabToDelete == null) {
             return;
         }
 
-        int current = mTabsManager.positionOf(currentTab);
-
         if (!UrlUtils.isSpecialUrl(tabToDelete.getUrl()) && !isIncognito()) {
             mPreferences.setSavedUrl(tabToDelete.getUrl());
         }
@@ -1017,37 +1042,22 @@ public abstract class BrowserActivity extends ThemableBrowserActivity implements
         if (isShown) {
             mBrowserFrame.setBackgroundColor(mBackgroundColor);
         }
-        if (current > position) {
-            mTabsManager.deleteTab(position);
-            mEventBus.post(new BrowserEvents.TabsChanged());
-        } else if (mTabsManager.size() > position + 1) {
-            if (current == position) {
-                showTab(position + 1);
-                mTabsManager.deleteTab(position);
-                mEventBus.post(new BrowserEvents.TabsChanged());
-            } else {
-                mTabsManager.deleteTab(position);
-            }
-        } else if (mTabsManager.size() > 1) {
-            if (current == position) {
-                showTab(position - 1);
-                mTabsManager.deleteTab(position);
-                mEventBus.post(new BrowserEvents.TabsChanged());
-            } else {
-                mTabsManager.deleteTab(position);
-            }
-        } else {
-            if (currentTab != null && (UrlUtils.isSpecialUrl(currentTab.getUrl())
-                    || currentTab.getUrl().equals(mHomepage))) {
-                closeActivity();
-            } else {
-                mTabsManager.deleteTab(position);
-                performExitCleanUp();
-                tabToDelete.pauseTimers();
-                mEventBus.post(new BrowserEvents.TabsChanged());
-                finish();
-            }
+        final LightningView currentTab = mTabsManager.getCurrentTab();
+        mTabsManager.deleteTab(position);
+        final LightningView afterTab = mTabsManager.getCurrentTab();
+        if (afterTab == null) {
+//            if (currentTab != null && (UrlUtils.isSpecialUrl(currentTab.getUrl())
+//                    || currentTab.getUrl().equals(mPreferenceManager.getHomepage()))) {
+//                closeActivity();
+//            } else {
+            performExitCleanUp();
+            finish();
+//            }
+        } else if (afterTab != currentTab) {
+            switchTabs(currentTab, afterTab);
+            currentTab.pauseTimers();
         }
+
         mEventBus.post(new BrowserEvents.TabsChanged());
 
         if (shouldClose) {
@@ -2007,29 +2017,26 @@ public abstract class BrowserActivity extends ThemableBrowserActivity implements
         }
 
         /**
-         * When receive a {@link acr.browser.lightning.bus.BookmarkEvents.WantToBookmarkCurrentPage}
+         * When receive a {@link BookmarkEvents.ToggleBookmarkForCurrentPage}
          * message this receiver answer firing the
-         * {@link acr.browser.lightning.bus.BrowserEvents.AddBookmark} message
+         * {@link BrowserEvents.BookmarkAdded} message
          *
          * @param event an event that the user wishes to bookmark the current page
          */
         @Subscribe
-        public void bookmarkCurrentPage(final BookmarkEvents.WantToBookmarkCurrentPage event) {
+        public void bookmarkCurrentPage(final BookmarkEvents.ToggleBookmarkForCurrentPage event) {
             final LightningView currentTab = mTabsManager.getCurrentTab();
-            if (currentTab != null) {
-                mEventBus.post(new BrowserEvents.AddBookmark(currentTab.getTitle(), currentTab.getUrl()));
+            final String url = currentTab != null ? currentTab.getUrl() : null;
+            final String title = currentTab != null ? currentTab.getTitle() : null;
+            if (url == null) {
+                return;
             }
-        }
 
-        /**
-         * This message is received when a bookmark was added by the
-         * {@link acr.browser.lightning.fragment.BookmarksFragment}
-         *
-         * @param event the event that a bookmark has been added
-         */
-        @Subscribe
-        public void bookmarkAdded(final BookmarkEvents.Added event) {
-            mSearchAdapter.refreshBookmarks();
+            if (!mBookmarkManager.isBookmark(url)) {
+                addBookmark(title, url);
+            } else {
+                deleteBookmark(title, url);
+            }
         }
 
         /**

app/src/main/java/acr/browser/lightning/activity/TabsManager.java

@@ -9,6 +9,8 @@ import android.support.v7.app.AlertDialog;
 import android.util.Log;
 import android.webkit.WebView;
 
+import com.squareup.otto.Bus;
+
 import java.util.ArrayList;
 import java.util.List;
 
@@ -16,8 +18,10 @@ import javax.inject.Inject;
 import javax.inject.Singleton;
 
 import acr.browser.lightning.R;
+import acr.browser.lightning.bus.BrowserEvents;
 import acr.browser.lightning.constant.Constants;
 import acr.browser.lightning.preference.PreferenceManager;
+import acr.browser.lightning.utils.UrlUtils;
 import acr.browser.lightning.utils.Utils;
 import acr.browser.lightning.view.LightningView;
 
@@ -35,6 +39,9 @@ public class TabsManager {
     @Inject
     PreferenceManager mPreferenceManager;
 
+    @Inject
+    Bus mEventBus;
+
     @Inject
     public TabsManager() {}
 
@@ -182,7 +189,7 @@ public class TabsManager {
      * @return The removed tab reference or null
      */
     @Nullable
-    public synchronized LightningView deleteTab(final int position) {
+    private synchronized LightningView removeTab(final int position) {
         if (position >= mWebViewList.size()) {
             return null;
         }
@@ -191,9 +198,29 @@ public class TabsManager {
             mCurrentTab = null;
         }
         tab.onDestroy();
+        Log.d(Constants.TAG, tab.toString());
         return tab;
     }
 
+    public synchronized void deleteTab(int position) {
+        final LightningView currentTab = getCurrentTab();
+        int current = positionOf(currentTab);
+
+        if (current == position) {
+            if (size() == 1) {
+                mCurrentTab = null;
+            } else if (current < size() - 1 ) {
+                // There is another tab after this one
+                mCurrentTab = getTabAtPosition(current + 1);
+            } else {
+                mCurrentTab = getTabAtPosition(current - 1);
+            }
+            removeTab(current);
+        } else {
+            removeTab(position);
+        }
+    }
+
     /**
      * Return the position of the given tab.
      *

app/src/main/java/acr/browser/lightning/bus/BookmarkEvents.java

@@ -23,20 +23,9 @@ public final class BookmarkEvents {
     }
 
     /**
-     * The user ask to bookmark the currently displayed page
+     * The user ask to add/del a bookmark to the currently displayed page
      */
-    public static class WantToBookmarkCurrentPage {
-    }
-
-    /**
-     * The bookmark was added
-     */
-    public static class Added {
-        public final HistoryItem item;
-
-        public Added(final HistoryItem item) {
-            this.item = item;
-        }
+    public static class ToggleBookmarkForCurrentPage {
     }
 
     /**

app/src/main/java/acr/browser/lightning/bus/BrowserEvents.java

@@ -12,14 +12,13 @@ public final class BrowserEvents {
     }
 
     /**
-     * Used to reply to the {@link acr.browser.lightning.fragment.BookmarksFragment} message
-     * {@link acr.browser.lightning.bus.BookmarkEvents.WantToBookmarkCurrentPage}. The interaction
-     * result is a new bookmark added.
+     * The {@link acr.browser.lightning.activity.BrowserActivity} signal a new bookmark was added
+     * (mainly to the {@link acr.browser.lightning.fragment.BookmarksFragment}).
      */
-    public static class AddBookmark {
+    public static class BookmarkAdded {
         public final String title, url;
 
-        public AddBookmark(final String title, final String url) {
+        public BookmarkAdded(final String title, final String url) {
             this.title = title;
             this.url = url;
         }

app/src/main/java/acr/browser/lightning/fragment/BookmarksFragment.java

@@ -30,7 +30,6 @@ import com.squareup.otto.Bus;
 import com.squareup.otto.Subscribe;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 
 import javax.inject.Inject;
@@ -49,7 +48,6 @@ import acr.browser.lightning.dialog.LightningDialogBuilder;
 import acr.browser.lightning.preference.PreferenceManager;
 import acr.browser.lightning.async.ImageDownloadTask;
 import acr.browser.lightning.utils.ThemeUtils;
-import acr.browser.lightning.utils.UrlUtils;
 import acr.browser.lightning.view.LightningView;
 
 /**
@@ -192,22 +190,17 @@ public class BookmarksFragment extends Fragment implements View.OnClickListener,
     }
 
     @Subscribe
-    public void addBookmark(final BrowserEvents.AddBookmark event) {
-        final HistoryItem item = new HistoryItem(event.url, event.title);
-        if (!UrlUtils.isSpecialUrl(item.getUrl())) {
-            if (mBookmarkManager.addBookmark(item)) {
-                mBookmarks.add(item);
-                Collections.sort(mBookmarks, new BookmarkManager.SortIgnoreCase());
-                mBookmarkAdapter.notifyDataSetChanged();
-                mEventBus.post(new BookmarkEvents.Added(item));
-                updateBookmarkIndicator(event.url);
-            }
-        }
+    public void addBookmark(final BrowserEvents.BookmarkAdded event) {
+        updateBookmarkIndicator(event.url);
+        String folder = mBookmarkManager.getCurrentFolder();
+        setBookmarkDataSet(mBookmarkManager.getBookmarksFromFolder(folder, true), false);
     }
 
     @Subscribe
     public void currentPageInfo(final BrowserEvents.CurrentPageUrl event) {
         updateBookmarkIndicator(event.url);
+        String folder = mBookmarkManager.getCurrentFolder();
+        setBookmarkDataSet(mBookmarkManager.getBookmarksFromFolder(folder, true), false);
     }
 
     @Subscribe
@@ -316,7 +309,7 @@ public class BookmarksFragment extends Fragment implements View.OnClickListener,
     public void onClick(View v) {
         switch (v.getId()) {
             case R.id.action_add_bookmark:
-                mEventBus.post(new BookmarkEvents.WantToBookmarkCurrentPage());
+                mEventBus.post(new BookmarkEvents.ToggleBookmarkForCurrentPage());
                 break;
             case R.id.action_reading:
                 LightningView currentTab = mTabsManager.getCurrentTab();