Implement feedback in Chromoting Help & Feedback screen

remoting/android/java/src/org/chromium/chromoting/HelpActivity.java

Index: remoting/android/java/src/org/chromium/chromoting/HelpActivity.java
diff --git a/remoting/android/java/src/org/chromium/chromoting/HelpActivity.java b/remoting/android/java/src/org/chromium/chromoting/HelpActivity.java
index 0404827cb0dc86930b90efd2de0197c3e5a7a2d8..547011e5ea7dc93e7db4d58edda1e72fec167f3b 100644
--- a/remoting/android/java/src/org/chromium/chromoting/HelpActivity.java
+++ b/remoting/android/java/src/org/chromium/chromoting/HelpActivity.java
@@ -5,23 +5,42 @@
 package org.chromium.chromoting;
 
 import android.app.Activity;
+import android.content.ComponentName;
 import android.content.Intent;
+import android.content.ServiceConnection;
 import android.content.pm.PackageInfo;
 import android.content.pm.PackageManager;
+import android.graphics.Bitmap;
 import android.net.Uri;
+import android.os.Binder;
 import android.os.Bundle;
+import android.os.IBinder;
+import android.os.Parcel;
+import android.os.RemoteException;
 import android.text.TextUtils;
+import android.util.Log;
 import android.view.Menu;
 import android.view.MenuItem;
+import android.view.View;
 import android.webkit.WebView;
 import android.webkit.WebViewClient;
 
+import org.chromium.ui.UiUtils;
+
 /**
  * The Activity for showing the Help screen.
  */
 public class HelpActivity extends Activity {
     private static final String PLAY_STORE_URL = "market://details?id=";
 
+    /**
+     * Maximum dimension for the screenshot to be sent to the Send Feedback handler.  This size
+     * ensures the size of bitmap < 1MB, which is a requirement of the handler.
+     */
+    private static final int MAX_FEEDBACK_SCREENSHOT_DIMENSION = 600;

[Jamie, 2014/03/06 01:52:01]

Is this assuming a square screenshot?

[Lambros, 2014/03/06 02:31:31]

I don't know :) Chrome uses this same value.

+
+    private static Bitmap mScreenshot;

[Lambros, 2014/03/06 00:38:30]

Global variables are evil, but there doesn't seem to be any other simple way of
passing the screenshot from the main activity to the Help activity. We do it the
same way in Chrome's Feedback implementation.

[Jamie, 2014/03/06 01:52:01]

I think this warrants a comment on the member itself, not just in the code
review.

[Lambros, 2014/03/06 02:31:31]

Done.

+
     /** Launches an external web browser or application. */
     private void openUrl(String url) {
         Uri uri = Uri.parse(url);
@@ -34,6 +53,41 @@ public class HelpActivity extends Activity {
         }
     }
 
+    private void sendFeedback() {
+        Intent intent = new Intent(Intent.ACTION_BUG_REPORT);
+        ServiceConnection conn = new ServiceConnection() {
+            @Override
+            public void onServiceConnected(ComponentName name, IBinder service) {
+                try {
+                    Parcel parcel = Parcel.obtain();
+                    if (mScreenshot != null) {
+                        mScreenshot.writeToParcel(parcel, 0);
+                    }
+                    service.transact(Binder.FIRST_CALL_TRANSACTION, parcel, null, 0);

[Jamie, 2014/03/06 01:52:01]

I had to look up FIRST_CALL_TRANSATION. As far as self-documenting code goes,
it's not much better than hard-coding a constant value. Is it worth defining a
constant, named appropriately for what the recipient of this transaction will do
when it processes it?

[Lambros, 2014/03/06 02:31:31]

Done, though I'm not sure I've thought of a good name for it.

+                    parcel.recycle();
+                } catch (RemoteException ex) {
+                    Log.d("help", "Unexpected error sending feedback: ", ex);

[Jamie, 2014/03/06 01:52:01]

Should this be Log.e?

[Lambros, 2014/03/06 02:31:31]

Done.

+                }
+            }
+
+            @Override
+            public void onServiceDisconnected(ComponentName name) {}
+        };
+
+        bindService(intent, conn, BIND_AUTO_CREATE);
+    }
+
+    /** Launches the Help activity. */
+    public static void launch(Activity activity, String helpUrl) {
+        View rootView = activity.getWindow().getDecorView().getRootView();
+        mScreenshot = UiUtils.generateScaledScreenshot(rootView, MAX_FEEDBACK_SCREENSHOT_DIMENSION,
+                Bitmap.Config.ARGB_8888);
+
+        Intent intent = new Intent(activity, HelpActivity.class);
+        intent.setData(Uri.parse(helpUrl));
+        activity.startActivity(intent);
+    }
+
     @Override
     public void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
@@ -72,6 +126,10 @@ public class HelpActivity extends Activity {
     @Override
     public boolean onOptionsItemSelected(MenuItem item) {
         switch (item.getItemId()) {
+            case R.id.actionbar_feedback:
+                sendFeedback();
+                return true;
+
             case R.id.actionbar_play_store:
                 openUrl(PLAY_STORE_URL + getPackageName());
                 return true;