Added a debug info UI for Blimp

blimp/client/app/android/java/src/org/chromium/blimp/toolbar/Toolbar.java

Index: blimp/client/app/android/java/src/org/chromium/blimp/toolbar/Toolbar.java
diff --git a/blimp/client/app/android/java/src/org/chromium/blimp/toolbar/Toolbar.java b/blimp/client/app/android/java/src/org/chromium/blimp/toolbar/Toolbar.java
index a6c188e7b07cd2f2f347ee155b73d929e70ac590..efa33565dfd1d5a6b3f6c5d58741f92b2679c2fe 100644
--- a/blimp/client/app/android/java/src/org/chromium/blimp/toolbar/Toolbar.java
+++ b/blimp/client/app/android/java/src/org/chromium/blimp/toolbar/Toolbar.java
@@ -4,51 +4,37 @@
 
 package org.chromium.blimp.toolbar;
 
-import android.content.ActivityNotFoundException;
 import android.content.Context;
-import android.content.Intent;
 import android.graphics.Bitmap;
-import android.net.Uri;
 import android.text.TextUtils;
 import android.util.AttributeSet;
 import android.view.View;
-import android.widget.AdapterView;
-import android.widget.AdapterView.OnItemClickListener;
-import android.widget.ArrayAdapter;
 import android.widget.ImageButton;
 import android.widget.LinearLayout;
-import android.widget.ListPopupWindow;
 import android.widget.ProgressBar;
 
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.blimp.R;
 import org.chromium.blimp.session.BlimpClientSession;
-import org.chromium.blimp.session.EngineInfo;
-import org.chromium.blimp.settings.AboutBlimpPreferences;
-import org.chromium.blimp.settings.Preferences;
 
 /**
  * A {@link View} that visually represents the Blimp toolbar, which lets users issue navigation
  * commands and displays relevant navigation UI.
  */
 @JNINamespace("blimp::client")
-public class Toolbar extends LinearLayout implements UrlBar.UrlBarObserver, View.OnClickListener,
-                                                     BlimpClientSession.ConnectionObserver {
+public class Toolbar extends LinearLayout implements UrlBar.UrlBarObserver, View.OnClickListener {
     private static final String TAG = "Toolbar";
-    private static final int ID_OPEN_IN_CHROME = 0;
-    private static final int ID_VERSION_INFO = 1;
 
     private long mNativeToolbarPtr;
 
     private Context mContext;
     private UrlBar mUrlBar;
+    private ToolbarMenu mToolbarMenu;
     private ImageButton mReloadButton;
     private ImageButton mMenuButton;
-    private ListPopupWindow mPopupMenu;
     private ProgressBar mProgressBar;
-
-    private EngineInfo mEngineInfo;
+    private BlimpClientSession mBlimpClientSession;
 
     /**
      * A URL to load when this object is initialized.  This handles the case where there is a URL
@@ -67,6 +53,13 @@ public class Toolbar extends LinearLayout implements UrlBar.UrlBarObserver, View
     }
 
     /**
+     * @return the mToolbarMenu
+     */
+    public ToolbarMenu getToolbarMenu() {
+        return mToolbarMenu;
+    }
+
+    /**
      * To be called when the native library is loaded so that this class can initialize its native
      * components.
      * @param blimpClientSession The {@link BlimpClientSession} that contains the content-lite
@@ -75,8 +68,12 @@ public class Toolbar extends LinearLayout implements UrlBar.UrlBarObserver, View
     public void initialize(BlimpClientSession blimpClientSession) {
         assert mNativeToolbarPtr == 0;
 
-        mNativeToolbarPtr = nativeInit(blimpClientSession);
+        mBlimpClientSession = blimpClientSession;
+        mNativeToolbarPtr = nativeInit(mBlimpClientSession);
         sendUrlTextInternal(mUrlToLoad);
+
+        mToolbarMenu = new ToolbarMenu(mContext, this);
+        mBlimpClientSession.addObserver(mToolbarMenu);
     }
 
     /**
@@ -84,6 +81,7 @@ public class Toolbar extends LinearLayout implements UrlBar.UrlBarObserver, View
      * this.
      */
     public void destroy() {
+        mBlimpClientSession.removeObserver(mToolbarMenu);
         if (mNativeToolbarPtr != 0) {
             nativeDestroy(mNativeToolbarPtr);
             mNativeToolbarPtr = 0;
@@ -97,15 +95,25 @@ public class Toolbar extends LinearLayout implements UrlBar.UrlBarObserver, View
      */
     public void loadUrl(String text) {
         mUrlBar.setText(text);
+        mBlimpClientSession.resetDebugStats();

[Kevin M, 2016/05/20 01:02:02]

I'm not too keen on making Toolbar also responsible for debug statistics
semantics - it adds side effects to the class that make it less testable and
also makes it harder for readers to comprehend what's in front of them.

I think it would be cleaner to delegate all debug stat state and logic  to the
activity that needs it.
* Decouple this relationship by publishing navigation events to an observer,
e.g. NavigationEventObserver.
* The debug view activity can be an instance of said observer interface. If one
of the methods is triggered, it would reset the stats.

[shaktisahu, 2016/05/22 22:36:56]

Yea, I will an interface for this. By the way, this class will undergo a lot of
changes in near feature. So not very keen on improving this by a lot.

[Kevin M, 2016/05/24 01:02:00]

Code finds a way to survive, so we should ensure that we mindfully balance
quality and expediency. :)

         sendUrlTextInternal(text);
     }
 
     /**
+     * Returns the URL from the URL bar.
+     * @return Current URL
+     */
+    public String getUrl() {
+        return mUrlBar.getText().toString();
+    }
+
+    /**
      * To be called when the user triggers a back navigation action.
      * @return Whether or not the back event was consumed.
      */
     public boolean onBackPressed() {
         if (mNativeToolbarPtr == 0) return false;
+        mBlimpClientSession.resetDebugStats();
         return nativeOnBackPressed(mNativeToolbarPtr);
     }
 
@@ -114,6 +122,7 @@ public class Toolbar extends LinearLayout implements UrlBar.UrlBarObserver, View
      */
     public void onForwardPressed() {
         if (mNativeToolbarPtr == 0) return;
+        mBlimpClientSession.resetDebugStats();
         nativeOnForwardPressed(mNativeToolbarPtr);
     }
 
@@ -132,7 +141,7 @@ public class Toolbar extends LinearLayout implements UrlBar.UrlBarObserver, View
         mMenuButton.setOnClickListener(new View.OnClickListener() {
             @Override
             public void onClick(View v) {
-                showMenu(v);
+                mToolbarMenu.showMenu(v);
             }
         });
 
@@ -178,87 +187,11 @@ public class Toolbar extends LinearLayout implements UrlBar.UrlBarObserver, View
         }
     }
 
-    private void showMenu(View anchorView) {
-        if (mPopupMenu == null) {
-            initializeMenu(anchorView);
-        }
-        mPopupMenu.show();
-        mPopupMenu.getListView().setDivider(null);
-    }
-
-    /**
-     * Creates and initializes the app menu anchored to the specified view.
-     * @param anchorView The anchor of the {@link ListPopupWindow}
-     */
-    private void initializeMenu(View anchorView) {
-        mPopupMenu = new ListPopupWindow(mContext);
-        mPopupMenu.setAdapter(new ArrayAdapter<String>(mContext, R.layout.toolbar_popup_item,
-                new String[] {mContext.getString(R.string.open_in_chrome),
-                        mContext.getString(R.string.version_info)}));
-        mPopupMenu.setAnchorView(anchorView);
-        mPopupMenu.setWidth(getResources().getDimensionPixelSize(R.dimen.toolbar_popup_item_width));
-        mPopupMenu.setVerticalOffset(-anchorView.getHeight());
-        mPopupMenu.setModal(true);
-        mPopupMenu.setOnItemClickListener(new OnItemClickListener() {
-            @Override
-            public void onItemClick(AdapterView<?> parent, View view, int position, long id) {
-                if (position == ID_OPEN_IN_CHROME) {
-                    openInChrome();
-                } else if (position == ID_VERSION_INFO) {
-                    showVersionInfo();
-                }
-                mPopupMenu.dismiss();
-            }
-        });
-    }
-
-    private void openInChrome() {
-        Intent intent = new Intent(Intent.ACTION_VIEW, Uri.parse(mUrlBar.getText().toString()));
-        intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
-        intent.setPackage("com.android.chrome");
-        try {
-            mContext.startActivity(intent);
-        } catch (ActivityNotFoundException e) {
-            // Chrome is probably not installed, so try with the default browser
-            intent.setPackage(null);
-            mContext.startActivity(intent);
-        }
-    }
-
-    private void showVersionInfo() {
-        Intent intent = new Intent();
-        intent.setClass(mContext, Preferences.class);
-        intent.putExtra(AboutBlimpPreferences.EXTRA_ASSIGNER_URL, mEngineInfo.assignerUrl);
-        intent.putExtra(AboutBlimpPreferences.EXTRA_ENGINE_IP, mEngineInfo.ipAddress);
-        intent.putExtra(AboutBlimpPreferences.EXTRA_ENGINE_VERSION, mEngineInfo.engineVersion);
-        mContext.startActivity(intent);
-    }
-
-    // BlimpClientSession.ConnectionObserver interface.
-    @Override
-    public void onAssignmentReceived(
-            int result, int suggestedMessageResourceId, EngineInfo engineInfo) {
-        mEngineInfo = engineInfo;
-    }
-
-    @Override
-    public void onConnected() {
-        if (mEngineInfo == null) return;
-
-        mEngineInfo.setConnected(true);
-    }
-
-    @Override
-    public void onDisconnected(String reason) {
-        if (mEngineInfo == null) return;
-
-        mEngineInfo.setConnected(false);
-    }
-
     // Methods that are called by native via JNI.
     @CalledByNative
     private void onEngineSentUrl(String url) {
         if (url != null) mUrlBar.setText(url);
+        mBlimpClientSession.resetDebugStats();
     }
 
     @CalledByNative