Skip to content

Commit fb5a496

Browse files
author
Jeff Brown
committed
Prefetch column names in bulk cursor adaptor.
If the remote end of a bulk cursor died, then it was possible for getColumnNames() to return null, violating the invariant that it never returns null. As a result, the application could crash in strange ways due to an NPE. Since we are often interested in the column names anyhow, prefetch them when setting up the bulk cursor adaptor. This way, a remote cursor will never return null even if the remote end died. It is possible for an application to continue to use a remote cursor even after the provider has died unless it needs to requery it for some reason. Of course at that point, bad things will happen... but usually the app is better prepared for it than if it just randomly encounters a null array of column names. This change also optimizes the bulk cursor adaptor to return the initial cursor window back to the client, potentially saving an extra RPC. Because the communication protocol between the CursorToBulkCursorAdaptor and BulkCursorToCursorAdaptor was getting a little hard to follow, introduced a new type called BulkCursorDescriptor to hold all of the necessary parameters. Deleted several unnecessary IBulkCursor methods that are never actually called remotely. Bug: 6168809 Change-Id: I9aaf6f067c6434a575e2fdbf678243d5ad10755f
1 parent 58984b0 commit fb5a496

File tree

9 files changed

+169
-226
lines changed

9 files changed

+169
-226
lines changed

core/java/android/content/ContentProviderNative.java

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package android.content;
1818

1919
import android.content.res.AssetFileDescriptor;
20+
import android.database.BulkCursorDescriptor;
2021
import android.database.BulkCursorNative;
2122
import android.database.BulkCursorToCursorAdaptor;
2223
import android.database.Cursor;
@@ -113,20 +114,14 @@ public boolean onTransact(int code, Parcel data, Parcel reply, int flags)
113114
if (cursor != null) {
114115
CursorToBulkCursorAdaptor adaptor = new CursorToBulkCursorAdaptor(
115116
cursor, observer, getProviderName());
116-
final IBinder binder = adaptor.asBinder();
117-
final int count = adaptor.count();
118-
final int index = BulkCursorToCursorAdaptor.findRowIdColumnIndex(
119-
adaptor.getColumnNames());
120-
final boolean wantsAllOnMoveCalls = adaptor.getWantsAllOnMoveCalls();
117+
BulkCursorDescriptor d = adaptor.getBulkCursorDescriptor();
121118

122119
reply.writeNoException();
123-
reply.writeStrongBinder(binder);
124-
reply.writeInt(count);
125-
reply.writeInt(index);
126-
reply.writeInt(wantsAllOnMoveCalls ? 1 : 0);
120+
reply.writeInt(1);
121+
d.writeToParcel(reply, Parcelable.PARCELABLE_WRITE_RETURN_VALUE);
127122
} else {
128123
reply.writeNoException();
129-
reply.writeStrongBinder(null);
124+
reply.writeInt(0);
130125
}
131126

132127
return true;
@@ -369,12 +364,9 @@ public Cursor query(Uri url, String[] projection, String selection,
369364

370365
DatabaseUtils.readExceptionFromParcel(reply);
371366

372-
IBulkCursor bulkCursor = BulkCursorNative.asInterface(reply.readStrongBinder());
373-
if (bulkCursor != null) {
374-
int rowCount = reply.readInt();
375-
int idColumnPosition = reply.readInt();
376-
boolean wantsAllOnMoveCalls = reply.readInt() != 0;
377-
adaptor.initialize(bulkCursor, rowCount, idColumnPosition, wantsAllOnMoveCalls);
367+
if (reply.readInt() != 0) {
368+
BulkCursorDescriptor d = BulkCursorDescriptor.CREATOR.createFromParcel(reply);
369+
adaptor.initialize(d);
378370
} else {
379371
adaptor.close();
380372
adaptor = null;

core/java/android/database/AbstractCursor.java

Lines changed: 32 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,39 @@
3333
public abstract class AbstractCursor implements CrossProcessCursor {
3434
private static final String TAG = "Cursor";
3535

36-
DataSetObservable mDataSetObservable = new DataSetObservable();
37-
ContentObservable mContentObservable = new ContentObservable();
36+
/**
37+
* @deprecated This is never updated by this class and should not be used
38+
*/
39+
@Deprecated
40+
protected HashMap<Long, Map<String, Object>> mUpdatedRows;
41+
42+
protected int mPos;
43+
44+
/**
45+
* This must be set to the index of the row ID column by any
46+
* subclass that wishes to support updates.
47+
*/
48+
protected int mRowIdColumnIndex;
49+
50+
/**
51+
* If {@link #mRowIdColumnIndex} is not -1 this contains contains the value of
52+
* the column at {@link #mRowIdColumnIndex} for the current row this cursor is
53+
* pointing at.
54+
*/
55+
protected Long mCurrentRowID;
56+
57+
protected boolean mClosed;
58+
protected ContentResolver mContentResolver;
59+
private Uri mNotifyUri;
60+
61+
private final Object mSelfObserverLock = new Object();
62+
private ContentObserver mSelfObserver;
63+
private boolean mSelfObserverRegistered;
3864

39-
Bundle mExtras = Bundle.EMPTY;
65+
private DataSetObservable mDataSetObservable = new DataSetObservable();
66+
private ContentObservable mContentObservable = new ContentObservable();
67+
68+
private Bundle mExtras = Bundle.EMPTY;
4069

4170
/* -------------------------------------------------------- */
4271
/* These need to be implemented by subclasses */
@@ -415,30 +444,4 @@ public void onChange(boolean selfChange) {
415444
}
416445
}
417446
}
418-
419-
/**
420-
* @deprecated This is never updated by this class and should not be used
421-
*/
422-
@Deprecated
423-
protected HashMap<Long, Map<String, Object>> mUpdatedRows;
424-
425-
/**
426-
* This must be set to the index of the row ID column by any
427-
* subclass that wishes to support updates.
428-
*/
429-
protected int mRowIdColumnIndex;
430-
431-
protected int mPos;
432-
/**
433-
* If {@link #mRowIdColumnIndex} is not -1 this contains contains the value of
434-
* the column at {@link #mRowIdColumnIndex} for the current row this cursor is
435-
* pointing at.
436-
*/
437-
protected Long mCurrentRowID;
438-
protected ContentResolver mContentResolver;
439-
protected boolean mClosed = false;
440-
private Uri mNotifyUri;
441-
private ContentObserver mSelfObserver;
442-
final private Object mSelfObserverLock = new Object();
443-
private boolean mSelfObserverRegistered;
444447
}
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
/*
2+
* Copyright (C) 2012 The Android Open Source Project
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package android.database;
18+
19+
import android.os.Parcel;
20+
import android.os.Parcelable;
21+
22+
/**
23+
* Describes the properties of a {@link CursorToBulkCursorAdaptor} that are
24+
* needed to initialize its {@link BulkCursorToCursorAdaptor} counterpart on the client's end.
25+
*
26+
* {@hide}
27+
*/
28+
public final class BulkCursorDescriptor implements Parcelable {
29+
public static final Parcelable.Creator<BulkCursorDescriptor> CREATOR =
30+
new Parcelable.Creator<BulkCursorDescriptor>() {
31+
@Override
32+
public BulkCursorDescriptor createFromParcel(Parcel in) {
33+
BulkCursorDescriptor d = new BulkCursorDescriptor();
34+
d.readFromParcel(in);
35+
return d;
36+
}
37+
38+
@Override
39+
public BulkCursorDescriptor[] newArray(int size) {
40+
return new BulkCursorDescriptor[size];
41+
}
42+
};
43+
44+
public IBulkCursor cursor;
45+
public String[] columnNames;
46+
public boolean wantsAllOnMoveCalls;
47+
public int count;
48+
public CursorWindow window;
49+
50+
@Override
51+
public int describeContents() {
52+
return 0;
53+
}
54+
55+
@Override
56+
public void writeToParcel(Parcel out, int flags) {
57+
out.writeStrongBinder(cursor.asBinder());
58+
out.writeStringArray(columnNames);
59+
out.writeInt(wantsAllOnMoveCalls ? 1 : 0);
60+
out.writeInt(count);
61+
if (window != null) {
62+
out.writeInt(1);
63+
window.writeToParcel(out, flags);
64+
} else {
65+
out.writeInt(0);
66+
}
67+
}
68+
69+
public void readFromParcel(Parcel in) {
70+
cursor = BulkCursorNative.asInterface(in.readStrongBinder());
71+
columnNames = in.readStringArray();
72+
wantsAllOnMoveCalls = in.readInt() != 0;
73+
count = in.readInt();
74+
if (in.readInt() != 0) {
75+
window = CursorWindow.CREATOR.createFromParcel(in);
76+
}
77+
}
78+
}

core/java/android/database/BulkCursorNative.java

Lines changed: 0 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -72,26 +72,6 @@ public boolean onTransact(int code, Parcel data, Parcel reply, int flags)
7272
return true;
7373
}
7474

75-
case COUNT_TRANSACTION: {
76-
data.enforceInterface(IBulkCursor.descriptor);
77-
int count = count();
78-
reply.writeNoException();
79-
reply.writeInt(count);
80-
return true;
81-
}
82-
83-
case GET_COLUMN_NAMES_TRANSACTION: {
84-
data.enforceInterface(IBulkCursor.descriptor);
85-
String[] columnNames = getColumnNames();
86-
reply.writeNoException();
87-
reply.writeInt(columnNames.length);
88-
int length = columnNames.length;
89-
for (int i = 0; i < length; i++) {
90-
reply.writeString(columnNames[i]);
91-
}
92-
return true;
93-
}
94-
9575
case DEACTIVATE_TRANSACTION: {
9676
data.enforceInterface(IBulkCursor.descriptor);
9777
deactivate();
@@ -125,14 +105,6 @@ public boolean onTransact(int code, Parcel data, Parcel reply, int flags)
125105
return true;
126106
}
127107

128-
case WANTS_ON_MOVE_TRANSACTION: {
129-
data.enforceInterface(IBulkCursor.descriptor);
130-
boolean result = getWantsAllOnMoveCalls();
131-
reply.writeNoException();
132-
reply.writeInt(result ? 1 : 0);
133-
return true;
134-
}
135-
136108
case GET_EXTRAS_TRANSACTION: {
137109
data.enforceInterface(IBulkCursor.descriptor);
138110
Bundle extras = getExtras();
@@ -217,52 +189,6 @@ public void onMove(int position) throws RemoteException {
217189
}
218190
}
219191

220-
public int count() throws RemoteException
221-
{
222-
Parcel data = Parcel.obtain();
223-
Parcel reply = Parcel.obtain();
224-
try {
225-
data.writeInterfaceToken(IBulkCursor.descriptor);
226-
227-
boolean result = mRemote.transact(COUNT_TRANSACTION, data, reply, 0);
228-
DatabaseUtils.readExceptionFromParcel(reply);
229-
230-
int count;
231-
if (result == false) {
232-
count = -1;
233-
} else {
234-
count = reply.readInt();
235-
}
236-
return count;
237-
} finally {
238-
data.recycle();
239-
reply.recycle();
240-
}
241-
}
242-
243-
public String[] getColumnNames() throws RemoteException
244-
{
245-
Parcel data = Parcel.obtain();
246-
Parcel reply = Parcel.obtain();
247-
try {
248-
data.writeInterfaceToken(IBulkCursor.descriptor);
249-
250-
mRemote.transact(GET_COLUMN_NAMES_TRANSACTION, data, reply, 0);
251-
DatabaseUtils.readExceptionFromParcel(reply);
252-
253-
String[] columnNames = null;
254-
int numColumns = reply.readInt();
255-
columnNames = new String[numColumns];
256-
for (int i = 0; i < numColumns; i++) {
257-
columnNames[i] = reply.readString();
258-
}
259-
return columnNames;
260-
} finally {
261-
data.recycle();
262-
reply.recycle();
263-
}
264-
}
265-
266192
public void deactivate() throws RemoteException
267193
{
268194
Parcel data = Parcel.obtain();
@@ -317,23 +243,6 @@ public int requery(IContentObserver observer) throws RemoteException {
317243
}
318244
}
319245

320-
public boolean getWantsAllOnMoveCalls() throws RemoteException {
321-
Parcel data = Parcel.obtain();
322-
Parcel reply = Parcel.obtain();
323-
try {
324-
data.writeInterfaceToken(IBulkCursor.descriptor);
325-
326-
mRemote.transact(WANTS_ON_MOVE_TRANSACTION, data, reply, 0);
327-
DatabaseUtils.readExceptionFromParcel(reply);
328-
329-
int result = reply.readInt();
330-
return result != 0;
331-
} finally {
332-
data.recycle();
333-
reply.recycle();
334-
}
335-
}
336-
337246
public Bundle getExtras() throws RemoteException {
338247
if (mExtras == null) {
339248
Parcel data = Parcel.obtain();

core/java/android/database/BulkCursorToCursorAdaptor.java

Lines changed: 9 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -30,34 +30,23 @@ public final class BulkCursorToCursorAdaptor extends AbstractWindowedCursor {
3030

3131
private SelfContentObserver mObserverBridge = new SelfContentObserver(this);
3232
private IBulkCursor mBulkCursor;
33-
private int mCount;
3433
private String[] mColumns;
3534
private boolean mWantsAllOnMoveCalls;
35+
private int mCount;
3636

3737
/**
3838
* Initializes the adaptor.
3939
* Must be called before first use.
4040
*/
41-
public void initialize(IBulkCursor bulkCursor, int count, int idIndex,
42-
boolean wantsAllOnMoveCalls) {
43-
mBulkCursor = bulkCursor;
44-
mColumns = null; // lazily retrieved
45-
mCount = count;
46-
mRowIdColumnIndex = idIndex;
47-
mWantsAllOnMoveCalls = wantsAllOnMoveCalls;
48-
}
49-
50-
/**
51-
* Returns column index of "_id" column, or -1 if not found.
52-
*/
53-
public static int findRowIdColumnIndex(String[] columnNames) {
54-
int length = columnNames.length;
55-
for (int i = 0; i < length; i++) {
56-
if (columnNames[i].equals("_id")) {
57-
return i;
58-
}
41+
public void initialize(BulkCursorDescriptor d) {
42+
mBulkCursor = d.cursor;
43+
mColumns = d.columnNames;
44+
mRowIdColumnIndex = DatabaseUtils.findRowIdColumnIndex(mColumns);
45+
mWantsAllOnMoveCalls = d.wantsAllOnMoveCalls;
46+
mCount = d.count;
47+
if (d.window != null) {
48+
setWindow(d.window);
5949
}
60-
return -1;
6150
}
6251

6352
/**
@@ -169,14 +158,6 @@ public boolean requery() {
169158
public String[] getColumnNames() {
170159
throwIfCursorIsClosed();
171160

172-
if (mColumns == null) {
173-
try {
174-
mColumns = mBulkCursor.getColumnNames();
175-
} catch (RemoteException ex) {
176-
Log.e(TAG, "Unable to fetch column names because the remote process is dead");
177-
return null;
178-
}
179-
}
180161
return mColumns;
181162
}
182163

0 commit comments

Comments
 (0)