[remoting android] Close navigation drawer before opening Help/Feedback.

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;
@@ skipping 90 matching lines @@
     private OAuthTokenConsumer mHostConnectingConsumer;
 
     private OAuthTokenConsumer mHostListRetrievingConsumer;
 
     private OAuthTokenConsumer mHostDeletingConsumer;
 
     private DrawerLayout mDrawerLayout;
 
     private ActionBarDrawerToggle mDrawerToggle;
 
+    /**
+     * Task to be run after the navigation drawer is closed. Can be null. This is used to run
+     * Help/Feedback tasks which require a screenshot with the drawer closed.
+     */
+    private Runnable mPendingDrawerCloseTask;
+
     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. */
     private void showNoAccountsDialog() {
         AlertDialog.Builder builder = new AlertDialog.Builder(this);
         builder.setMessage(R.string.noaccounts_message);
         builder.setPositiveButton(R.string.noaccounts_add_account,
@@ skipping 39 matching lines @@
      * Shows the appropriate view for the host list and hides the loading indicator. Shows either
      * the host list chooser or the host list empty view, depending on whether mHosts contains any
      * hosts.
      */
     private void updateHostListView() {
         mHostListView.setVisibility(mHosts.length == 0 ? View.GONE : View.VISIBLE);
         mEmptyView.setVisibility(mHosts.length == 0 ? View.VISIBLE : View.GONE);
         mProgressView.setVisibility(View.GONE);
     }
 
+    private void runPendingDrawerCloseTask() {
+        // Avoid potential recursion problems by null-ing the task first.
+        Runnable task = mPendingDrawerCloseTask;

[Yuwei, 2016/10/04 23:41:14]

Optionally you can have a queue of pending tasks rather than just one task so
that closeDrawerThenRun() can be run multiple times in a task.

[Lambros, 2016/10/05 00:03:15]

Acknowledged. No need for a queue just yet.

+        mPendingDrawerCloseTask = null;
+        if (task != null) {
+            task.run();
+        }
+    }
+
+    private void closeDrawerThenRun(Runnable task) {

[Yuwei, 2016/10/04 23:41:14]

Instead of having extra launch* methods here, could it be better to make
closeDrawerThenRun() public then create the anonymous class in
NavigationMenuAdapter? By doing so we don't need to go back and forth between
Chromoting and the Adapter when we later change the list items.

[Lambros, 2016/10/05 00:03:15]

If I'm not mistaken, this would mean doubly-nested anonymous Runnable classes in
NavigationMenuAdapter which would look really ugly.
That could be avoided by pulling out the Runnable-creation into new methods, but
that doesn't seem any better or worse than what I've got now.

[Yuwei, 2016/10/05 00:27:02]

Yep. Callback hell :) Though it can be simplified to:

()->{closeDrawerThenRun(()->{HelpSingleton.get().launchFeedback();});}

once the N SDK get rolled in.
Or you can just create them at the beginning of createNavigationMenu().

Maybe we can later implement the Promise and have something like this:

item.onClick().then(()->{
  return chromoting.closeDrawer();
}).then(()->{
  launchHelp(...);
});
Since Chromoting.java is already too messy to read, I would try not to introduce
more unrelated logic to this class. I don't really have a strong opinion for
which one is better, so you can go ahead if you like.

+        mPendingDrawerCloseTask = task;
+        if (mDrawerLayout.isDrawerOpen(Gravity.START)) {
+            mDrawerLayout.closeDrawer(Gravity.START);
+        } else {
+            runPendingDrawerCloseTask();
+        }
+    }
+
+    /** Closes any navigation drawer, then shows the Help screen. */
+    public void launchHelp(final HelpContext helpContext) {
+        closeDrawerThenRun(new Runnable() {
+            @Override
+            public void run() {
+                HelpSingleton.getInstance().launchHelp(Chromoting.this, helpContext);
+            }
+        });
+    }
+
+    /** Closes any navigation drawer, then shows the Feedback screen. */
+    public void launchFeedback() {
+        closeDrawerThenRun(new Runnable() {
+            @Override
+            public void run() {
+                HelpSingleton.getInstance().launchFeedback(Chromoting.this);
+            }
+        });
+    }
+
     /**
      * 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);
@@ skipping 13 matching lines @@
                         onHostClicked(position);
                     }
                 });
 
         mProgressView = findViewById(R.id.hostList_progress);
 
         findViewById(R.id.host_setup_link_android).setOnClickListener(this);
 
         mDrawerLayout = (DrawerLayout) findViewById(R.id.drawer_layout);
         mDrawerToggle = new ActionBarDrawerToggle(this, mDrawerLayout, toolbar,
-                R.string.open_navigation_drawer, R.string.close_navigation_drawer);
+                R.string.open_navigation_drawer, R.string.close_navigation_drawer) {
+            @Override
+            public void onDrawerClosed(View drawerView) {
+                super.onDrawerClosed(drawerView);
+                runPendingDrawerCloseTask();
+            }
+        };
         mDrawerLayout.addDrawerListener(mDrawerToggle);
 
         // Disable the hamburger icon animation. This is more complex than it ought to be.
         // The animation can be customized by tweaking some style parameters - see
         // http://developer.android.com/reference/android/support/v7/appcompat/R.styleable.html#DrawerArrowToggle .
         // But these can't disable the animation completely.
         // The icon can only be changed by disabling the drawer indicator, which has side-effects
         // that must be worked around. It disables the built-in click listener, so this has to be
         // implemented and added. This also requires that the toolbar be passed to the
         // ActionBarDrawerToggle ctor above (otherwise the listener is ignored and warnings are
@@ skipping 219 matching lines @@
         if (id == R.id.actionbar_directoryrefresh) {
             refreshHostList();
             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);
+        launchHelp(HelpContext.HOST_SETUP);
     }
 
     private void onDeleteHostClicked(int hostIndex) {
         HostInfo hostInfo = mHosts[hostIndex];
         final String hostId = hostInfo.id;
         String message = getString(R.string.confirm_host_delete_android, hostInfo.name);
         new AlertDialog.Builder(this)
                 .setMessage(message)
                 .setPositiveButton(android.R.string.yes,
                         new DialogInterface.OnClickListener() {
@@ skipping 221 matching lines @@
                 // Unreachable, but required by Google Java style and findbugs.
                 assert false : "Unreached";
         }
 
         if (dismissProgress && mProgressIndicator != null) {
             mProgressIndicator.dismiss();
             mProgressIndicator = null;
         }
     }
 }