Bug 884069 - Refactor DoorHanger to create a better Java API that doesn't depend on JSON. r=sriram,wesj
authorMargaret Leibovic <margaret.leibovic@gmail.com>
Fri, 21 Jun 2013 20:36:35 -0700
changeset 147564 4f98cdea7ea41b8295df48a0ea68041dbc5c2e2f
parent 147563 0a7b5696c49afa31bb6ff9aed758a6512a7c1f39
child 147565 e9b1e686b98389ccd7e5e08da025efd76b49d8c3
push id2697
push userbbajaj@mozilla.com
push dateMon, 05 Aug 2013 18:49:53 +0000
treeherdermozilla-beta@dfec938c7b63 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssriram, wesj
bugs884069
milestone24.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 884069 - Refactor DoorHanger to create a better Java API that doesn't depend on JSON. r=sriram,wesj
mobile/android/base/DoorHanger.java
mobile/android/base/DoorHangerPopup.java
--- a/mobile/android/base/DoorHanger.java
+++ b/mobile/android/base/DoorHanger.java
@@ -10,16 +10,17 @@ import org.mozilla.gecko.widget.Divider;
 import org.json.JSONArray;
 import org.json.JSONException;
 import org.json.JSONObject;
 
 import android.content.Context;
 import android.graphics.Rect;
 import android.os.Build;
 import android.text.SpannableString;
+import android.text.TextUtils;
 import android.text.method.LinkMovementMethod;
 import android.text.style.ForegroundColorSpan;
 import android.text.style.URLSpan;
 import android.util.Log;
 import android.view.LayoutInflater;
 import android.view.View;
 import android.view.ViewGroup;
 import android.widget.Button;
@@ -27,199 +28,174 @@ import android.widget.CheckBox;
 import android.widget.LinearLayout;
 import android.widget.Spinner;
 import android.widget.SpinnerAdapter;
 import android.widget.TextView;
 
 import java.util.ArrayList;
 import java.util.List;
 
-public class DoorHanger extends LinearLayout implements Button.OnClickListener {
+public class DoorHanger extends LinearLayout {
     private static final String LOGTAG = "GeckoDoorHanger";
 
-    // The popup that holds this doorhanger
-    private DoorHangerPopup mPopup;
-    private LinearLayout mChoicesLayout;
-    private TextView mTextView;
-    private List<PromptInput> mInputs;
-
     private static int sInputPadding = -1;
     private static int sSpinnerTextColor = -1;
     private static int sSpinnerTextSize = -1;
 
-    // LayoutParams used for adding button layouts
-    static private LayoutParams mLayoutParams;
+    private static LayoutParams sButtonParams;
+    static {
+        sButtonParams = new LayoutParams(LayoutParams.FILL_PARENT, LayoutParams.FILL_PARENT, 1.0f);
+    }
+
+    private final TextView mTextView;
+    private final LinearLayout mChoicesLayout;
+
+    // Divider between doorhangers.
+    private final View mDivider;
+
+    // The tab associated with this notification.
     private final int mTabId;
-    // Value used to identify the notification
+
+    // Value used to identify the notification.
     private final String mValue;
 
-    // Optional checkbox added underneath message text
+    private List<PromptInput> mInputs;
     private CheckBox mCheckBox;
 
-    // Divider between doorhangers.
-    private View mDivider;
-
     private int mPersistence = 0;
     private boolean mPersistWhileVisible = false;
     private long mTimeout = 0;
 
-    DoorHanger(Context context, DoorHangerPopup popup, int tabId, String value) {
+    public interface OnButtonClickListener {
+        public void onButtonClick(DoorHanger dh, String tag);
+    }
+
+    DoorHanger(Context context) {
+        this(context, 0, null);
+    }
+
+    DoorHanger(Context context, int tabId, String value) {
         super(context);
 
-        mPopup = popup;
         mTabId = tabId;
         mValue = value;
 
         if (sInputPadding == -1) {
             sInputPadding = getResources().getDimensionPixelSize(R.dimen.doorhanger_padding);
         }
         if (sSpinnerTextColor == -1) {
             sSpinnerTextColor = getResources().getColor(R.color.text_color_primary_disable_only);
         }
         if (sSpinnerTextSize == -1) {
             sSpinnerTextSize = getResources().getDimensionPixelSize(R.dimen.doorhanger_spinner_textsize);
         }
+
+        setOrientation(VERTICAL);
+
+        LayoutInflater.from(context).inflate(R.layout.doorhanger, this);
+        mTextView = (TextView) findViewById(R.id.doorhanger_title);
+        mChoicesLayout = (LinearLayout) findViewById(R.id.doorhanger_choices);
+        mDivider = findViewById(R.id.divider_doorhanger);
     }
- 
+
     int getTabId() {
         return mTabId;
     }
 
     String getValue() {
         return mValue;
     }
 
-    public void showDivider() {
+    List<PromptInput> getInputs() {
+        return mInputs;
+    }
+
+    CheckBox getCheckBox() {
+        return mCheckBox;
+    }
+
+    void showDivider() {
         mDivider.setVisibility(View.VISIBLE);
     }
 
-    public void hideDivider() {
+    void hideDivider() {
         mDivider.setVisibility(View.GONE);
     }
 
-    // Postpone stuff that needs to be done on the main thread
-    void init(String message, JSONArray buttons, JSONObject options) {
-        setOrientation(VERTICAL);
-
-        LayoutInflater.from(getContext()).inflate(R.layout.doorhanger, this);
-        setVisibility(View.GONE);
-
-        mTextView = (TextView) findViewById(R.id.doorhanger_title);
+    void setMessage(String message) {
         mTextView.setText(message);
-
-        mChoicesLayout = (LinearLayout) findViewById(R.id.doorhanger_choices);
-
-        mDivider = findViewById(R.id.divider_doorhanger);
-
-        // Set the doorhanger text and buttons
-        for (int i = 0; i < buttons.length(); i++) {
-            try {
-                JSONObject buttonObject = buttons.getJSONObject(i);
-                String label = buttonObject.getString("label");
-                int callBackId = buttonObject.getInt("callback");
-                addButton(label, callBackId);
-            } catch (JSONException e) {
-                Log.e(LOGTAG, "Error creating doorhanger button", e);
-            }
-         }
-
-        // Enable the button layout if we have buttons.
-        if (buttons.length() > 0) {
-            findViewById(R.id.divider_choices).setVisibility(View.VISIBLE);
-            mChoicesLayout.setVisibility(View.VISIBLE);
-        }
-
-        setOptions(options);
     }
 
-    private void addButton(String aText, int aCallback) {
-        if (mLayoutParams == null)
-            mLayoutParams = new LayoutParams(LayoutParams.FILL_PARENT,
-                                             LayoutParams.FILL_PARENT,
-                                             1.0f);
+    void addLink(String label, String url, String delimiter) {
+        String title = mTextView.getText().toString();
+        SpannableString titleWithLink = new SpannableString(title + delimiter + label);
+        URLSpan linkSpan = new URLSpan(url) {
+            @Override
+            public void onClick(View view) {
+                Tabs.getInstance().loadUrlInTab(getURL());
+            }
+        };
+
+        // Prevent text outside the link from flashing when clicked.
+        ForegroundColorSpan colorSpan = new ForegroundColorSpan(mTextView.getCurrentTextColor());
+        titleWithLink.setSpan(colorSpan, 0, title.length(), 0);
+
+        titleWithLink.setSpan(linkSpan, title.length() + 1, titleWithLink.length(), 0);
+        mTextView.setText(titleWithLink);
+        mTextView.setMovementMethod(LinkMovementMethod.getInstance());
+    }
 
-        Button button = (Button) LayoutInflater.from(getContext()).inflate(R.layout.doorhanger_button, null);
-        button.setText(aText);
-        button.setTag(Integer.toString(aCallback));
-        button.setOnClickListener(this);
+    void addButton(final String text, final String tag, final OnButtonClickListener listener) {
+        final Button button = (Button) LayoutInflater.from(getContext()).inflate(R.layout.doorhanger_button, null);
+        button.setText(text);
+        button.setTag(tag);
 
-        if (mChoicesLayout.getChildCount() > 0) {
+        button.setOnClickListener(new Button.OnClickListener() {
+            @Override
+            public void onClick(View v) {
+                listener.onButtonClick(DoorHanger.this, tag);
+            }
+        });
+
+        if (mChoicesLayout.getChildCount() == 0) {
+            // If this is the first button we're adding, make the choices layout visible.
+            mChoicesLayout.setVisibility(View.VISIBLE);
+            findViewById(R.id.divider_choices).setVisibility(View.VISIBLE);
+        } else {
+            // Add a divider for additional buttons.
             Divider divider = new Divider(getContext(), null);
             divider.setOrientation(Divider.Orientation.VERTICAL);
             divider.setBackgroundColor(0xFFD1D5DA);
             mChoicesLayout.addView(divider);
         }
 
-        mChoicesLayout.addView(button, mLayoutParams);
-    }
-
-    @Override
-    public void onClick(View v) {
-        JSONObject response = new JSONObject();
-        try {
-            response.put("callback", v.getTag().toString());
-
-            // If the checkbox is being used, pass its value
-            if (mCheckBox != null)
-                response.put("checked", mCheckBox.isChecked());
-
-            if (mInputs != null) {
-                JSONObject inputs = new JSONObject();
-                for (PromptInput input : mInputs) {
-                    inputs.put(input.getId(), input.getValue());
-                }
-                response.put("inputs", inputs);
-            }
-        } catch (JSONException e) {
-            Log.e(LOGTAG, "Error creating onClick response", e);
-        }
-
-        GeckoEvent e = GeckoEvent.createBroadcastEvent("Doorhanger:Reply", response.toString());
-        GeckoAppShell.sendEventToGecko(e);
-        mPopup.removeDoorHanger(this);
-
-        // This will hide the doorhanger (and hide the popup if there are no
-        // more doorhangers to show)
-        mPopup.updatePopup();
+        mChoicesLayout.addView(button, sButtonParams);
     }
 
-    private void setOptions(final JSONObject options) {
-        try {
-            mPersistence = options.getInt("persistence");
-        } catch (JSONException e) { }
+    void setOptions(final JSONObject options) {
+        final int persistence = options.optInt("persistence");
+        if (persistence > 0) {
+            mPersistence = persistence;
+        }
 
-        try {
-            mPersistWhileVisible = options.getBoolean("persistWhileVisible");
-        } catch (JSONException e) { }
-
-        try {
-            mTimeout = options.getLong("timeout");
-        } catch (JSONException e) { }
+        mPersistWhileVisible = options.optBoolean("persistWhileVisible");
 
-        try {
-            JSONObject link = options.getJSONObject("link");
-            String title = mTextView.getText().toString();
-            String linkLabel = link.getString("label");
-            String linkUrl = link.getString("url");
-            SpannableString titleWithLink = new SpannableString(title + " " + linkLabel);
-            URLSpan linkSpan = new URLSpan(linkUrl) {
-                @Override
-                public void onClick(View view) {
-                    Tabs.getInstance().loadUrlInTab(this.getURL());
-                }
-            };
+        final long timeout = options.optLong("timeout");
+        if (timeout > 0) {
+            mTimeout = timeout;
+        }
 
-            // prevent text outside the link from flashing when clicked
-            ForegroundColorSpan colorSpan = new ForegroundColorSpan(mTextView.getCurrentTextColor());
-            titleWithLink.setSpan(colorSpan, 0, title.length(), 0);
-
-            titleWithLink.setSpan(linkSpan, title.length() + 1, titleWithLink.length(), 0);
-            mTextView.setText(titleWithLink);
-            mTextView.setMovementMethod(LinkMovementMethod.getInstance());
-        } catch (JSONException e) { }
+        final JSONObject link = options.optJSONObject("link");
+        if (link != null) {
+            try {
+                final String linkLabel = link.getString("label");
+                final String linkUrl = link.getString("url");
+                addLink(linkLabel, linkUrl, " ");
+            } catch (JSONException e) { }
+        }
 
         final JSONArray inputs = options.optJSONArray("inputs");
         if (inputs != null) {
             mInputs = new ArrayList<PromptInput>();
 
             final ViewGroup group = (ViewGroup) findViewById(R.id.doorhanger_inputs);
             group.setVisibility(VISIBLE);
 
@@ -230,22 +206,22 @@ public class DoorHanger extends LinearLa
 
                     View v = input.getView(getContext());
                     styleInput(input, v);
                     group.addView(v);
                 } catch(JSONException ex) { }
             }
         }
 
-        try {
-            String checkBoxText = options.getString("checkbox");
+        final String checkBoxText = options.optString("checkbox");
+        if (!TextUtils.isEmpty(checkBoxText)) {
             mCheckBox = (CheckBox) findViewById(R.id.doorhanger_checkbox);
             mCheckBox.setText(checkBoxText);
             mCheckBox.setVisibility(VISIBLE);
-        } catch (JSONException e) { }
+        }
     }
 
     private void styleInput(PromptInput input, View view) {
         if (input instanceof PromptInput.MenulistInput) {
             styleSpinner(input, view);
         } else {
             // add some top and bottom padding to separate inputs
             view.setPadding(0, sInputPadding,
@@ -290,20 +266,25 @@ public class DoorHanger extends LinearLa
             spinInput.textView.setTextColor(sSpinnerTextColor);
             spinInput.textView.setTextSize(sSpinnerTextSize);
 
             // If this spinner has a label, offset it to also be aligned with the doorhanger text.
             spinInput.textView.setPadding(rect.left + textPadding, 0, 0, 0);
         }
     }
 
-    // This method checks with persistence and timeout options to see if
-    // it's okay to remove a doorhanger.
-    boolean shouldRemove() {
-        if (mPersistWhileVisible && mPopup.isShowing()) {
+
+    /*
+     * Checks with persistence and timeout options to see if it's okay to remove a doorhanger.
+     *
+     * @param isShowing Whether or not this doorhanger is currently visible to the user.
+     *                 (e.g. the DoorHanger view might be VISIBLE, but its parent could be hidden)
+     */
+    boolean shouldRemove(boolean isShowing) {
+        if (mPersistWhileVisible && isShowing) {
             // We still want to decrement mPersistence, even if the popup is showing
             if (mPersistence != 0)
                 mPersistence--;
             return false;
         }
 
         // If persistence is set to -1, the doorhanger will never be
         // automatically removed.
--- a/mobile/android/base/DoorHangerPopup.java
+++ b/mobile/android/base/DoorHangerPopup.java
@@ -4,34 +4,39 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko;
 
 import org.mozilla.gecko.util.GeckoEventListener;
 import org.mozilla.gecko.widget.ArrowPopup;
 
 import org.json.JSONArray;
+import org.json.JSONException;
 import org.json.JSONObject;
 
 import android.os.Build;
 import android.util.Log;
 import android.view.View;
+import android.widget.CheckBox;
 
 import java.util.HashSet;
+import java.util.List;
 
 public class DoorHangerPopup extends ArrowPopup
-                             implements GeckoEventListener, Tabs.OnTabsChangedListener {
+                             implements GeckoEventListener,
+                                        Tabs.OnTabsChangedListener,
+                                        DoorHanger.OnButtonClickListener {
     private static final String LOGTAG = "GeckoDoorHangerPopup";
 
     // Stores a set of all active DoorHanger notifications. A DoorHanger is
     // uniquely identified by its tabId and value.
     private HashSet<DoorHanger> mDoorHangers;
 
-    DoorHangerPopup(GeckoApp aActivity, View aAnchor) {
-        super(aActivity, aAnchor);
+    DoorHangerPopup(GeckoApp activity, View anchor) {
+        super(activity, anchor);
 
         mDoorHangers = new HashSet<DoorHanger>();
 
         registerEventListener("Doorhanger:Add");
         registerEventListener("Doorhanger:Remove");
         Tabs.registerOnTabsChangedListener(this);
     }
 
@@ -116,38 +121,87 @@ public class DoorHangerPopup extends Arr
     /**
      * Adds a doorhanger.
      *
      * This method must be called on the UI thread.
      */
     void addDoorHanger(final int tabId, final String value, final String message,
                        final JSONArray buttons, final JSONObject options) {
         // Don't add a doorhanger for a tab that doesn't exist
-        if (Tabs.getInstance().getTab(tabId) == null)
+        if (Tabs.getInstance().getTab(tabId) == null) {
             return;
+        }
 
         // Replace the doorhanger if it already exists
         DoorHanger oldDoorHanger = getDoorHanger(tabId, value);
-        if (oldDoorHanger != null)
+        if (oldDoorHanger != null) {
             removeDoorHanger(oldDoorHanger);
+        }
 
-        final DoorHanger newDoorHanger = new DoorHanger(mActivity, this, tabId, value);
+        if (!mInflated) {
+            init();
+        }
+
+        final DoorHanger newDoorHanger = new DoorHanger(mActivity, tabId, value);
+        newDoorHanger.setMessage(message);
+        newDoorHanger.setOptions(options);
+
+        for (int i = 0; i < buttons.length(); i++) {
+            try {
+                JSONObject buttonObject = buttons.getJSONObject(i);
+                String label = buttonObject.getString("label");
+                String tag = String.valueOf(buttonObject.getInt("callback"));
+                newDoorHanger.addButton(label, tag, this);
+            } catch (JSONException e) {
+                Log.e(LOGTAG, "Error creating doorhanger button", e);
+            }
+        }
+
         mDoorHangers.add(newDoorHanger);
-
-        if (!mInflated)
-            init();
-
-        newDoorHanger.init(message, buttons, options);
         mContent.addView(newDoorHanger);
 
         // Only update the popup if we're adding a notifcation to the selected tab
         if (tabId == Tabs.getInstance().getSelectedTab().getId())
             updatePopup();
     }
 
+
+    /*
+     * DoorHanger.OnButtonClickListener implementation
+     */
+    @Override
+    public void onButtonClick(DoorHanger dh, String tag) {
+        JSONObject response = new JSONObject();
+        try {
+            response.put("callback", tag);
+
+            CheckBox checkBox = dh.getCheckBox();
+            // If the checkbox is being used, pass its value
+            if (checkBox != null) {
+                response.put("checked", checkBox.isChecked());
+            }
+
+            List<PromptInput> doorHangerInputs = dh.getInputs();
+            if (doorHangerInputs != null) {
+                JSONObject inputs = new JSONObject();
+                for (PromptInput input : doorHangerInputs) {
+                    inputs.put(input.getId(), input.getValue());
+                }
+                response.put("inputs", inputs);
+            }
+        } catch (JSONException e) {
+            Log.e(LOGTAG, "Error creating onClick response", e);
+        }
+
+        GeckoEvent e = GeckoEvent.createBroadcastEvent("Doorhanger:Reply", response.toString());
+        GeckoAppShell.sendEventToGecko(e);
+        removeDoorHanger(dh);
+        updatePopup();
+    }
+
     /**
      * Gets a doorhanger.
      *
      * This method must be called on the UI thread.
      */
     DoorHanger getDoorHanger(int tabId, String value) {
         for (DoorHanger dh : mDoorHangers) {
             if (dh.getTabId() == tabId && dh.getValue().equals(value))
@@ -173,17 +227,17 @@ public class DoorHangerPopup extends Arr
      *
      * This method must be called on the UI thread.
      */
     void removeTransientDoorHangers(int tabId) {
         // Make a temporary set to avoid a ConcurrentModificationException
         HashSet<DoorHanger> doorHangersToRemove = new HashSet<DoorHanger>();
         for (DoorHanger dh : mDoorHangers) {
             // Only remove transient doorhangers for the given tab
-            if (dh.getTabId() == tabId && dh.shouldRemove())
+            if (dh.getTabId() == tabId && dh.shouldRemove(isShowing()))
                 doorHangersToRemove.add(dh);
         }
 
         for (DoorHanger dh : doorHangersToRemove) {
             removeDoorHanger(dh);
         }
     }