Commit b31b5e58 authored by Jan Krcal's avatar Jan Krcal Committed by Commit Bot

Reland "[protobuf] Convert serialized_invalidation from nano to lite"

Compared to the original CL, this one fixes (in the second patch set) a
bug in PendingIvalidation.encodeToString() that caused frequent
NullPointerException crashes on canary. A corresponding unit-test is
added.

Bug: 782237
Change-Id: Ia2eacd4bc71122e4b3ca721b44f2b4472530861a
Reviewed-on: https://chromium-review.googlesource.com/970341Reviewed-by: default avatarTommy Nyquist <nyquist@chromium.org>
Commit-Queue: Jan Krcal <jkrcal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547691}
parent 407b3437
...@@ -207,10 +207,10 @@ if (is_android) { ...@@ -207,10 +207,10 @@ if (is_android) {
"//base:base_java", "//base:base_java",
"//components/signin/core/browser/android:java", "//components/signin/core/browser/android:java",
"//components/sync/android:sync_java", "//components/sync/android:sync_java",
"//third_party/android_protobuf:protobuf_nano_javalib",
"//third_party/cacheinvalidation:cacheinvalidation_javalib", "//third_party/cacheinvalidation:cacheinvalidation_javalib",
"//third_party/cacheinvalidation:cacheinvalidation_proto_java", "//third_party/cacheinvalidation:cacheinvalidation_proto_java",
"//third_party/jsr-305:jsr_305_javalib", "//third_party/jsr-305:jsr_305_javalib",
"//third_party/protobuf:protobuf_lite_javalib",
] ]
java_files = [ java_files = [
"android/java/src/org/chromium/components/invalidation/InvalidationClientService.java", "android/java/src/org/chromium/components/invalidation/InvalidationClientService.java",
...@@ -223,6 +223,7 @@ if (is_android) { ...@@ -223,6 +223,7 @@ if (is_android) {
sources = [ sources = [
"$proto_path/serialized_invalidation.proto", "$proto_path/serialized_invalidation.proto",
] ]
generate_lite = true
} }
android_library("javatests") { android_library("javatests") {
testonly = true testonly = true
......
...@@ -7,8 +7,6 @@ package org.chromium.components.invalidation; ...@@ -7,8 +7,6 @@ package org.chromium.components.invalidation;
import android.os.Bundle; import android.os.Bundle;
import android.util.Base64; import android.util.Base64;
import com.google.protobuf.nano.MessageNano;
import org.chromium.base.Log; import org.chromium.base.Log;
import org.chromium.base.VisibleForTesting; import org.chromium.base.VisibleForTesting;
import org.chromium.components.invalidation.SerializedInvalidation.Invalidation; import org.chromium.components.invalidation.SerializedInvalidation.Invalidation;
...@@ -79,12 +77,13 @@ public class PendingInvalidation { ...@@ -79,12 +77,13 @@ public class PendingInvalidation {
*/ */
public String encodeToString() { public String encodeToString() {
assert mObjectSource != 0; assert mObjectSource != 0;
Invalidation invalidation = new Invalidation(); Invalidation.Builder invalidationBuilder =
invalidation.objectSource = mObjectSource; Invalidation.newBuilder().setObjectSource(mObjectSource);
invalidation.objectId = mObjectId; // The following setters update internal state of |invalidationBuilder|.
invalidation.version = mVersion; if (mObjectId != null) invalidationBuilder.setObjectId(mObjectId);
invalidation.payload = mPayload; if (mVersion != 0L) invalidationBuilder.setVersion(mVersion);
return Base64.encodeToString(MessageNano.toByteArray(invalidation), Base64.DEFAULT); if (mPayload != null) invalidationBuilder.setPayload(mPayload);
return Base64.encodeToString(invalidationBuilder.build().toByteArray(), Base64.DEFAULT);
} }
/** /**
...@@ -95,8 +94,10 @@ public class PendingInvalidation { ...@@ -95,8 +94,10 @@ public class PendingInvalidation {
public static Bundle decodeToBundle(String encoded) { public static Bundle decodeToBundle(String encoded) {
Invalidation invalidation = decodeToInvalidation(encoded); Invalidation invalidation = decodeToInvalidation(encoded);
if (invalidation == null) return null; if (invalidation == null) return null;
return createBundle(invalidation.objectId, invalidation.objectSource, return createBundle(invalidation.hasObjectId() ? invalidation.getObjectId() : null,
invalidation.version != null ? invalidation.version : 0L, invalidation.payload); invalidation.getObjectSource(),
invalidation.hasVersion() ? invalidation.getVersion() : 0L,
invalidation.hasPayload() ? invalidation.getPayload() : null);
} }
/** /**
...@@ -107,8 +108,10 @@ public class PendingInvalidation { ...@@ -107,8 +108,10 @@ public class PendingInvalidation {
public static PendingInvalidation decodeToPendingInvalidation(String encoded) { public static PendingInvalidation decodeToPendingInvalidation(String encoded) {
Invalidation invalidation = decodeToInvalidation(encoded); Invalidation invalidation = decodeToInvalidation(encoded);
if (invalidation == null) return null; if (invalidation == null) return null;
return new PendingInvalidation(invalidation.objectId, invalidation.objectSource, return new PendingInvalidation(
invalidation.version, invalidation.payload); invalidation.hasObjectId() ? invalidation.getObjectId() : null,
invalidation.getObjectSource(), invalidation.getVersion(),
invalidation.hasPayload() ? invalidation.getPayload() : null);
} }
@Nullable @Nullable
...@@ -117,13 +120,13 @@ public class PendingInvalidation { ...@@ -117,13 +120,13 @@ public class PendingInvalidation {
byte[] decoded = Base64.decode(encoded, Base64.DEFAULT); byte[] decoded = Base64.decode(encoded, Base64.DEFAULT);
Invalidation invalidation; Invalidation invalidation;
try { try {
invalidation = MessageNano.mergeFrom(new Invalidation(), decoded); invalidation = Invalidation.parseFrom(decoded);
} catch (IOException e) { } catch (IOException e) {
Log.e(TAG, "Could not parse the serialized invalidations.", e); Log.e(TAG, "Could not parse the serialized invalidations.", e);
return null; return null;
} }
assert invalidation != null; assert invalidation != null;
if (invalidation.objectSource == null || invalidation.objectSource == 0) return null; if (!invalidation.hasObjectSource() || invalidation.getObjectSource() == 0) return null;
return invalidation; return invalidation;
} }
......
...@@ -20,48 +20,64 @@ import org.chromium.base.test.BaseRobolectricTestRunner; ...@@ -20,48 +20,64 @@ import org.chromium.base.test.BaseRobolectricTestRunner;
@RunWith(BaseRobolectricTestRunner.class) @RunWith(BaseRobolectricTestRunner.class)
@Config(manifest = Config.NONE) @Config(manifest = Config.NONE)
public class PendingInvalidationTest { public class PendingInvalidationTest {
private static String sObjecId = "ObjectId"; @Test
private static int sObjectSource = 4; public void testFullData() {
private static long sVersion = 5; String objectId = "ObjectId";
private static String sPayload = "Payload"; int objectSource = 4;
long version = 5;
String payload = "Payload";
doTestParseFromBundle(objectId, objectSource, version, payload);
doTestParseToAndFromProtocolBuffer(objectId, objectSource, version, payload);
doTestParseToAndFromProtocolBufferThroughBundle(objectId, objectSource, version, payload);
}
@Test @Test
public void testParseFromBundle() { public void testNoData() {
String objectId = null;
int objectSource = 4;
long version = 0L;
String payload = null;
doTestParseFromBundle(objectId, objectSource, version, payload);
doTestParseToAndFromProtocolBuffer(objectId, objectSource, version, payload);
doTestParseToAndFromProtocolBufferThroughBundle(objectId, objectSource, version, payload);
}
public void doTestParseFromBundle(
String objectId, int objectSource, long version, String payload) {
PendingInvalidation invalidation = PendingInvalidation invalidation =
new PendingInvalidation(sObjecId, sObjectSource, sVersion, sPayload); new PendingInvalidation(objectId, objectSource, version, payload);
Bundle bundle = Bundle bundle = PendingInvalidation.createBundle(objectId, objectSource, version, payload);
PendingInvalidation.createBundle(sObjecId, sObjectSource, sVersion, sPayload);
PendingInvalidation parsedInvalidation = new PendingInvalidation(bundle); PendingInvalidation parsedInvalidation = new PendingInvalidation(bundle);
assertEquals(sObjecId, parsedInvalidation.mObjectId); assertEquals(objectId, parsedInvalidation.mObjectId);
assertEquals(sObjectSource, parsedInvalidation.mObjectSource); assertEquals(objectSource, parsedInvalidation.mObjectSource);
assertEquals(sVersion, parsedInvalidation.mVersion); assertEquals(version, parsedInvalidation.mVersion);
assertEquals(sPayload, parsedInvalidation.mPayload); assertEquals(payload, parsedInvalidation.mPayload);
assertEquals(invalidation, parsedInvalidation); assertEquals(invalidation, parsedInvalidation);
} }
@Test public void doTestParseToAndFromProtocolBuffer(
public void testParseToAndFromProtocolBuffer() { String objectId, int objectSource, long version, String payload) {
PendingInvalidation invalidation = PendingInvalidation invalidation =
new PendingInvalidation(sObjecId, sObjectSource, sVersion, sPayload); new PendingInvalidation(objectId, objectSource, version, payload);
PendingInvalidation parsedInvalidation = PendingInvalidation parsedInvalidation =
PendingInvalidation.decodeToPendingInvalidation(invalidation.encodeToString()); PendingInvalidation.decodeToPendingInvalidation(invalidation.encodeToString());
assertEquals(sObjecId, parsedInvalidation.mObjectId); assertEquals(objectId, parsedInvalidation.mObjectId);
assertEquals(sObjectSource, parsedInvalidation.mObjectSource); assertEquals(objectSource, parsedInvalidation.mObjectSource);
assertEquals(sVersion, parsedInvalidation.mVersion); assertEquals(version, parsedInvalidation.mVersion);
assertEquals(sPayload, parsedInvalidation.mPayload); assertEquals(payload, parsedInvalidation.mPayload);
assertEquals(invalidation, parsedInvalidation); assertEquals(invalidation, parsedInvalidation);
} }
@Test public void doTestParseToAndFromProtocolBufferThroughBundle(
public void testParseToAndFromProtocolBufferThroughBundle() { String objectId, int objectSource, long version, String payload) {
PendingInvalidation invalidation = PendingInvalidation invalidation =
new PendingInvalidation(sObjecId, sObjectSource, sVersion, sPayload); new PendingInvalidation(objectId, objectSource, version, payload);
Bundle bundle = PendingInvalidation.decodeToBundle(invalidation.encodeToString()); Bundle bundle = PendingInvalidation.decodeToBundle(invalidation.encodeToString());
PendingInvalidation parsedInvalidation = new PendingInvalidation(bundle); PendingInvalidation parsedInvalidation = new PendingInvalidation(bundle);
assertEquals(sObjecId, parsedInvalidation.mObjectId); assertEquals(objectId, parsedInvalidation.mObjectId);
assertEquals(sObjectSource, parsedInvalidation.mObjectSource); assertEquals(objectSource, parsedInvalidation.mObjectSource);
assertEquals(sVersion, parsedInvalidation.mVersion); assertEquals(version, parsedInvalidation.mVersion);
assertEquals(sPayload, parsedInvalidation.mPayload); assertEquals(payload, parsedInvalidation.mPayload);
assertEquals(invalidation, parsedInvalidation); assertEquals(invalidation, parsedInvalidation);
} }
} }
...@@ -7,6 +7,8 @@ ...@@ -7,6 +7,8 @@
// //
syntax = "proto2"; syntax = "proto2";
package org.chromium.components.invalidation; package org.chromium.components.invalidation;
// TODO(jkrcal): Remove when protobuf 4.0 is out, https://crbug.com/800281.
option optimize_for = LITE_RUNTIME; option optimize_for = LITE_RUNTIME;
message Invalidation { message Invalidation {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment