Bug 1027137 - PageActionLayout cleanup. r=wesj
authorBrian Nicholson <bnicholson@mozilla.com>
Tue, 15 Jul 2014 11:19:50 -0700
changeset 216122 6b76b46afe7acf3871da508bd8b8e3f0b85831e0
parent 216121 c4185cf15dfd87b0bd98bc07063ed9b7f7a879c5
child 216123 e9d78c3d2eb584b74266b6579cbb1e5ee45005fd
push id515
push userraliiev@mozilla.com
push dateMon, 06 Oct 2014 12:51:51 +0000
treeherdermozilla-release@267c7a481bef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerswesj
bugs1027137
milestone33.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1027137 - PageActionLayout cleanup. r=wesj
mobile/android/base/toolbar/PageActionLayout.java
--- a/mobile/android/base/toolbar/PageActionLayout.java
+++ b/mobile/android/base/toolbar/PageActionLayout.java
@@ -10,41 +10,45 @@ import org.mozilla.gecko.GeckoAppShell;
 import org.mozilla.gecko.GeckoEvent;
 import org.mozilla.gecko.R;
 import org.mozilla.gecko.gfx.BitmapUtils;
 import org.mozilla.gecko.util.EventCallback;
 import org.mozilla.gecko.util.NativeEventListener;
 import org.mozilla.gecko.util.NativeJSObject;
 import org.mozilla.gecko.util.ThreadUtils;
 import org.mozilla.gecko.widget.GeckoPopupMenu;
+
 import android.content.Context;
 import android.content.res.Resources;
 import android.graphics.drawable.Drawable;
 import android.os.Bundle;
 import android.util.AttributeSet;
 import android.view.Menu;
 import android.view.MenuItem;
 import android.view.View;
 import android.widget.ImageButton;
 import android.widget.ImageView;
 import android.widget.LinearLayout;
 
+import java.util.Iterator;
+import java.util.List;
 import java.util.UUID;
 import java.util.ArrayList;
 
 public class PageActionLayout extends LinearLayout implements NativeEventListener,
                                                               View.OnClickListener,
                                                               View.OnLongClickListener {
     private static final String MENU_BUTTON_KEY = "MENU_BUTTON_KEY";
     private static final int DEFAULT_PAGE_ACTIONS_SHOWN = 2;
 
-    private ArrayList<PageAction> mPageActionList;
+    private final Context mContext;
+    private final LinearLayout mLayout;
+    private final List<PageAction> mPageActionList;
+
     private GeckoPopupMenu mPageActionsMenu;
-    private Context mContext;
-    private LinearLayout mLayout;
 
     // By default it's two, can be changed by calling setNumberShown(int)
     private int mMaxVisiblePageActions;
 
     public PageActionLayout(Context context, AttributeSet attrs) {
         super(context, attrs);
         mContext = context;
         mLayout = this;
@@ -58,18 +62,18 @@ public class PageActionLayout extends Li
             "PageActions:Remove");
     }
 
     private void setNumberShown(int count) {
         ThreadUtils.assertOnUiThread();
 
         mMaxVisiblePageActions = count;
 
-        for(int index = 0; index < count; index++) {
-            if ((this.getChildCount() - 1) < index) {
+        for (int index = 0; index < count; index++) {
+            if ((getChildCount() - 1) < index) {
                 mLayout.addView(createImageButton());
             }
         }
     }
 
     public void onDestroy() {
         EventDispatcher.getInstance().unregisterGeckoThreadListener(this,
             "PageActions:Add",
@@ -112,24 +116,25 @@ public class PageActionLayout extends Li
             }, important);
         } else if (event.equals("PageActions:Remove")) {
             final String id = message.getString("id");
 
             removePageAction(id);
         }
     }
 
-    private void addPageAction(final String id, final String title, final String imageData, final OnPageActionClickListeners mOnPageActionClickListeners, boolean mImportant) {
+    private void addPageAction(final String id, final String title, final String imageData,
+            final OnPageActionClickListeners onPageActionClickListeners, boolean important) {
         ThreadUtils.assertOnUiThread();
 
-        final PageAction pageAction = new PageAction(id, title, null, mOnPageActionClickListeners, mImportant);
+        final PageAction pageAction = new PageAction(id, title, null, onPageActionClickListeners, important);
 
         int insertAt = mPageActionList.size();
-        while(insertAt > 0 && mPageActionList.get(insertAt-1).isImportant()) {
-          insertAt--;
+        while (insertAt > 0 && mPageActionList.get(insertAt - 1).isImportant()) {
+            insertAt--;
         }
         mPageActionList.add(insertAt, pageAction);
 
         BitmapUtils.getDrawable(mContext, imageData, new BitmapUtils.BitmapLoader() {
             @Override
             public void onBitmapFound(final Drawable d) {
                 if (mPageActionList.contains(pageAction)) {
                     pageAction.setDrawable(d);
@@ -137,30 +142,33 @@ public class PageActionLayout extends Li
                 }
             }
         });
     }
 
     private void removePageAction(String id) {
         ThreadUtils.assertOnUiThread();
 
-        for(int i = 0; i < mPageActionList.size(); i++) {
-            if (mPageActionList.get(i).getID().equals(id)) {
-                mPageActionList.remove(i);
+        final Iterator<PageAction> iter = mPageActionList.iterator();
+        while (iter.hasNext()) {
+            final PageAction pageAction = iter.next();
+            if (pageAction.getID().equals(id)) {
+                iter.remove();
                 refreshPageActionIcons();
                 return;
             }
         }
     }
 
     private ImageButton createImageButton() {
         ThreadUtils.assertOnUiThread();
 
+        final int width = mContext.getResources().getDimensionPixelSize(R.dimen.page_action_button_width);
         ImageButton imageButton = new ImageButton(mContext, null, R.style.UrlBar_ImageButton_Icon);
-        imageButton.setLayoutParams(new LayoutParams(mContext.getResources().getDimensionPixelSize(R.dimen.page_action_button_width), LayoutParams.MATCH_PARENT));
+        imageButton.setLayoutParams(new LayoutParams(width, LayoutParams.MATCH_PARENT));
         imageButton.setScaleType(ImageView.ScaleType.CENTER_INSIDE);
         imageButton.setOnClickListener(this);
         imageButton.setOnLongClickListener(this);
         return imageButton;
     }
 
     @Override
     public void onClick(View v) {
@@ -181,16 +189,18 @@ public class PageActionLayout extends Li
             showMenu(v, mPageActionList.size() - mMaxVisiblePageActions + 1);
             return true;
         } else {
             return getPageActionWithId(buttonClickedId).onLongClick();
         }
     }
 
     private void setActionForView(final ImageButton view, final PageAction pageAction) {
+        ThreadUtils.assertOnUiThread();
+
         if (pageAction == null) {
             view.setTag(null);
             view.setImageDrawable(null);
             view.setVisibility(View.GONE);
             view.setContentDescription(null);
             return;
         }
 
@@ -199,22 +209,23 @@ public class PageActionLayout extends Li
         view.setVisibility(View.VISIBLE);
         view.setContentDescription(pageAction.getTitle());
     }
 
     private void refreshPageActionIcons() {
         ThreadUtils.assertOnUiThread();
 
         final Resources resources = mContext.getResources();
-        for(int index = 0; index < this.getChildCount(); index++) {
-            final ImageButton v = (ImageButton)this.getChildAt(index);
-            final PageAction pageAction = getPageActionForViewAt(index);
+        for (int i = 0; i < this.getChildCount(); i++) {
+            final ImageButton v = (ImageButton) this.getChildAt(i);
+            final PageAction pageAction = getPageActionForViewAt(i);
 
-            // If there are more pageactions then buttons, set the menu icon. Otherwise set the page action's icon if there is a page action.
-            if (index == (this.getChildCount() - 1) && mPageActionList.size() > mMaxVisiblePageActions) {
+            // If there are more page actions than buttons, set the menu icon.
+            // Otherwise, set the page action's icon if there is a page action.
+            if ((i == this.getChildCount() - 1) && (mPageActionList.size() > mMaxVisiblePageActions)) {
                 v.setTag(MENU_BUTTON_KEY);
                 v.setImageDrawable(resources.getDrawable(R.drawable.icon_pageaction));
                 v.setVisibility((pageAction != null) ? View.VISIBLE : View.GONE);
                 v.setContentDescription(resources.getString(R.string.page_action_dropmarker_description));
             } else {
                 setActionForView(v, pageAction);
             }
         }
@@ -228,68 +239,65 @@ public class PageActionLayout extends Li
          * Also, the order of the pageAction is important i.e. if a page action is added, instead of shifting the pagactions to the
          * left to make space for the new one, it would be more visually appealing to have the pageaction appear in the blank space.
          *
          * buttonIndex is needed for this reason because every new View added to PageActionLayout gets added to the right of its neighbouring View.
          * Hence the button on the very leftmost has the index 0. We want our pageactions to start from the rightmost
          * and hence we maintain the insertion order of the child Views which is essentially the reverse of their index
          */
 
-        int buttonIndex = (this.getChildCount() - 1) - index;
-        int totalVisibleButtons = ((mPageActionList.size() < this.getChildCount()) ? mPageActionList.size() : this.getChildCount());
+        final int buttonIndex = (this.getChildCount() - 1) - index;
 
         if (mPageActionList.size() > buttonIndex) {
             // Return the pageactions starting from the end of the list for the number of visible pageactions.
-            return mPageActionList.get((mPageActionList.size() - totalVisibleButtons) + buttonIndex);
+            final int buttonCount = Math.min(mPageActionList.size(), getChildCount());
+            return mPageActionList.get((mPageActionList.size() - buttonCount) + buttonIndex);
         }
         return null;
     }
 
     private PageAction getPageActionWithId(String id) {
         ThreadUtils.assertOnUiThread();
 
-        for(int i = 0; i < mPageActionList.size(); i++) {
-            PageAction pageAction = mPageActionList.get(i);
+        for (PageAction pageAction : mPageActionList) {
             if (pageAction.getID().equals(id)) {
                 return pageAction;
             }
         }
         return null;
     }
 
-    private void showMenu(View mPageActionButton, int toShow) {
+    private void showMenu(View pageActionButton, int toShow) {
         ThreadUtils.assertOnUiThread();
 
         if (mPageActionsMenu == null) {
-            mPageActionsMenu = new GeckoPopupMenu(mPageActionButton.getContext(), mPageActionButton);
+            mPageActionsMenu = new GeckoPopupMenu(pageActionButton.getContext(), pageActionButton);
             mPageActionsMenu.inflate(0);
             mPageActionsMenu.setOnMenuItemClickListener(new GeckoPopupMenu.OnMenuItemClickListener() {
                 @Override
                 public boolean onMenuItemClick(MenuItem item) {
                     int id = item.getItemId();
-                    for(int i = 0; i < mPageActionList.size(); i++) {
+                    for (int i = 0; i < mPageActionList.size(); i++) {
                         PageAction pageAction = mPageActionList.get(i);
                         if (pageAction.key() == id) {
                             pageAction.onClick();
                             return true;
                         }
                     }
                     return false;
                 }
             });
         }
         Menu menu = mPageActionsMenu.getMenu();
         menu.clear();
 
-        for(int i = 0; i < mPageActionList.size(); i++) {
-            if (i < toShow) {
-                PageAction pageAction = mPageActionList.get(i);
-                MenuItem item = menu.add(Menu.NONE, pageAction.key(), Menu.NONE, pageAction.getTitle());
-                item.setIcon(pageAction.getDrawable());
-            }
+        for (int i = 0; i < mPageActionList.size() && i < toShow; i++) {
+            PageAction pageAction = mPageActionList.get(i);
+            MenuItem item = menu.add(Menu.NONE, pageAction.key(), Menu.NONE, pageAction.getTitle());
+            item.setIcon(pageAction.getDrawable());
         }
         mPageActionsMenu.show();
     }
 
     private static interface OnPageActionClickListeners {
         public void onClick(String id);
         public boolean onLongClick(String id);
     }