Implementing Host List Context Menu and Delete Feature

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

 // Copyright 2013 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.annotation.SuppressLint;
 import android.app.ProgressDialog;
 import android.content.DialogInterface;
 import android.content.Intent;
 import android.content.SharedPreferences;
 import android.content.pm.PackageManager;
 import android.content.res.Configuration;
 import android.graphics.drawable.Drawable;
 import android.os.Bundle;
 import android.provider.Settings;
 import android.support.v4.graphics.drawable.DrawableCompat;
 import android.support.v4.widget.DrawerLayout;
 import android.support.v7.app.ActionBarDrawerToggle;
 import android.support.v7.app.AlertDialog;
 import android.support.v7.app.AppCompatActivity;
 import android.support.v7.widget.Toolbar;
+import android.view.ContextMenu;
 import android.view.Gravity;
 import android.view.Menu;
 import android.view.MenuItem;
 import android.view.View;
 import android.widget.AdapterView;
 import android.widget.ArrayAdapter;
 import android.widget.LinearLayout;
 import android.widget.ListView;
 import android.widget.Toast;
 
@@ skipping 76 matching lines @@
      */
     boolean mTriedNewAuthToken;
 
     /**
      * Flag to track whether a call to AccountManager.getAuthToken() is currently pending.
      * This avoids infinitely-nested calls in case onStart() gets triggered a second time
      * while a token is being fetched.
      */
     private boolean mWaitingForAuthToken = false;
 
+    /** ID of the host in progress of deletion. **/
+    private String mHostIdBeingDeleted;
+
     private DrawerLayout mDrawerLayout;
 
     private ActionBarDrawerToggle mDrawerToggle;
 
     private AccountSwitcher mAccountSwitcher;
 
     /** The currently-connected Client, if any. */
     private Client mClient;
 
     /** Shows a warning explaining that a Google account is required, then closes the activity. */
@@ skipping 60 matching lines @@
         setContentView(R.layout.main);
 
         Toolbar toolbar = (Toolbar) findViewById(R.id.toolbar);
         setSupportActionBar(toolbar);
 
         mTriedNewAuthToken = false;
         mHostListManager = new HostListManager();
 
         // Get ahold of our view widgets.
         mHostListView = (ListView) findViewById(R.id.hostList_chooser);
+        registerForContextMenu(mHostListView);
         mEmptyView = findViewById(R.id.hostList_empty);
         mHostListView.setOnItemClickListener(
                 new AdapterView.OnItemClickListener() {
                     @Override
                     public void onItemClick(AdapterView<?> parent, View view, int position,
                             long id) {
                         onHostClicked(position);
                     }
                 });
 
@@ skipping 155 matching lines @@
 
     /** Called when a child Activity exits and sends a result back to this Activity. */
     @Override
     public void onActivityResult(int requestCode, int resultCode, Intent data) {
         mAccountSwitcher.onActivityResult(requestCode, resultCode, data);
 
         if (requestCode == OAuthTokenFetcher.REQUEST_CODE_RECOVER_FROM_OAUTH_ERROR) {
             if (resultCode == RESULT_OK) {
                 // User gave OAuth permission to this app (or recovered from any OAuth failure),
                 // so retry fetching the token.
-                requestAuthToken(false);
+                refreshHostList(false);
             } else {
                 // User denied permission or cancelled the dialog, so cancel the request.
                 mWaitingForAuthToken = false;
                 updateHostListView();
             }
         }
     }
 
     /** Called when a permissions request has returned. */
     @Override
@@ skipping 12 matching lines @@
     }
 
     /** Called when the display is rotated (as registered in the manifest). */
     @Override
     public void onConfigurationChanged(Configuration newConfig) {
         super.onConfigurationChanged(newConfig);
 
         mDrawerToggle.onConfigurationChanged(newConfig);
     }
 
-    /** Called to initialize the action bar. */
+    private int getHostIndexForMenu(ContextMenu.ContextMenuInfo menuInfo) {

[Lambros, 2016/05/12 23:42:13]

static

[Yuwei, 2016/05/13 19:09:32]

Done.

+        return ((AdapterView.AdapterContextMenuInfo) menuInfo).position;
+    }
+
+    @Override
+    public void onCreateContextMenu(ContextMenu menu, View v,
+                                    ContextMenu.ContextMenuInfo menuInfo) {

[Lambros, 2016/05/12 23:42:13]

nit: Java line-continuation is +8-space indent.

[Yuwei, 2016/05/13 19:09:32]

Done.

+        super.onCreateContextMenu(menu, v, menuInfo);
+        if (v.getId() == R.id.hostList_chooser) {
+            getMenuInflater().inflate(R.menu.options_actionbar, menu);
+            HostInfo info = mHosts[getHostIndexForMenu(menuInfo)];
+            menu.setHeaderTitle(info.name);
+        }
+    }
+
+    @Override
+    public boolean onContextItemSelected(MenuItem item) {
+        int i = item.getItemId();

[Lambros, 2016/05/12 23:42:13]

nit: 'id' instead of 'i'.

[Yuwei, 2016/05/13 19:09:32]

Done.

+        if (i == R.id.actionbar_connect) {
+            onHostClicked(getHostIndexForMenu(item.getMenuInfo()));

[Lambros, 2016/05/12 23:42:13]

Maybe pull out the index?
int hostIndex = getHostIndexForMenu(item.getMenuInfo());

[Yuwei, 2016/05/13 19:09:32]

Done.

+        } else if (i == R.id.actionbar_delete) {
+            HostInfo hostInfo = mHosts[getHostIndexForMenu(item.getMenuInfo())];
+            AlertDialog.Builder builder = new AlertDialog.Builder(this);
+            String message = getString(R.string.confirm_host_delete_android, hostInfo.name);
+            final String hostId = hostInfo.id;
+            builder.setMessage(message)
+                    .setPositiveButton(android.R.string.yes,
+                            new DialogInterface.OnClickListener() {
+                                @Override
+                                public void onClick(DialogInterface dialog, int which) {
+                                    deleteHost(false, hostId);
+                                    dialog.dismiss();
+                                }
+                            })
+                    .setNegativeButton(android.R.string.no, null)
+                    .create().show();
+        } else {
+            return false;
+        }
+        return true;
+    }
+
+        /** Called to initialize the action bar. */

[Lambros, 2016/05/12 23:42:13]

nit: indentation

[Yuwei, 2016/05/13 19:09:32]

Done.

     @Override
     public boolean onCreateOptionsMenu(Menu menu) {
         getMenuInflater().inflate(R.menu.chromoting_actionbar, menu);
         mRefreshButton = menu.findItem(R.id.actionbar_directoryrefresh);
 
         if (mAccount == null) {
             // If there is no account, don't allow the user to refresh the listing.
             mRefreshButton.setEnabled(false);
         }
 
         ChromotingUtil.tintMenuIcons(this, menu);
 
         return super.onCreateOptionsMenu(menu);
     }
 
     /** Called whenever an action bar button is pressed. */
     @Override
     public boolean onOptionsItemSelected(MenuItem item) {
         if (mDrawerToggle.onOptionsItemSelected(item)) {
             return true;
         }
 
         int id = item.getItemId();
         if (id == R.id.actionbar_directoryrefresh) {
-            refreshHostList();
+            refreshHostList(false);
             return true;
         }
         return super.onOptionsItemSelected(item);
     }
 
     /** Called when the user touches hyperlinked text. */
     @Override
     public void onClick(View view) {
         HelpSingleton.getInstance().launchHelp(this, HelpContext.HOST_SETUP);
     }
@@ skipping 30 matching lines @@
                         }
                     }
                 });
 
         SessionConnector connector = new SessionConnector(mClient, this, this, mHostListManager);
         mAuthenticator = new SessionAuthenticator(this, mClient, host);
         connector.connectToHost(mAccount, mToken, host, mAuthenticator,
                 getPreferences(MODE_PRIVATE).getString(PREFERENCE_EXPERIMENTAL_FLAGS, ""));
     }
 
-    private void refreshHostList() {
+    private void showAuthErrorMessage(OAuthTokenFetcher.Error error) {
+        String explanation = getString(error == OAuthTokenFetcher.Error.NETWORK
+                ? R.string.error_network_error : R.string.error_unexpected);
+        Toast.makeText(Chromoting.this, explanation, Toast.LENGTH_LONG).show();
+    }
+
+    private void refreshHostList(boolean expireCurrentToken) {
         if (mWaitingForAuthToken) {
             return;
         }
-
-        mTriedNewAuthToken = false;
         showHostListLoadingIndicator();
 
         // The refresh button simply makes use of the currently-chosen account.
-        requestAuthToken(false);
+        requestAuthToken(expireCurrentToken, new OAuthTokenFetcher.Callback() {
+            @Override
+            public void onTokenFetched(String token) {
+                mHostListManager.retrieveHostList(mToken, Chromoting.this);
+            }
+
+            @Override
+            public void onError(OAuthTokenFetcher.Error error) {
+                showAuthErrorMessage(error);
+                updateHostListView();
+            }
+        });
     }
 
-    private void requestAuthToken(boolean expireCurrentToken) {
+    private void deleteHost(boolean expireCurrentToken, final String hostId) {
+        if (mWaitingForAuthToken) {
+            return;
+        }
+        showHostListLoadingIndicator();
+
+        mHostIdBeingDeleted = hostId;
+
+        requestAuthToken(expireCurrentToken, new OAuthTokenFetcher.Callback() {
+            @Override
+            public void onTokenFetched(String token) {
+                mHostListManager.deleteHost(token, hostId, Chromoting.this);
+            }
+
+            @Override
+            public void onError(OAuthTokenFetcher.Error error) {
+                showAuthErrorMessage(error);
+            }
+        });
+    }
+
+    // TODO(yuweih): This implementation have undefined behavior if multiple auth token requests
+    // start at the same time. In that case we should consider queueing up the requests.
+    private void requestAuthToken(boolean expireCurrentToken,
+                                  final OAuthTokenFetcher.Callback callback) {

[Lambros, 2016/05/12 23:42:13]

nit: line-continuation is +8 space.

         mWaitingForAuthToken = true;
 
         OAuthTokenFetcher fetcher = new OAuthTokenFetcher(this, mAccount, TOKEN_SCOPE,
                 new OAuthTokenFetcher.Callback() {
                     @Override
                     public void onTokenFetched(String token) {
                         mWaitingForAuthToken = false;
                         mToken = token;
-                        mHostListManager.retrieveHostList(mToken, Chromoting.this);
+                        callback.onTokenFetched(token);
                     }
 
                     @Override
                     public void onError(OAuthTokenFetcher.Error error) {
                         mWaitingForAuthToken = false;
-                        updateHostListView();
-                        String explanation = getString(error == OAuthTokenFetcher.Error.NETWORK
-                                ? R.string.error_network_error : R.string.error_unexpected);
-                        Toast.makeText(Chromoting.this, explanation, Toast.LENGTH_LONG).show();
+                        callback.onError(error);
                     }
                 });
 
         if (expireCurrentToken) {
             fetcher.clearAndFetch(mToken);
             mToken = null;
         } else {
             fetcher.fetch();
         }
     }
 
     @Override
     public void onAccountSelected(String accountName) {
         mAccount = accountName;
 
         // The current host list is no longer valid for the new account, so clear the list.
         mHosts = new HostInfo[0];
         updateUi();
-        refreshHostList();
+        refreshHostList(false);
     }
 
     @Override
     public void onAccountsListEmpty() {
         showNoAccountsDialog();
     }
 
     @Override
     public void onRequestCloseDrawer() {
         mDrawerLayout.closeDrawers();
     }
 
     @Override
     public void onHostListReceived(HostInfo[] hosts) {
         // Store a copy of the array, so that it can't be mutated by the HostListManager. HostInfo
         // is an immutable type, so a shallow copy of the array is sufficient here.
+        mTriedNewAuthToken = false;

[Lambros, 2016/05/12 23:42:13]

I think we can get rid of the retry-if-auth-token-expired logic. The old
AccountsManager API would often return invalid OAuth tokens which the
application needed to test and refresh explicitly. The new GoogleAuthUtil class
doesn't have this problem - it always hands us good tokens (it refreshes
automatically if expired).

Don't need to worry about it for this CL.

[Yuwei, 2016/05/13 19:09:32]

Done.

         mHosts = Arrays.copyOf(hosts, hosts.length);
         updateHostListView();
         updateUi();
     }
 
     @Override
     public void onHostUpdated() {
         // Not implemented Yet.
     }
 
     @Override
     public void onHostDeleted() {
-        // Not implemented Yet.
+        mHostIdBeingDeleted = null;
+        mHostListManager.retrieveHostList(mToken, this);
     }
 
     @Override
-    public void onError(HostListManager.Error error) {
+    public void onError(HostListManager.RequestType type, HostListManager.Error error) {
         String explanation = null;
         switch (error) {
             case AUTH_FAILED:
                 break;
             case NETWORK_ERROR:
                 explanation = getString(R.string.error_network_error);
                 break;
             case UNEXPECTED_RESPONSE:
             case SERVICE_UNAVAILABLE:
             case UNKNOWN:
                 explanation = getString(R.string.error_unexpected);
                 break;
             default:
                 // Unreachable.
                 return;
         }
 
         if (explanation != null) {
             Toast.makeText(this, explanation, Toast.LENGTH_LONG).show();
             updateHostListView();
             return;
         }
 
         // This is the AUTH_FAILED case.
 
         if (!mTriedNewAuthToken) {
             // This was our first connection attempt.
             mTriedNewAuthToken = true;
-            requestAuthToken(true);
-
+            switch (type) {
+                case RETRIEVE_HOST_LIST:
+                    refreshHostList(true);
+                    break;
+                case DELETE_HOST:
+                    deleteHost(true, mHostIdBeingDeleted);
+            }
             // We're not in an error state *yet*.
-            return;
         } else {
             // Authentication truly failed.
+            mTriedNewAuthToken = false;
+            mHostIdBeingDeleted = null;
             Log.e(TAG, "Fresh auth token was rejected.");
             explanation = getString(R.string.error_authentication_failed);
             Toast.makeText(this, explanation, Toast.LENGTH_LONG).show();
             updateHostListView();
         }
     }
 
     /**
      * Updates the infotext and host list display.
      */
@@ skipping 40 matching lines @@
                 // Unreachable, but required by Google Java style and findbugs.
                 assert false : "Unreached";
         }
 
         if (dismissProgress && mProgressIndicator != null) {
             mProgressIndicator.dismiss();
             mProgressIndicator = null;
         }
     }
 }