From c2370f7faf266b957b641af14f38d4a7fead659e Mon Sep 17 00:00:00 2001
From: John Knox <jknox@fb.com>
Date: Fri, 6 Dec 2019 05:27:28 -0800
Subject: [PATCH] Speed up re-connect after re-opening app

Summary:
The problem is that whenever an app is shutdown, and then reopened, the flipper dir gets reset when getting the CSR for connecting to flipper.
This causes the first connection attempt to fail always, and it goes through the whole cert exchange, taking longer than necessary.

Fixes it by loading the csr from disk if it's not loaded yet, without blowing away the whole certs state.

A side effect of this would be that as long as some file exists where the csr lives, flipper state would never get reset, so it wouldn't be able to fix itself automatically anymore. To keep that working, I've made `resetFlipperDir()` public and am calling it explicitly when starting certificate exchange. This should ensure that we still reset when we need to, but not unnecessarily.

The reason it went wrong is that getCSR used to be called only at cert exchange, when resetting and generating a new one was always desirable. However, when we shipped the fix for changeable android serials, it started to be used as a normal getter.

Reviewed By: timur-valiev

Differential Revision: D18834806

fbshipit-source-id: 56ca7e03e1aa9011f836bc9c021cf3048f7dc1e4
---
 xplat/Flipper/ConnectionContextStore.cpp       | 17 +++++++++++++++--
 xplat/Flipper/ConnectionContextStore.h         |  2 +-
 xplat/Flipper/FlipperConnectionManagerImpl.cpp |  4 ++++
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/xplat/Flipper/ConnectionContextStore.cpp b/xplat/Flipper/ConnectionContextStore.cpp
index b3697dd81..b1d448ab2 100644
--- a/xplat/Flipper/ConnectionContextStore.cpp
+++ b/xplat/Flipper/ConnectionContextStore.cpp
@@ -44,10 +44,19 @@ bool ConnectionContextStore::hasRequiredFiles() {
 }
 
 std::string ConnectionContextStore::getCertificateSigningRequest() {
+  // Use in-memory CSR if already loaded
   if (csr != "") {
     return csr;
   }
-  resetFlipperDir();
+
+  // Attempt to load existing CSR from previous run of the app
+  csr = loadStringFromFile(absoluteFilePath(CSR_FILE_NAME));
+  if (csr != "") {
+    return csr;
+  }
+
+  // Clean all state and generate a new one
+  resetState();
   bool success = generateCertSigningRequest(
       deviceData_.appId.c_str(),
       absoluteFilePath(CSR_FILE_NAME).c_str(),
@@ -104,7 +113,11 @@ std::string ConnectionContextStore::getCertificateDirectoryPath() {
   return absoluteFilePath("");
 }
 
-bool ConnectionContextStore::resetFlipperDir() {
+bool ConnectionContextStore::resetState() {
+  // Clear in-memory state
+  csr = "";
+
+  // Delete state from disk
   std::string dirPath = absoluteFilePath("");
   struct stat info;
   if (stat(dirPath.c_str(), &info) != 0) {
diff --git a/xplat/Flipper/ConnectionContextStore.h b/xplat/Flipper/ConnectionContextStore.h
index eb4043eee..bb4baa9bb 100644
--- a/xplat/Flipper/ConnectionContextStore.h
+++ b/xplat/Flipper/ConnectionContextStore.h
@@ -27,13 +27,13 @@ public:
   std::string getCertificateDirectoryPath();
   std::string getDeviceId();
   void storeConnectionConfig(folly::dynamic& config);
+  bool resetState();
 
  private:
   DeviceData deviceData_;
   std::string csr = "";
 
   std::string absoluteFilePath(const char* filename);
-  bool resetFlipperDir();
 };
 
 } // namespace flipper
diff --git a/xplat/Flipper/FlipperConnectionManagerImpl.cpp b/xplat/Flipper/FlipperConnectionManagerImpl.cpp
index 8458a458b..9cbcfb5e9 100644
--- a/xplat/Flipper/FlipperConnectionManagerImpl.cpp
+++ b/xplat/Flipper/FlipperConnectionManagerImpl.cpp
@@ -187,6 +187,10 @@ void FlipperConnectionManagerImpl::doCertificateExchange() {
           .get();
   connectingInsecurely->complete();
 
+  auto resettingState = flipperState_->start("Reset state");
+  contextStore_->resetState();
+  resettingState->complete();
+
   requestSignedCertFromFlipper();
 }