Skip to content

Commit

Permalink
Use real Android code for ContentProviderClient.release
Browse files Browse the repository at this point in the history
This fixes two issues:
1) Because 'release' was shadowed, the underlying CloseGuard for the
ContentProviderClient was never closed, leading to CloseGuard errors that could
not be suppressed.

2) The behavior of 'release' was not correct for SDK > 23, where calling it
multiple times does not result in an IllegalStateException.

Also, update ShadowContentProviderClientTest to use a real ContentProvider, and
remove the interaction test. Trying to release a ContentProviderClient for a
mock ContentProvider results in an exception.

PiperOrigin-RevId: 413795247
  • Loading branch information
hoisie authored and copybara-robolectric committed Dec 3, 2021
1 parent bcb31db commit dccf8ef
Show file tree
Hide file tree
Showing 34 changed files with 1,810 additions and 255 deletions.
12 changes: 10 additions & 2 deletions .github/workflows/build_native_runtime.yml
Expand Up @@ -32,6 +32,14 @@ jobs:
with:
java-version: 11.0.8

- name: Install Ninja (Mac OS)
if: runner.os =='macOS'
run: brew install ninja

- name: Install Ninja (Linux)
if: runner.os =='Linux'
run: sudo apt-get install ninja-build

- name: Cache ICU build output
id: cache-icu
uses: actions/cache@v2
Expand Down Expand Up @@ -61,8 +69,8 @@ jobs:
run: |
mkdir build
cd build
ICU_ROOT_DIR=$HOME/icu-bin cmake -B . -S ../nativeruntime/cpp
make
ICU_ROOT_DIR=$HOME/icu-bin cmake -B . -S ../nativeruntime/cpp -G Ninja
ninja
- name: Rename libnativeruntime for Linux
if: runner.os == 'Linux'
Expand Down
8 changes: 8 additions & 0 deletions .github/workflows/tests.yml
Expand Up @@ -52,6 +52,10 @@ jobs:
make -j4
make install
- name: Install Ninja (Linux)
if: runner.os =='Linux'
run: sudo apt-get install ninja-build

- name: Build
run: ICU_ROOT_DIR=$HOME/icu-bin SKIP_ERRORPRONE=true SKIP_JAVADOC=true ./gradlew clean assemble testClasses --parallel --stacktrace --no-watch-fs

Expand Down Expand Up @@ -93,6 +97,10 @@ jobs:
path: ~/icu-bin
key: ${{ runner.os }}-${{env.CPU_ARCH}}-icu-${{ hashFiles('nativeruntime/external/icu/**') }}

- name: Install Ninja
if: runner.os =='Linux'
run: sudo apt-get install ninja-build

- name: Run unit tests
run: |
ICU_ROOT_DIR=$HOME/icu-bin SKIP_ERRORPRONE=true SKIP_JAVADOC=true ./gradlew test --info --stacktrace --continue \
Expand Down
4 changes: 2 additions & 2 deletions nativeruntime/build.gradle
Expand Up @@ -29,7 +29,7 @@ task cmakeNativeRuntime {
mkdir "$buildDir/cpp"
exec {
workingDir "$buildDir/cpp"
commandLine 'cmake', "-B", ".", "-S","$projectDir/cpp/"
commandLine 'cmake', "-B", ".", "-S","$projectDir/cpp/", "-G", "Ninja"
}
}
}
Expand Down Expand Up @@ -80,7 +80,7 @@ task makeNativeRuntime {
doLast {
exec {
workingDir "$buildDir/cpp"
commandLine 'make'
commandLine 'ninja'
}
}
}
Expand Down
9 changes: 2 additions & 7 deletions nativeruntime/cpp/CMakeLists.txt
Expand Up @@ -3,11 +3,6 @@ cmake_minimum_required(VERSION 3.10)
# This is needed to ensure that static libraries can be linked into shared libraries.
set(CMAKE_POSITION_INDEPENDENT_CODE ON)

# Building the nativeruntime does not work with GCC due to libstddc++ linker errors.
# TODO: figure out which linker args are needed to build with GCC.
set(CMAKE_C_COMPILER "clang")
set(CMAKE_CXX_COMPILER "clang++")

# Some libutils headers require C++17
set (CMAKE_CXX_STANDARD 17)

Expand Down Expand Up @@ -100,6 +95,8 @@ target_link_libraries(androidsqlite
${STATIC_ICUI18N_LIBRARY}
${STATIC_ICUUC_LIBRARY}
${STATIC_ICUDATA_LIBRARY}
-ldl
-lpthread
)

include_directories(${JNI_INCLUDE_DIRS})
Expand Down Expand Up @@ -142,8 +139,6 @@ if (CMAKE_HOST_SYSTEM_NAME MATCHES "Linux")
target_link_libraries(nativeruntime
-static-libgcc
-static-libstdc++
-ldl
-lpthread
-Wl,--no-undefined # print an error if there are any undefined symbols
)
endif()
@@ -1,19 +1,22 @@
package org.robolectric.android.controller;

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertThrows;

import android.app.Application;
import android.content.ContentProviderClient;
import android.content.ContentResolver;
import android.content.pm.PathPermission;
import android.content.pm.ProviderInfo;
import android.net.Uri;
import android.os.Build;
import androidx.test.core.app.ApplicationProvider;
import androidx.test.ext.junit.runners.AndroidJUnit4;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.robolectric.Robolectric;
import org.robolectric.RuntimeEnvironment;
import org.robolectric.shadows.testing.TestContentProvider1;
import org.robolectric.shadows.testing.TestContentProvider3And4;

Expand Down Expand Up @@ -42,7 +45,7 @@ public void shouldInitializeFromManifestProviderInfo() throws Exception {
assertThat(myContentProvider.getReadPermission()).isEqualTo("READ_PERMISSION");
assertThat(myContentProvider.getWritePermission()).isEqualTo("WRITE_PERMISSION");

assertThat(myContentProvider.getPathPermissions()).asList().hasSize(1);
assertThat(myContentProvider.getPathPermissions()).hasLength(1);
PathPermission pathPermission = myContentProvider.getPathPermissions()[0];
assertThat(pathPermission.getPath()).isEqualTo("/path/*");
assertThat(pathPermission.getType()).isEqualTo(PathPermission.PATTERN_SIMPLE_GLOB);
Expand All @@ -55,10 +58,10 @@ public void shouldRegisterWithContentResolver() throws Exception {
controller.create().get();

ContentProviderClient client =
contentResolver.acquireContentProviderClient(
"org.robolectric.authority1");
client.query(Uri.parse("something"), new String[]{"title"}, "*", new String[]{}, "created");
contentResolver.acquireContentProviderClient("org.robolectric.authority1");
client.query(Uri.parse("something"), new String[] {"title"}, "*", new String[] {}, "created");
assertThat(controller.get().transcript).containsExactly("onCreate", "query for something");
close(client);
}

@Test
Expand All @@ -70,6 +73,7 @@ public void shouldResolveProvidersWithMultipleAuthorities() throws Exception {
contentResolver.acquireContentProviderClient("org.robolectric.authority3");
client.query(Uri.parse("something"), new String[] {"title"}, "*", new String[] {}, "created");
assertThat(contentProvider.transcript).containsExactly("onCreate", "query for something");
close(client);
}

@Test
Expand Down Expand Up @@ -98,9 +102,11 @@ public void withoutManifest_shouldRegisterWithContentResolver() throws Exception
providerInfo.authority = "some-authority";
controller.create(providerInfo);

ContentProviderClient client = contentResolver.acquireContentProviderClient(providerInfo.authority);
client.query(Uri.parse("something"), new String[]{"title"}, "*", new String[]{}, "created");
ContentProviderClient client =
contentResolver.acquireContentProviderClient(providerInfo.authority);
client.query(Uri.parse("something"), new String[] {"title"}, "*", new String[] {}, "created");
assertThat(controller.get().transcript).containsExactly("onCreate", "query for something");
close(client);
}

@Test
Expand All @@ -111,11 +117,17 @@ public void contentProviderShouldBeCreatedBeforeBeingRegistered() throws Excepti
ContentProviderClient contentProviderClient =
contentResolver.acquireContentProviderClient("x-authority");
assertThat(contentProviderClient.getLocalContentProvider()).isSameInstanceAs(xContentProvider);
close(contentProviderClient);
}

@Test(expected = IllegalArgumentException.class)
@Test
public void createContentProvider_nullAuthority() throws Exception {
Robolectric.buildContentProvider(XContentProvider.class).create(new ProviderInfo()).get();
assertThrows(
IllegalArgumentException.class,
() ->
Robolectric.buildContentProvider(XContentProvider.class)
.create(new ProviderInfo())
.get());
}

static class XContentProvider extends TestContentProvider1 {
Expand All @@ -129,9 +141,20 @@ public boolean onCreate() {
contentProviderClient == null
? "x-authority" + " not registered" + " yet"
: "x-authority" + " is registered");
if (contentProviderClient != null) {
close(contentProviderClient);
}
return false;
}
}

static class NotInManifestContentProvider extends TestContentProvider1 {}

private static void close(ContentProviderClient client) {
if (RuntimeEnvironment.getApiLevel() > Build.VERSION_CODES.M) {
client.close();
} else {
client.release();
}
}
}
Expand Up @@ -39,6 +39,7 @@ public void setUp() throws Exception {
@After
public void tearDown() {
database.close();
cursor.close();
}

@Test
Expand Down Expand Up @@ -482,6 +483,7 @@ private Cursor createCursor() {

private void setupEmptyResult() {
database.execSQL("DELETE FROM table_name;");
cursor.close();
cursor = createCursor();
}

Expand Down

0 comments on commit dccf8ef

Please sign in to comment.