Ensure no-hosts view doesn't appear while loading

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

Index: remoting/android/java/src/org/chromium/chromoting/Chromoting.java
diff --git a/remoting/android/java/src/org/chromium/chromoting/Chromoting.java b/remoting/android/java/src/org/chromium/chromoting/Chromoting.java
index 800be2e323e043b80d7705411ddc1359ec91d527..898d2531e874622f15662e2961ddb312197136d1 100644
--- a/remoting/android/java/src/org/chromium/chromoting/Chromoting.java
+++ b/remoting/android/java/src/org/chromium/chromoting/Chromoting.java
@@ -78,9 +78,12 @@ public class Chromoting extends AppCompatActivity implements ConnectionListener,
     /** 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;
 
@@ -148,14 +151,19 @@ public class Chromoting extends AppCompatActivity implements ConnectionListener,
         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);
         }
     }
 
@@ -176,7 +184,7 @@ public class Chromoting extends AppCompatActivity implements ConnectionListener,
 
         // 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
@@ -352,7 +360,7 @@ public class Chromoting extends AppCompatActivity implements ConnectionListener,
             } else {
                 // User denied permission or cancelled the dialog, so cancel the request.
                 mWaitingForAuthToken = false;
-                setHostListProgressVisible(false);
+                switchHostListView(false);
             }
         }
     }
@@ -453,7 +461,7 @@ public class Chromoting extends AppCompatActivity implements ConnectionListener,
         }
 
         mTriedNewAuthToken = false;
-        setHostListProgressVisible(true);
+        switchHostListView(true);
 
         // The refresh button simply makes use of the currently-chosen account.
         requestAuthToken(false);
@@ -474,7 +482,7 @@ public class Chromoting extends AppCompatActivity implements ConnectionListener,
                     @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();
                     }
@@ -513,7 +521,7 @@ public class Chromoting extends AppCompatActivity implements ConnectionListener,
         // 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();
     }
 
@@ -538,7 +546,7 @@ public class Chromoting extends AppCompatActivity implements ConnectionListener,
 
         if (explanation != null) {
             Toast.makeText(this, explanation, Toast.LENGTH_LONG).show();
-            setHostListProgressVisible(false);
+            switchHostListView(false);
             return;
         }
 
@@ -556,7 +564,7 @@ public class Chromoting extends AppCompatActivity implements ConnectionListener,
             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);
         }
     }