From ae49ab84f8cbe146370eca2d7b857ce7a480e827 Mon Sep 17 00:00:00 2001 From: Brian Nicholson Date: Tue, 15 Jul 2014 11:19:50 -0700 Subject: [PATCH] Bug 1027137 - PageActionLayout cleanup. r=wesj --- .../base/toolbar/PageActionLayout.java | 72 ++++++++++--------- 1 file changed, 40 insertions(+), 32 deletions(-) diff --git a/mobile/android/base/toolbar/PageActionLayout.java b/mobile/android/base/toolbar/PageActionLayout.java index e85e2b96d0b..83bccea22c6 100644 --- a/mobile/android/base/toolbar/PageActionLayout.java +++ b/mobile/android/base/toolbar/PageActionLayout.java @@ -15,6 +15,7 @@ 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; @@ -27,6 +28,8 @@ 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; @@ -36,10 +39,11 @@ public class PageActionLayout extends LinearLayout implements NativeEventListene private static final String MENU_BUTTON_KEY = "MENU_BUTTON_KEY"; private static final int DEFAULT_PAGE_ACTIONS_SHOWN = 2; - private ArrayList mPageActionList; + private final Context mContext; + private final LinearLayout mLayout; + private final List 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; @@ -63,8 +67,8 @@ public class PageActionLayout extends LinearLayout implements NativeEventListene 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()); } } @@ -117,14 +121,15 @@ public class PageActionLayout extends LinearLayout implements NativeEventListene } } - 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); @@ -142,9 +147,11 @@ public class PageActionLayout extends LinearLayout implements NativeEventListene 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 iter = mPageActionList.iterator(); + while (iter.hasNext()) { + final PageAction pageAction = iter.next(); + if (pageAction.getID().equals(id)) { + iter.remove(); refreshPageActionIcons(); return; } @@ -154,8 +161,9 @@ public class PageActionLayout extends LinearLayout implements NativeEventListene 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); @@ -186,6 +194,8 @@ public class PageActionLayout extends LinearLayout implements NativeEventListene } private void setActionForView(final ImageButton view, final PageAction pageAction) { + ThreadUtils.assertOnUiThread(); + if (pageAction == null) { view.setTag(null); view.setImageDrawable(null); @@ -204,12 +214,13 @@ public class PageActionLayout extends LinearLayout implements NativeEventListene 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); @@ -233,12 +244,12 @@ public class PageActionLayout extends LinearLayout implements NativeEventListene * 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; } @@ -246,8 +257,7 @@ public class PageActionLayout extends LinearLayout implements NativeEventListene 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; } @@ -255,17 +265,17 @@ public class PageActionLayout extends LinearLayout implements NativeEventListene 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(); @@ -279,12 +289,10 @@ public class PageActionLayout extends LinearLayout implements NativeEventListene 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(); }