Ensure no-hosts view doesn't appear while loading

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.AlertDialog;
 import android.app.ProgressDialog;
 import android.content.DialogInterface;
@@ skipping 60 matching lines @@
 
     /** Helper for fetching the host list. */
     private HostListLoader mHostListLoader;
 
     /** List of hosts. */
     private HostInfo[] mHosts = new HostInfo[0];
 
     /** Refresh button. */
     private MenuItem mRefreshButton;
 
-    /** Host list as it appears to the user. */
+    /** Host list chooser view shown when at least one host is configured. */
     private ListView mHostListView;
 
+    /** View shown when the user has no configured hosts or host list couldn't be retrieved. */
+    private View mEmptyView;
+
     /** Progress view shown instead of the host list when the host list is loading. */
     private View mProgressView;
 
     /** Dialog for reporting connection progress. */
     private ProgressDialog mProgressIndicator;
 
     /**
      * Helper used by SessionConnection for session authentication. Receives onNewIntent()
      * notifications to handle third-party authentication.
      */
@@ skipping 47 matching lines @@
                 @Override
                 public void onCancel(DialogInterface dialog) {
                     finish();
                 }
             });
 
         AlertDialog dialog = builder.create();
         dialog.show();
     }
 
-    /** Shows or hides the progress indicator for loading the host list. */
-    private void setHostListProgressVisible(boolean visible) {
-        mHostListView.setVisibility(visible ? View.GONE : View.VISIBLE);
-        mProgressView.setVisibility(visible ? View.VISIBLE : View.GONE);
-
-        // Hiding the host-list does not automatically hide the empty view, so do that here.
-        if (visible) {
-            mHostListView.getEmptyView().setVisibility(View.GONE);
+    /**
+     * Shows the appropriate view for the host list. If loading is true, shows the loading
+     * indicator. If loading is false, shows either the host list chooser or the host list empty
+     * view, depending on whether mHosts contains any hosts. */
+    private void switchHostListView(boolean loading) {

[Lambros, 2016/01/20 19:23:46]

This name doesn't indicate what the boolean parameter means, so the calling code
is not very readable.
Perhaps a better name is showHostListLoadingIndicator() or similar?

[rkjnsn, 2016/01/20 19:39:41]

My concern is that that doesn't reflect that it's actually switching between
three different views. I think I would be surprised to see the logic for
choosing between the empty view and the chooser in a function called
showHostListLoadingIndicator. Maybe a better name for the boolean, itself, would
help? Or maybe this method should be split into two and not take a bool, at all.

+        if (loading) {
+            mHostListView.setVisibility(View.GONE);
+            mEmptyView.setVisibility(View.GONE);
+            mProgressView.setVisibility(View.VISIBLE);
+        } else {
+            mHostListView.setVisibility(mHosts.length == 0 ? View.GONE : View.VISIBLE);
+            mEmptyView.setVisibility(mHosts.length == 0 ? View.VISIBLE : View.GONE);
+            mProgressView.setVisibility(View.GONE);
         }
     }
 
     /**
      * Called when the activity is first created. Loads the native library and requests an
      * authentication token from the system.
      */
     @Override
     public void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
         setContentView(R.layout.main);
 
         Toolbar toolbar = (Toolbar) findViewById(R.id.toolbar);
         setSupportActionBar(toolbar);
 
         mTriedNewAuthToken = false;
         mHostListLoader = new HostListLoader();
 
         // Get ahold of our view widgets.
         mHostListView = (ListView) findViewById(R.id.hostList_chooser);
-        mHostListView.setEmptyView(findViewById(R.id.hostList_empty));
+        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);
                     }
                 });
 
         mProgressView = findViewById(R.id.hostList_progress);
@@ skipping 155 matching lines @@
         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);
             } else {
                 // User denied permission or cancelled the dialog, so cancel the request.
                 mWaitingForAuthToken = false;
-                setHostListProgressVisible(false);
+                switchHostListView(false);
             }
         }
     }
 
     /** Called when a permissions request has returned. */
     @Override
     public void onRequestPermissionsResult(int requestCode, String[] permissions,
             int[] grantResults) {
         // This is currently only used by AccountSwitcherBasic.
         // Check that the user has granted the needed permission, and reload the accounts.
@@ skipping 80 matching lines @@
         mAuthenticator = new SessionAuthenticator(this, host);
         connector.connectToHost(mAccount, mToken, host, mAuthenticator);
     }
 
     private void refreshHostList() {
         if (mWaitingForAuthToken) {
             return;
         }
 
         mTriedNewAuthToken = false;
-        setHostListProgressVisible(true);
+        switchHostListView(true);
 
         // The refresh button simply makes use of the currently-chosen account.
         requestAuthToken(false);
     }
 
     private void requestAuthToken(boolean expireCurrentToken) {
         mWaitingForAuthToken = true;
 
         OAuthTokenFetcher fetcher = new OAuthTokenFetcher(this, mAccount,
                 new OAuthTokenFetcher.Callback() {
                     @Override
                     public void onTokenFetched(String token) {
                         mWaitingForAuthToken = false;
                         mToken = token;
                         mHostListLoader.retrieveHostList(mToken, Chromoting.this);
                     }
 
                     @Override
                     public void onError(int errorResource) {
                         mWaitingForAuthToken = false;
-                        setHostListProgressVisible(false);
+                        switchHostListView(false);
                         String explanation = getString(errorResource);
                         Toast.makeText(Chromoting.this, explanation, Toast.LENGTH_LONG).show();
                     }
                 });
 
         if (expireCurrentToken) {
             fetcher.clearAndFetch(mToken);
             mToken = null;
         } else {
             fetcher.fetch();
@@ skipping 18 matching lines @@
     @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 HostListLoader. HostInfo
         // is an immutable type, so a shallow copy of the array is sufficient here.
         mHosts = Arrays.copyOf(hosts, hosts.length);
-        setHostListProgressVisible(false);
+        switchHostListView(false);
         updateUi();
     }
 
     @Override
     public void onError(HostListLoader.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();
-            setHostListProgressVisible(false);
+            switchHostListView(false);
             return;
         }
 
         // This is the AUTH_FAILED case.
 
         if (!mTriedNewAuthToken) {
             // This was our first connection attempt.
             mTriedNewAuthToken = true;
             requestAuthToken(true);
 
             // We're not in an error state *yet*.
             return;
         } else {
             // Authentication truly failed.
             Log.e(TAG, "Fresh auth token was rejected.");
             explanation = getString(R.string.error_authentication_failed);
             Toast.makeText(this, explanation, Toast.LENGTH_LONG).show();
-            setHostListProgressVisible(false);
+            switchHostListView(false);
         }
     }
 
     /**
      * Updates the infotext and host list display.
      */
     private void updateUi() {
         if (mRefreshButton != null) {
             mRefreshButton.setEnabled(mAccount != null);
         }
@@ skipping 36 matching lines @@
                 // Unreachable, but required by Google Java style and findbugs.
                 assert false : "Unreached";
         }
 
         if (dismissProgress && mProgressIndicator != null) {
             mProgressIndicator.dismiss();
             mProgressIndicator = null;
         }
     }
 }