Fix cast_channel::KeepAliveDelegate DCHECK failure.

extensions/browser/api/cast_channel/keep_alive_delegate.cc

Index: extensions/browser/api/cast_channel/keep_alive_delegate.cc
diff --git a/extensions/browser/api/cast_channel/keep_alive_delegate.cc b/extensions/browser/api/cast_channel/keep_alive_delegate.cc
index f77c9b0beb00deceaa8f9a2011d1e6f27caf9d3a..d9ad15b773a519de5edaf37fb9c50368abd1bf60 100644
--- a/extensions/browser/api/cast_channel/keep_alive_delegate.cc
+++ b/extensions/browser/api/cast_channel/keep_alive_delegate.cc
@@ -176,21 +176,30 @@ void KeepAliveDelegate::OnError(ChannelError error_state) {
 }
 
 void KeepAliveDelegate::OnMessage(const CastMessage& message) {
-  DCHECK(started_);
   DCHECK(thread_checker_.CalledOnValidThread());
   VLOG(2) << "KeepAlive::OnMessage : " << message.payload_utf8();
 
-  ResetTimers();
-
-  if (NestedPayloadTypeEquals(kHeartbeatPingType, message)) {
-    VLOG(2) << "Received PING.";
-    SendKeepAliveMessage(pong_message_, kHeartbeatPongType);
-  } else if (NestedPayloadTypeEquals(kHeartbeatPongType, message)) {
-    VLOG(2) << "Received PONG.";
+  if (started_) {
+    ResetTimers();
+
+    if (NestedPayloadTypeEquals(kHeartbeatPingType, message)) {
+      VLOG(2) << "Received PING.";
+      SendKeepAliveMessage(pong_message_, kHeartbeatPongType);
+      return;
+    } else if (NestedPayloadTypeEquals(kHeartbeatPongType, message)) {
+      VLOG(2) << "Received PONG.";
+      return;
+    }
   } else {
-    // PING and PONG messages are intentionally suppressed from layers above.
-    inner_delegate_->OnMessage(message);
+    if (NestedPayloadTypeEquals(kHeartbeatPingType, message) ||
+        NestedPayloadTypeEquals(kHeartbeatPongType, message)) {
+      VLOG(2) << "Received PING or PONG before starting or after stopping.";
+      return;
+    }
   }
+
+  // PING and PONG messages are intentionally suppressed from layers above.

[Wez, 2017/01/04 00:10:17]

nit: This comment is not very clear, especially after the rewrite - perhaps
rephrase to "If we reach here then the message is neither PING nor PONG, so pass
it on for handling by the layer above."

IMO it'd be more straightforward to read wrt the suppression of ping/pongs if
you have the top-level conditionals be on the payload type, and then the
internal behaviour conditional on |started_|, though I appreciate that that
means checking |started_| in two distinct cases.

[miu, 2017/01/12 22:31:27]

Done.

Looking at this code again, I also couldn't help myself: The multiple calls to
NestedPayloadTypeEquals() were very wasteful! Each does lots of string copying
and JSON parsing and then throws away all that work after doing a simple string
equals. So, I changed it to a function that just does the parsing once and
extracts the needed field, and now we can compare it multiple times cheaply.

[Wez, 2017/01/13 22:37:11]

Nice :)

+  inner_delegate_->OnMessage(message);
 }
 
 void KeepAliveDelegate::Stop() {