Bug 1414084 - Part 13 - Cache PageActions. r?grisha draft
authorJan Henning <jh+bugzilla@buttercookie.de>
Mon, 26 Feb 2018 21:50:50 +0100
changeset 823888 b3b141829012f006aaeddd3b09f387120ffc5faf
parent 823887 de83af1d10c5636294e8c2a93b9c192a91d0515f
child 823889 73af07e0d7456d324113079928b9910a08ca4bf5
push id117809
push usermozilla@buttercookie.de
push dateSun, 29 Jul 2018 18:19:15 +0000
reviewersgrisha
bugs1414084
milestone63.0a1
Bug 1414084 - Part 13 - Cache PageActions. r?grisha Since converting a PageAction message into an actual PageAction object also en- tails parsing the image data URL into a drawable, we leave that task to the PageActionLayout. This means that the PageAction cache needs to operate slightly differently than the MenuItem cache. First, we store all PageAction BundleEvent messages that arrive while no PageActionLayout is ready and then forward them en masse when one becomes available. Secondly, if the PageActionLayout is going away again, we then also take a list of already parsed PageAction objects for safekeeping. MozReview-Commit-ID: AcPPONXqe46
mobile/android/base/java/org/mozilla/gecko/AddonUICache.java
mobile/android/base/java/org/mozilla/gecko/toolbar/PageActionLayout.java
--- a/mobile/android/base/java/org/mozilla/gecko/AddonUICache.java
+++ b/mobile/android/base/java/org/mozilla/gecko/AddonUICache.java
@@ -1,30 +1,36 @@
 /* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
  * vim: ts=4 sw=4 expandtab:
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko;
 
+import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
 import android.support.annotation.VisibleForTesting;
 import android.text.TextUtils;
 import android.util.Log;
 import android.view.Menu;
 import android.view.MenuItem;
 import android.view.SubMenu;
 
 import org.mozilla.gecko.util.BundleEventListener;
 import org.mozilla.gecko.util.EventCallback;
 import org.mozilla.gecko.util.GeckoBundle;
 
 import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
 
+import static org.mozilla.gecko.toolbar.PageActionLayout.PageAction;
+import static org.mozilla.gecko.toolbar.PageActionLayout.PageActionLayoutDelegate;
+
 /**
  * For certain UI items added by add-ons or other JS/Gecko code, Gecko notifies us whenever an item
  * is added, changed or removed. Since we must not miss any of these notifications and need to re-
  * member the current list of active UI items even if e.g. we're in background and our activities
  * have been destroyed, we need a class whose lifetime matches (or even exceeds) that of Gecko.
  *
  * This class fulfills this purpose - it will be initialised early during app startup and just like
  * Gecko, once initialised it will remain alive until the OS kills our app.
@@ -55,16 +61,24 @@ public class AddonUICache implements Bun
     }
 
     private static final AddonUICache instance = new AddonUICache();
 
     private final Map<String, MenuItemInfo> mAddonMenuItemsCache = new LinkedHashMap<>();
     private int mAddonMenuNextID = ADDON_MENU_OFFSET;
     private Menu mMenu;
 
+    // A collection of PageAction messages that are still pending transformation into
+    // a full PageAction - most importantly transformation of the image data URI
+    // into a Drawable.
+    private final Map<String, GeckoBundle> mPendingPageActionQueue = new LinkedHashMap<>();
+    // A collection of PageActions ready for immediate usage.
+    private List<PageAction> mResolvedPageActionCache;
+    private PageActionLayoutDelegate mPageActionDelegate;
+
     private boolean mInitialized;
 
     public static AddonUICache getInstance() {
         return instance;
     }
 
     private AddonUICache() { }
 
@@ -72,26 +86,31 @@ public class AddonUICache implements Bun
         if (mInitialized) {
             return;
         }
 
         EventDispatcher.getInstance().registerUiThreadListener(this,
             "Menu:Add",
             "Menu:Update",
             "Menu:Remove",
+            "PageActions:Add",
+            "PageActions:Remove",
             null);
 
         mInitialized = true;
     }
 
     @VisibleForTesting
     /* package, intended private */ void reset() {
         mAddonMenuItemsCache.clear();
         mAddonMenuNextID = ADDON_MENU_OFFSET;
         mMenu = null;
+        mPendingPageActionQueue.clear();
+        mResolvedPageActionCache = null;
+        mPageActionDelegate = null;
     }
 
     @Override
     public void handleMessage(String event, GeckoBundle message, EventCallback callback) {
         switch (event) {
             case "Menu:Add":
                 final MenuItemInfo info = new MenuItemInfo();
                 info.label = message.getString("name");
@@ -120,20 +139,70 @@ public class AddonUICache implements Bun
             case "Menu:Remove":
                 removeAddonMenuItem(message.getString("uuid"));
                 break;
 
             case "Menu:Update":
                 updateAddonMenuItem(message.getString("uuid"),
                                     message.getBundle("options"));
                 break;
+
+            case "PageActions:Add":
+                if (mPageActionDelegate != null) {
+                    mPageActionDelegate.addPageAction(message);
+                } else {
+                    mPendingPageActionQueue.put(message.getString("id"), message);
+                }
+                break;
+
+            case "PageActions:Remove":
+                if (mPageActionDelegate != null) {
+                    mPageActionDelegate.removePageAction(message);
+                } else {
+                    mPendingPageActionQueue.remove(message.getString("id"));
+                }
+                break;
         }
     }
 
     /**
+     * If a list of {@link PageAction PageActions} has previously been provided in
+     * {@link AddonUICache#removePageActionLayoutDelegate}, it will be transferred back to the
+     * {@link PageActionLayoutDelegate}.
+     * In addition, any <code>GeckoBundles</code> containing <code>PageAction</code> messages that
+     * arrived while no delegate was available will now be transmitted.
+     * <p>
+     * Following this, any <code>PageAction</code> messages that arrive will be forwarded
+     * immediately to the provided delegate.
+     */
+    public void setPageActionLayoutDelegate(final @NonNull PageActionLayoutDelegate newDelegate) {
+        newDelegate.setCachedPageActions(mResolvedPageActionCache);
+        mResolvedPageActionCache = null;
+
+        for (GeckoBundle pageActionMessage : mPendingPageActionQueue.values()) {
+            newDelegate.addPageAction(pageActionMessage);
+        }
+        mPendingPageActionQueue.clear();
+
+        mPageActionDelegate = newDelegate;
+    }
+
+    /**
+     * Clears the current PageActionDelegate and optionally takes a list of PageActions
+     * that will be stored, e.g. if the class that provided the delegate is going away.
+     *
+     * In addition, all PageAction EventDispatcher messages that arrive while no delegate is
+     * available will be stored for later retrieval.
+     */
+    public void removePageActionLayoutDelegate(final @Nullable List<PageAction> pageActionsToCache) {
+        mPageActionDelegate = null;
+        mResolvedPageActionCache = pageActionsToCache;
+    }
+
+    /**
      * Starts handling add-on menu items for the given {@link Menu} and also adds any
      * menu items that have already been cached.
      */
     public void onCreateOptionsMenu(Menu menu) {
         mMenu = menu;
 
         // Add add-on menu items, if any exist.
         if (mMenu != null) {
@@ -273,9 +342,10 @@ public class AddonUICache implements Bun
                 if (parent != null) {
                     return parent;
                 }
             }
         }
 
         return null;
     }
+
 }
--- a/mobile/android/base/java/org/mozilla/gecko/toolbar/PageActionLayout.java
+++ b/mobile/android/base/java/org/mozilla/gecko/toolbar/PageActionLayout.java
@@ -1,26 +1,25 @@
 /* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.toolbar;
 
+import org.mozilla.gecko.AddonUICache;
 import org.mozilla.gecko.EventDispatcher;
 import org.mozilla.gecko.GeckoSharedPrefs;
 import org.mozilla.gecko.R;
 import org.mozilla.gecko.Tab;
 import org.mozilla.gecko.Tabs;
 import org.mozilla.gecko.preferences.GeckoPreferences;
 import org.mozilla.gecko.pwa.PwaUtils;
 import org.mozilla.gecko.util.DrawableUtil;
 import org.mozilla.gecko.util.ResourceDrawableUtils;
-import org.mozilla.gecko.util.BundleEventListener;
-import org.mozilla.gecko.util.EventCallback;
 import org.mozilla.gecko.util.GeckoBundle;
 import org.mozilla.gecko.util.ShortcutUtils;
 import org.mozilla.gecko.util.ThreadUtils;
 import org.mozilla.gecko.widget.GeckoPopupMenu;
 import org.mozilla.gecko.widget.themed.ThemedImageButton;
 import org.mozilla.gecko.widget.themed.ThemedLinearLayout;
 
 import android.content.Context;
@@ -40,58 +39,87 @@ import android.widget.LinearLayout;
 
 import java.util.Iterator;
 import java.util.List;
 import java.util.UUID;
 import java.util.ArrayList;
 
 import static org.mozilla.gecko.toolbar.PageActionLayout.PageAction.UUID_PAGE_ACTION_PWA;
 
-public class PageActionLayout extends ThemedLinearLayout implements BundleEventListener,
-        View.OnClickListener,
-        View.OnLongClickListener {
+public class PageActionLayout extends ThemedLinearLayout
+        implements View.OnClickListener, View.OnLongClickListener {
     private static final String MENU_BUTTON_KEY = "MENU_BUTTON_KEY";
     private static final int DEFAULT_PAGE_ACTIONS_SHOWN = 2;
     public static final String PREF_PWA_ONBOARDING = GeckoPreferences.NON_PREF_PREFIX + "pref_pwa_onboarding";
 
+    public interface PageActionLayoutDelegate {
+        void addPageAction(GeckoBundle message);
+        void removePageAction(GeckoBundle message);
+        void setCachedPageActions(List<PageAction> cachedPageActions);
+    }
 
     private final Context mContext;
     private final LinearLayout mLayout;
-    private final List<PageAction> mPageActionList;
+    private List<PageAction> mPageActionList;
 
     private GeckoPopupMenu mPageActionsMenu;
 
     // By default it's two, can be changed by calling setNumberShown(int)
     private int mMaxVisiblePageActions;
     private PwaConfirm mPwaConfirm;
 
     public PageActionLayout(Context context, AttributeSet attrs) {
         super(context, attrs);
         mContext = context;
         mLayout = this;
-
-        mPageActionList = new ArrayList<PageAction>();
-        setNumberShown(DEFAULT_PAGE_ACTIONS_SHOWN);
-        refreshPageActionIcons();
     }
 
     @Override
     protected void onAttachedToWindow() {
         super.onAttachedToWindow();
 
-        EventDispatcher.getInstance().registerUiThreadListener(this,
-                "PageActions:Add",
-                "PageActions:Remove");
+        // Calling this will cause the AddonUICache to synchronously call addPageAction for all
+        // PageAction messages it received while no PageActionLayout was available.
+        // Processing those messages entails converting a data: URI into a drawable, which can take
+        // a few ms per message and therefore in theory cause some jank.
+        // In practice however, PageAction messages are commonly only generated when tabs (from the
+        // BrowserApp UI) are actually loading, so the AddonUICache's message queueing mechanism
+        // only becomes relevant if the BrowserApp UI has then been backgrounded *and* subsequently
+        // destroyed by the OS while some tabs are still loading. Merely starting up Gecko through a
+        // GeckoView-based activity on the other hand is not enough to queue up PageAction messages.
+        // Therefore, this case should happen rarely enough that we can tolerate it without having
+        // to think about moving the image processing into some asynchronous code path.
+        AddonUICache.getInstance().setPageActionLayoutDelegate(new PageActionLayoutDelegate() {
+            @Override
+            public void addPageAction(final GeckoBundle message) {
+                onAddPageAction(message);
+            }
+
+            @Override
+            public void removePageAction(final GeckoBundle message) {
+                onRemovePageAction(message);
+            }
+
+            @Override
+            public void setCachedPageActions(final List<PageAction> cachedPageActions) {
+                if (cachedPageActions != null) {
+                    mPageActionList = cachedPageActions;
+                } else {
+                    mPageActionList = new ArrayList<>();
+                }
+
+                setNumberShown(DEFAULT_PAGE_ACTIONS_SHOWN);
+                refreshPageActionIcons();
+            }
+        });
     }
 
     @Override
     protected void onDetachedFromWindow() {
-        EventDispatcher.getInstance().unregisterUiThreadListener(this,
-                "PageActions:Add",
-                "PageActions:Remove");
+        AddonUICache.getInstance().removePageActionLayoutDelegate(mPageActionList);
 
         super.onDetachedFromWindow();
     }
 
     @Override
     public void setPrivateMode(boolean isPrivate) {
         super.setPrivateMode(isPrivate);
         for (int i = 0; i < getChildCount(); i++) {
@@ -109,62 +137,62 @@ public class PageActionLayout extends Th
 
         for (int index = 0; index < count; index++) {
             if ((getChildCount() - 1) < index) {
                 mLayout.addView(createImageButton());
             }
         }
     }
 
-    @Override // BundleEventListener
-    public void handleMessage(final String event, final GeckoBundle message,
-                              final EventCallback callback) {
+    private void onAddPageAction(final GeckoBundle message) {
         ThreadUtils.assertOnUiThread();
 
         hidePreviousConfirmPrompt();
 
-        if ("PageActions:Add".equals(event)) {
-            final String id = message.getString("id");
+        final String id = message.getString("id");
+
+        boolean alreadyAdded = isPwaAdded(id);
+        if (alreadyAdded) {
+            return;
+        }
+
+        maybeShowPwaOnboarding(id);
 
-            boolean alreadyAdded = isPwaAdded(id);
-            if (alreadyAdded) {
-                return;
+        final String title = message.getString("title");
+        final String imageURL = message.getString("icon");
+        final boolean important = message.getBoolean("important");
+        final boolean useTint = message.getBoolean("useTint");
+
+        addPageAction(id, title, imageURL, useTint, new OnPageActionClickListeners() {
+            @Override
+            public void onClick(final String id) {
+                if (UUID_PAGE_ACTION_PWA.equals(id)) {
+                    mPwaConfirm = PwaConfirm.show(getContext());
+                    return;
+                }
+                final GeckoBundle data = new GeckoBundle(1);
+                data.putString("id", id);
+                EventDispatcher.getInstance().dispatch("PageActions:Clicked", data);
             }
 
-            maybeShowPwaOnboarding(id);
-
-            final String title = message.getString("title");
-            final String imageURL = message.getString("icon");
-            final boolean important = message.getBoolean("important");
-            final boolean useTint = message.getBoolean("useTint");
+            @Override
+            public boolean onLongClick(String id) {
+                final GeckoBundle data = new GeckoBundle(1);
+                data.putString("id", id);
+                EventDispatcher.getInstance().dispatch("PageActions:LongClicked", data);
+                return true;
+            }
+        }, important);
+    }
 
-            addPageAction(id, title, imageURL, useTint, new OnPageActionClickListeners() {
-                @Override
-                public void onClick(final String id) {
-                    if (UUID_PAGE_ACTION_PWA.equals(id)) {
-                        mPwaConfirm = PwaConfirm.show(getContext());
-                        return;
-                    }
-                    final GeckoBundle data = new GeckoBundle(1);
-                    data.putString("id", id);
-                    EventDispatcher.getInstance().dispatch("PageActions:Clicked", data);
-                }
+    private void onRemovePageAction(final GeckoBundle message) {
+        ThreadUtils.assertOnUiThread();
 
-                @Override
-                public boolean onLongClick(String id) {
-                    final GeckoBundle data = new GeckoBundle(1);
-                    data.putString("id", id);
-                    EventDispatcher.getInstance().dispatch("PageActions:LongClicked", data);
-                    return true;
-                }
-            }, important);
-
-        } else if ("PageActions:Remove".equals(event)) {
-            removePageAction(message.getString("id"));
-        }
+        hidePreviousConfirmPrompt();
+        removePageAction(message.getString("id"));
     }
 
     private void maybeShowPwaOnboarding(String id) {
         // only show pwa at normal mode
         final Tab selectedTab = Tabs.getInstance().getSelectedTab();
         if (!PwaUtils.shouldAddPwaShortcut(selectedTab)) {
             return;
         }