[caBLE] Fix a bug that keep alive frames are not handled correctly.

If there are more than 1 keep alive frames received before getting
the ack on request, the second will be thought as a continuation
fragment and fail with "Malformed Frame Continuation Fragment".

Change-Id: I69fb5c024f14fe19bfc60da2a2392b663bdcea66
Bug: 880053
Reviewed-on: https://chromium-review.googlesource.com/1237267
Commit-Queue: Yuankai Yu <yyk@google.com>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593644}

device/fido/ble/fido_ble_transaction.cc

@@ -115,8 +115,13 @@ void FidoBleTransaction::OnResponseFragment(std::vector<uint8_t> data) {
   // It is possible to receive the last response fragment before the write of
   // the last request fragment has been acknowledged. If this is the case, do
   // not run the completion callback yet.
-  if (!has_pending_request_fragment_write_)
+  // It is OK to process keep alive frames before the request frame is
+  // acknowledged.
+  if (!has_pending_request_fragment_write_ ||
+      response_frame_assembler_->GetFrame()->command() ==
+          FidoBleDeviceCommand::kKeepAlive) {
     ProcessResponseFrame();
+  }
 }
 
 void FidoBleTransaction::ProcessResponseFrame() {

device/fido/ble/fido_ble_transaction_unittest.cc

@@ -132,6 +132,38 @@ TEST_F(FidoBleTransactionTest, WriteRequestFrame_DelayedWriteAck) {
   EXPECT_EQ(frame, receiver.value());
 }
 
+// Tests a scenario where keep alive frames are obtained before the control
+// point write was acknowledged. The keep alive should be processed.
+TEST_F(FidoBleTransactionTest, WriteRequestFrame_DelayedWriteAck_KeepAlive) {
+  FidoBleConnection::WriteCallback delayed_write_callback;
+
+  EXPECT_CALL(connection(), WriteControlPointPtr)
+      .WillOnce(::testing::Invoke(
+          [&](auto&&, auto* cb) { delayed_write_callback = std::move(*cb); }));
+
+  FidoBleFrame frame(FidoBleDeviceCommand::kPing, std::vector<uint8_t>(10));
+  FidoBleFrame tup_needed_frame(
+      FidoBleDeviceCommand::kKeepAlive,
+      {base::strict_cast<uint8_t>(FidoBleFrame::KeepaliveCode::TUP_NEEDED)});
+  FrameCallbackReceiver receiver;
+
+  // Send two keep alives then the actual response.
+  transaction().WriteRequestFrame(frame, receiver.callback());
+  for (auto&& byte_fragment : ToByteFragments(tup_needed_frame))
+    transaction().OnResponseFragment(std::move(byte_fragment));
+  for (auto&& byte_fragment : ToByteFragments(tup_needed_frame))
+    transaction().OnResponseFragment(std::move(byte_fragment));
+  for (auto&& byte_fragment : ToByteFragments(frame))
+    transaction().OnResponseFragment(std::move(byte_fragment));
+
+  scoped_task_environment().RunUntilIdle();
+  EXPECT_FALSE(receiver.was_called());
+
+  std::move(delayed_write_callback).Run(true);
+  receiver.WaitForCallback();
+  EXPECT_EQ(frame, receiver.value());
+}
+
 // Tests a case where the control point length is too small.
 TEST_F(FidoBleTransactionTest, WriteRequestFrame_ControlPointLength_TooSmall) {
   static constexpr uint16_t kTooSmallControlPointLength = 2u;