Implement feedback in Chromoting Help & Feedback screen

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

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 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);
         Intent intent = new Intent(Intent.ACTION_VIEW, uri);
 
         // Verify that the device can launch an application for this intent, otherwise
         // startActivity() may crash the application.
         if (intent.resolveActivity(getPackageManager()) != null) {
             startActivity(intent);
         }
     }
 
+    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);
 
         WebView webView = new WebView(this);
         setContentView(webView);
 
         getActionBar().setTitle(getString(R.string.actionbar_help_title));
 
         CharSequence appName = getTitle();
@@ skipping 18 matching lines @@
 
     @Override
     public boolean onCreateOptionsMenu(Menu menu) {
         getMenuInflater().inflate(R.menu.help_actionbar, menu);
         return super.onCreateOptionsMenu(menu);
     }
 
     @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;
 
             default:
                 return super.onOptionsItemSelected(item);
         }
     }
 }