Follow up changes to ag/1627041.

Change-Id: I376ab5a26ff98eca1cda6a9e43f47e666e8001c3

4 files changed, 89 insertions, 46 deletions

diff --git a/src/com/android/documentsui/RefreshTask.java b/src/com/android/documentsui/RefreshTask.java
index 252197616..46920822a 100644
--- a/src/com/android/documentsui/RefreshTask.java
+++ b/src/com/android/documentsui/RefreshTask.java
@@ -28,10 +28,10 @@ import android.net.Uri;
 import android.os.CancellationSignal;
 import android.util.Log;
 
-import com.android.documentsui.TimeoutTask;
+import com.android.documentsui.base.ApplicationScope;
 import com.android.documentsui.base.CheckedTask;
+import com.android.documentsui.base.Shared;
 import com.android.documentsui.base.State;
-import com.android.documentsui.dirlist.Model;
 
 import java.util.function.Consumer;
 
@@ -44,60 +44,74 @@ public class RefreshTask extends TimeoutTask<Void, Boolean> {
 
     private final static String TAG = "RefreshTask";
 
-    private final Context mContext;
+    private final @ApplicationScope Context mContext;
     private final State mState;
+    private final Uri mUri;
     private final Consumer<Boolean> mCallback;
     private final CancellationSignal mSignal;
 
-
-    public RefreshTask(State state, Activity activity, Consumer<Boolean> callback) {
-        this(state, activity, activity::isDestroyed, callback);
-    }
-
-    public RefreshTask(State state, Fragment fragment, Consumer<Boolean> callback) {
-        this(state, fragment.getContext(), fragment::isDetached, callback);
-    }
-
-    public RefreshTask(State state, Context context, Check check, Consumer<Boolean> callback) {
+    public RefreshTask(State state, Uri uri, long timeout, @ApplicationScope Context context, Check check,
+            Consumer<Boolean> callback) {
         super(check);
-        mContext = context.getApplicationContext();
+        mUri = uri;
+        mContext = context;
         mState = state;
         mCallback = callback;
         mSignal = new CancellationSignal();
+        setTimeout(timeout);
     }
 
     @Override
     public @Nullable Boolean run(Void... params) {
-        final Uri uri = mState.stack.peek().derivedUri;
+        if (mUri == null) {
+            Log.w(TAG, "Attempted to refresh on a null uri. Aborting.");
+            return false;
+        }
+
+        if (mUri != mState.stack.peek().derivedUri) {
+            Log.w(TAG, "Attempted to refresh on a non-top-level uri. Aborting.");
+            return false;
+        }
+
+        // API O introduces ContentResolver#refresh, and if available and the ContentProvider
+        // supports it, the ContentProvider will automatically send a content updated notification
+        // and we will update accordingly. Else, we just tell the callback that Refresh is not
+        // supported.
+        if (!Shared.ENABLE_OMC_API_FEATURES) {
+            Log.w(TAG, "Attempted to call Refresh on an older Android platform. Aborting.");
+            return false;
+        }
+
         final ContentResolver resolver = mContext.getContentResolver();
-        final String authority = uri.getAuthority();
-        boolean refreshed = false;
+        final String authority = mUri.getAuthority();
+        boolean refreshSupported = false;
         ContentProviderClient client = null;
         try {
             client = DocumentsApplication.acquireUnstableProviderOrThrow(resolver, authority);
-            refreshed = client.refresh(uri, null, mSignal);
+            refreshSupported = client.refresh(mUri, null, mSignal);
         } catch (Exception e) {
             Log.w(TAG, "Failed to refresh", e);
         } finally {
             ContentProviderClient.releaseQuietly(client);
         }
-        return refreshed;
+        return refreshSupported;
     }
 
     @Override
     protected void onTimeout() {
         mSignal.cancel();
+        Log.w(TAG, "Provider taking too long to respond. Cancelling.");
     }
 
     @Override
-    public void finish(Boolean refreshed) {
+    public void finish(Boolean refreshSupported) {
         if (DEBUG) {
-            if (refreshed) {
-                Log.v(TAG, "Provider has new content and has refreshed");
+            if (refreshSupported) {
+                Log.v(TAG, "Provider supports refresh and has refreshed");
             } else {
-                Log.v(TAG, "Provider has no new content and did not refresh");
+                Log.v(TAG, "Provider does not support refresh and did not refresh");
             }
         }
-        mCallback.accept(refreshed);
+        mCallback.accept(refreshSupported);
     }
 }
diff --git a/src/com/android/documentsui/TimeoutTask.java b/src/com/android/documentsui/TimeoutTask.java
index ffa4da624..5fb20cfae 100644
--- a/src/com/android/documentsui/TimeoutTask.java
+++ b/src/com/android/documentsui/TimeoutTask.java
@@ -58,7 +58,9 @@ public abstract class TimeoutTask<Input, Output> extends CheckedTask<Input, Outp
         }, mTimeout);
     }
 
-    // Override this do more proper clean up in case of timeout, such as using
-    // CancellationSignal#cancel.
-    protected void onTimeout() { }
+    /*
+     * Override this do more proper clean up in case of timeout, such as using
+     * CancellationSignal#cancel.
+     */
+    protected void onTimeout() {}
 }
diff --git a/src/com/android/documentsui/base/ApplicationScope.java b/src/com/android/documentsui/base/ApplicationScope.java
new file mode 100644
index 000000000..738b3a202
--- /dev/null
+++ b/src/com/android/documentsui/base/ApplicationScope.java
@@ -0,0 +1,31 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.documentsui.base;
+
+import static java.lang.annotation.ElementType.FIELD;
+import static java.lang.annotation.ElementType.PARAMETER;
+import static java.lang.annotation.RetentionPolicy.SOURCE;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+@Retention(SOURCE)
+@Target({
+        FIELD, PARAMETER
+})
+public @interface ApplicationScope {
+}
diff --git a/src/com/android/documentsui/dirlist/DirectoryFragment.java b/src/com/android/documentsui/dirlist/DirectoryFragment.java
index c1bd936f8..26520e833 100644
--- a/src/com/android/documentsui/dirlist/DirectoryFragment.java
+++ b/src/com/android/documentsui/dirlist/DirectoryFragment.java
@@ -1145,18 +1145,18 @@ public class DirectoryFragment extends Fragment
             cache.removeUri(mModel.getItemUri(ids[i]));
         }
 
-        if (Shared.ENABLE_OMC_API_FEATURES) {
-            RefreshTask refreshTask = new RefreshTask(mState, this,
-                    (Boolean refreshed) -> {
-                        new Handler(Looper.getMainLooper())
-                                .post(() -> mRefreshLayout.setRefreshing(false));
-                    });
-            refreshTask.setTimeout(REFRESH_SPINNER_TIMEOUT);
-            refreshTask.executeOnExecutor(mActivity.getExecutorForCurrentDirectory());
-        } else {
-            // If Refresh API isn't available, we will explicitly reload the loader
-            getLoaderManager().restartLoader(LOADER_ID, null, mLoaderCallbacks);
-        }
+        final Uri uri = mState.stack.peek().derivedUri;
+        RefreshTask task = new RefreshTask(mState, uri, REFRESH_SPINNER_TIMEOUT,
+                getContext().getApplicationContext(), this::isDetached,
+                (Boolean refreshSupported) -> {
+                    if (refreshSupported) {
+                        mRefreshLayout.setRefreshing(false);
+                    } else {
+                        // If Refresh API isn't available, we will explicitly reload the loader
+                        getLoaderManager().restartLoader(LOADER_ID, null, mLoaderCallbacks);
+                    }
+                });
+        task.executeOnExecutor(mActivity.getExecutorForCurrentDirectory());
     }
 
     private final class ModelUpdateListener implements EventListener<Model.Update> {
@@ -1299,8 +1299,6 @@ public class DirectoryFragment extends Fragment
 
             mAdapter.notifyDataSetChanged();
             mModel.update(result);
-            mRefreshLayout.setEnabled(
-                    result.cursor.getExtras().getBoolean(EXTRA_REFRESH_SUPPORTED, false));
 
             updateLayout(mState.derivedMode);
 
@@ -1331,7 +1329,7 @@ public class DirectoryFragment extends Fragment
             mLocalState.mLastSortDimensionId = curSortedDimension.getId();
             mLocalState.mLastSortDirection = curSortedDimension.getSortDirection();
 
-            if (!Shared.ENABLE_OMC_API_FEATURES && mRefreshLayout.isRefreshing()) {
+            if (mRefreshLayout.isRefreshing()) {
                 new Handler().postDelayed(
                         () -> mRefreshLayout.setRefreshing(false),
                         REFRESH_SPINNER_TIMEOUT);
@@ -1341,12 +1339,10 @@ public class DirectoryFragment extends Fragment
         @Override
         public void onLoaderReset(Loader<DirectoryResult> loader) {
             if (DEBUG) Log.d(TAG, "Resetting loader for: "
-                    + DocumentInfo.debugString(mLocalState.mDocument));
+                        + DocumentInfo.debugString(mLocalState.mDocument));
             mModel.onLoaderReset();
 
-            if (!Shared.ENABLE_OMC_API_FEATURES) {
-                mRefreshLayout.setRefreshing(false);
-            }
+            mRefreshLayout.setRefreshing(false);
         }
     }
 }