Added ClientEvent proto and structure for storing events in the factory.

google_apis/gcm/protocol/mcs.proto

Index: google_apis/gcm/protocol/mcs.proto
diff --git a/google_apis/gcm/protocol/mcs.proto b/google_apis/gcm/protocol/mcs.proto
index b8fb2d0276545ac48d49a1d3265a2ec8fdc95f52..3174a9d158bbce0025c6187e7422b547248ab250 100644
--- a/google_apis/gcm/protocol/mcs.proto
+++ b/google_apis/gcm/protocol/mcs.proto
@@ -69,6 +69,38 @@ message HeartbeatConfig {
   optional int32 interval_ms = 3;
 }
 
+// This message must match the definition on the receiving end in GCM. That is
+// defined in google3/buzz/mobile/proto/gtalk_core.proto.

[Peter Beverloo, 2016/11/28 15:05:15]

Let's not refer to google3 paths, as useful as may be.

The concern you're expressing goes for any other message in this file as well.
Since we're the MCS client for a server maintained elsewhere, it's up to the
mutual OWNERS to ensure that this keeps in sync.

[harkness, 2016/12/01 10:34:34]

Done.

+message ClientEvent {
+  enum Type {
+    UNKNOWN = 0;
+    // Count of discarded events if the buffer filled up and was trimmed.
+    DISCARDED_EVENTS = 1;
+    // Failed connection event: the connection faliled to be established or we

[Peter Beverloo, 2016/11/28 15:05:15]

failed

[harkness, 2016/12/01 10:34:34]

Done.

+    // had a login error.
+    FAILED_CONNECTION = 2;
+    // Successful connection event: information about the last successful
+    // connection, including the time at which it was established.
+    SUCCESSFUL_CONNECTION = 3;
+  }
+
+  // Common fields [1-99]
+  optional Type type = 1;
+
+  // Fields for DISCARDED_EVENTS messages [100-199]
+  optional uint32 number_discarded_events = 100;
+
+  // Fields for FAILED_CONNECTION and SUCCESSFUL_CONNECTION messages [200-299]
+  optional int32 network_type = 200;
+  optional int32 network_port = 201;

[Peter Beverloo, 2016/11/28 15:05:15]

nit: this is unused. maybe mark as "reserved" until we use it? (Can we?)

[harkness, 2016/12/01 10:34:34]

Done.

+  optional uint64 time_connection_started_ms = 202;
+  optional uint64 time_connection_ended_ms = 203;
+  optional int32 error_code = 204;

[Peter Beverloo, 2016/11/28 15:05:15]

While the server-side does not, I would very much prefer if we can comment on
what values we're going to store in these int32s. E.g. is |error_code| going to
be a net::Error? (Do we even know yet?)

[harkness, 2016/12/01 10:34:34]

Done.

+
+  // Fields for SUCCESSFUL_CONNECTION messages [300-399]
+  optional uint64 time_connection_established_ms = 300;
+}
+
 /**
   TAG: 2
   */
@@ -113,6 +145,11 @@ message LoginRequest {
 
   optional int32 network_type = 17;
   optional int64 status = 18;
+
+  // 19, 20, and 21 are not currently populated by Chrome.

[Peter Beverloo, 2016/11/28 15:05:15]

Protobufs have a statement to reserve fields, which is good to use together with
a comment like this.

  reserved 19, 20, 21;

[harkness, 2016/12/01 10:34:34]

Done.

+
+  // Events recorded on the client after the last successful connection.
+  repeated ClientEvent client_event = 22;
 }
 
 /**