Increase test coverage for Content:// urls

chrome/test/android/javatests/src/org/chromium/chrome/test/TestContentProvider.java

Index: chrome/test/android/javatests/src/org/chromium/chrome/test/TestContentProvider.java
diff --git a/chrome/test/android/javatests/src/org/chromium/chrome/test/TestContentProvider.java b/chrome/test/android/javatests/src/org/chromium/chrome/test/TestContentProvider.java
index f13980e883dbe720d89b6f3cddb1d141ce488e50..2665c9293db0b51a704048a0776f104983ea8250 100644
--- a/chrome/test/android/javatests/src/org/chromium/chrome/test/TestContentProvider.java
+++ b/chrome/test/android/javatests/src/org/chromium/chrome/test/TestContentProvider.java
@@ -11,37 +11,50 @@ package org.chromium.chrome.test;
 import android.content.ContentProvider;
 import android.content.ContentValues;
 import android.content.Context;
-import android.content.res.AssetFileDescriptor;
 import android.database.AbstractCursor;
 import android.database.Cursor;
 import android.net.Uri;
 import android.os.ParcelFileDescriptor;
-import android.util.Log;
 
-import java.io.FileOutputStream;
+import org.chromium.base.Log;
+import org.chromium.base.test.util.UrlUtils;
+
+import java.io.File;
+import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.net.URLConnection;
 import java.util.HashMap;
 import java.util.Map;
 
-// Note: if you move this class, make sure you have also updated AndroidManifest.xml
+/**
+ * Content provider for testing content:// urls.
+ * Note: if you move this class, make sure you have also updated AndroidManifest.xml
+ */
 public class TestContentProvider extends ContentProvider {
+    private static final String ANDROID_DATA_FILE_PATH = "android/";
     private static final String AUTHORITY = "org.chromium.chrome.test.TestContentProvider";
     private static final String CONTENT_SCHEME = "content://";
-    private static final String CONTENT_TYPE = "image/png";
     private static final String GET_RESOURCE_REQUEST_COUNT = "get_resource_request_count";
-    private static final String RESET_RESOURCE_REQUEST_COUNT = "reset_resource_request_count";
+    private static final String RESET_RESOURCE_REQUEST_COUNTS = "reset_resource_request_counts";
     private static final String TAG = "TestContentProvider";
     private enum ColumnIndex {
         RESOURCE_REQUEST_COUNT_COLUMN,
     }
-    private final Map<String, Integer> mResourceRequestCount;
+    private Map<String, Integer> mResourceRequestCount;
+
+    // Content providers can be accessed from multiple threads.
+    private Object mLock = new Object();
 
     public static String createContentUrl(String target) {
         return CONTENT_SCHEME + AUTHORITY + "/" + target;
     }
 
     private static Uri createRequestUri(final String target, String resource) {
-        return Uri.parse(createContentUrl(target) + "?" + resource);
+        if (resource == null) {
+            return Uri.parse(createContentUrl(target));
+        } else {
+            return Uri.parse(createContentUrl(target) + "?" + resource);
+        }
     }
 
     public static int getResourceRequestCount(Context context, String resource) {
@@ -55,36 +68,44 @@ public class TestContentProvider extends ContentProvider {
         }
     }
 
-    public static void resetResourceRequestCount(Context context, String resource) {
-        Uri uri = createRequestUri(RESET_RESOURCE_REQUEST_COUNT, resource);
+    public static void resetResourceRequestCounts(Context context) {
+        Uri uri = createRequestUri(RESET_RESOURCE_REQUEST_COUNTS, null);
         // A null cursor is returned for this request.
         context.getContentResolver().query(uri, null, null, null, null);
     }
 
-    public TestContentProvider() {
-        super();
-        mResourceRequestCount = new HashMap<String, Integer>();
-    }
-
     @Override
     public boolean onCreate() {
         return true;
     }
 
     @Override
-    public AssetFileDescriptor openAssetFile(Uri uri, String mode) {
+    public ParcelFileDescriptor openFile(final Uri uri, String mode) throws FileNotFoundException {
         String resource = uri.getLastPathSegment();
-        if (mResourceRequestCount.containsKey(resource)) {
-            mResourceRequestCount.put(resource, mResourceRequestCount.get(resource) + 1);
-        } else {
-            mResourceRequestCount.put(resource, 1);
+        synchronized (mLock) {
+            if (mResourceRequestCount.containsKey(resource)) {
+                mResourceRequestCount.put(resource, mResourceRequestCount.get(resource));

[Ted C, 2016/11/04 22:04:45]

why drop the "+ 1" here?

won't this always keep it as 1?

[sgurun-gerrit only, 2016/11/04 22:40:44]

good catch. totally unintentional.    fortunately caught during review!!

This caused testContentUrlIframeAccessFromContentUrl to fail as the request
count now increased to 2. I fixed the test to check for "notlessthan 1" as the
other tests.

Now, you probably ask why the resource count is 2 and not 1. That is because in
our base/.../ContentUriUtils.java, the contentUriExists() calls content
resolver's openFileDescriptor() method to check if Uri exists. and later, to
read the resource, we call openFileDescriptor() again. This is probably why the
original tests always called notlessthan 1.

https://cs.chromium.org/chromium/src/base/android/java/src/org/chromium/base/...


Fixed.

+            } else {
+                mResourceRequestCount.put(resource, 1);
+            }
+        }
+        return createAsset(resource);
+    }
+
+    private static ParcelFileDescriptor createAsset(String resource) {
+        try {
+            return ParcelFileDescriptor.open(
+                    new File(UrlUtils.getTestFilePath(ANDROID_DATA_FILE_PATH + resource)),
+                    ParcelFileDescriptor.MODE_READ_ONLY);
+        } catch (IOException e) {
+            Log.e(TAG, e.getMessage(), e);
         }
-        return createImage();
+        return null;
     }
 
     @Override
     public String getType(Uri uri) {
-        return CONTENT_TYPE;
+        return URLConnection.guessContentTypeFromName(uri.getLastPathSegment());
     }
 
     @Override
@@ -181,46 +202,17 @@ public class TestContentProvider extends ContentProvider {
     @Override
     public Cursor query(Uri uri, String[] projection, String selection,
                         String[] selectionArgs, String sortOrder) {
-        String action = uri.getLastPathSegment();
-        String resource = uri.getQuery();
-        if (GET_RESOURCE_REQUEST_COUNT.equals(action)) {
-            return new ProviderStateCursor(mResourceRequestCount.containsKey(resource)
-                    ? mResourceRequestCount.get(resource) : 0);
-        } else if (RESET_RESOURCE_REQUEST_COUNT.equals(action)) {
-            mResourceRequestCount.put(resource, 0);
-        }
-        return null;
-    }
-
-    // 1x1 black dot png image.
-    private static final byte[] IMAGE = {
-        (byte) 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00,
-        0x0d, 0x49, 0x48, 0x44, 0x52, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
-        0x01, 0x08, 0x00, 0x00, 0x00, 0x00, 0x3a, 0x7e, (byte) 0x9b, 0x55, 0x00,
-        0x00, 0x00, 0x01, 0x73, 0x52, 0x47, 0x42, 0x00, (byte) 0xae, (byte) 0xce,
-        0x1c, (byte) 0xe9, 0x00, 0x00, 0x00, 0x0d, 0x49, 0x44, 0x41, 0x54, 0x08,
-        0x1d, 0x01, 0x02, 0x00, (byte) 0xfd, (byte) 0xff, 0x00, 0x00, 0x00, 0x02,
-        0x00, 0x01, (byte) 0xcd, (byte) 0xe3, (byte) 0xd1, 0x2b, 0x00, 0x00, 0x00,
-        0x00, 0x49, 0x45, 0x4e, 0x44, (byte) 0xae, 0x42, 0x60, (byte) 0x82
-    };
-
-    private static AssetFileDescriptor createImage() {
-        ParcelFileDescriptor[] pfds = null;
-        FileOutputStream fileOut = null;
-        try {
-            try {
-                pfds = ParcelFileDescriptor.createPipe();
-                fileOut = new FileOutputStream(pfds[1].getFileDescriptor());
-                fileOut.write(IMAGE);
-                fileOut.flush();
-                return new AssetFileDescriptor(pfds[0], 0, -1);
-            } finally {
-                if (fileOut != null) fileOut.close();
-                if (pfds != null && pfds[1] != null) pfds[1].close();
+        synchronized (mLock) {
+            String action = uri.getLastPathSegment();
+            String resource = uri.getQuery();
+            if (GET_RESOURCE_REQUEST_COUNT.equals(action)) {
+                return new ProviderStateCursor(mResourceRequestCount.containsKey(resource)
+                                ? mResourceRequestCount.get(resource)

[Ted C, 2016/11/04 22:04:45]

should be indented 8 from the start of the previous line and the : 0 would fit
here too

[sgurun-gerrit only, 2016/11/04 22:40:45]

git cl format decided that. I can fix it if you like to. I moved it up but when
I run git cl format again, it came back :)

[Ted C, 2016/11/04 22:48:35]

git cl format just doesn't work well on java sadly...i'd change it but I won't
force you to

+                                : 0);
+            } else if (RESET_RESOURCE_REQUEST_COUNTS.equals(action)) {
+                mResourceRequestCount = new HashMap<String, Integer>();
             }
-        } catch (IOException e) {
-            Log.e(TAG, e.getMessage(), e);
+            return null;
         }
-        return null;
     }
 }