Merge pull request #7818 from brave/zero-peers-fix

Retry if no peers fetched
brave · Feb 5, 2021 · 047fe09 · 047fe09
2 parents 2818ddc + 00d5fa5
commit 047fe09
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 3 deletions.
diff --git a/components/ipfs/ipfs_navigation_throttle.cc b/components/ipfs/ipfs_navigation_throttle.cc
@@ -9,6 +9,7 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/rand_util.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "brave/components/ipfs/ipfs_constants.h"
 #include "brave/components/ipfs/ipfs_interstitial_controller_client.h"
@@ -27,6 +28,18 @@
 
 namespace {
 
+// Used to retry request if we got zero peers from ipfs service
+// Actual value will be generated randomly in range
+// (kMinimalPeersRetryIntervalMs, kPeersRetryRate*kMinimalPeersRetryIntervalMs)
+const int kMinimalPeersRetryIntervalMs = 50;
+const int kPeersRetryRate = 3;
+
+base::TimeDelta CalculatePeersRetryTime() {
+  return base::TimeDelta::FromMilliseconds(
+      base::RandInt(kMinimalPeersRetryIntervalMs,
+                    kPeersRetryRate * kMinimalPeersRetryIntervalMs));
+}
+
 // Used to scope the posted navigation task to the lifetime of |web_contents|.
 class IPFSWebContentsLifetimeHelper
     : public content::WebContentsUserData<IPFSWebContentsLifetimeHelper> {
@@ -107,15 +120,19 @@ IpfsNavigationThrottle::WillStartRequest() {
   // Check # of connected peers before using local node.
   if (is_local_mode && ipfs_service_->IsDaemonLaunched()) {
     resume_pending_ = true;
-    ipfs_service_->GetConnectedPeers(
-        base::BindOnce(&IpfsNavigationThrottle::OnGetConnectedPeers,
-                       weak_ptr_factory_.GetWeakPtr()));
+    GetConnectedPeers();
     return content::NavigationThrottle::DEFER;
   }
 
   return content::NavigationThrottle::PROCEED;
 }
 
+void IpfsNavigationThrottle::GetConnectedPeers() {
+  ipfs_service_->GetConnectedPeers(
+      base::BindOnce(&IpfsNavigationThrottle::OnGetConnectedPeers,
+                     weak_ptr_factory_.GetWeakPtr()));
+}
+
 void IpfsNavigationThrottle::OnGetConnectedPeers(
     bool success,
     const std::vector<std::string>& peers) {
@@ -130,6 +147,15 @@ void IpfsNavigationThrottle::OnGetConnectedPeers(
     return;
   }
 
+  if (success && peers.empty()) {
+    resume_pending_ = true;
+    base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
+        FROM_HERE,
+        base::BindOnce(&IpfsNavigationThrottle::GetConnectedPeers,
+                       weak_ptr_factory_.GetWeakPtr()),
+        CalculatePeersRetryTime());
+    return;
+  }
   // Show interstitial page if kIPFSAutoFallbackToGateway is not set to true,
   // which will cancel the deferred navigation.
   if (!pref_service_->FindPreference(kIPFSAutoFallbackToGateway) ||

diff --git a/components/ipfs/ipfs_navigation_throttle.h b/components/ipfs/ipfs_navigation_throttle.h
@@ -48,6 +48,10 @@ class IpfsNavigationThrottle : public content::NavigationThrottle {
  private:
   FRIEND_TEST_ALL_PREFIXES(IpfsNavigationThrottleUnitTest,
                            DeferUntilIpfsProcessLaunched);
+  FRIEND_TEST_ALL_PREFIXES(IpfsNavigationThrottleUnitTest,
+                           DeferUntilPeersFetched);
+
+  void GetConnectedPeers();
   void ShowInterstitial();
   void LoadPublicGatewayURL();
   void OnGetConnectedPeers(bool success, const std::vector<std::string>& peers);

diff --git a/components/ipfs/ipfs_navigation_throttle_unittest.cc b/components/ipfs/ipfs_navigation_throttle_unittest.cc
@@ -197,6 +197,49 @@ TEST_F(IpfsNavigationThrottleUnitTest, SequentialRequests) {
   ASSERT_TRUE(service->WasConnectedPeersCalledForTest());
 }
 
+TEST_F(IpfsNavigationThrottleUnitTest, DeferUntilPeersFetched) {
+  profile()->GetPrefs()->SetInteger(
+      kIPFSResolveMethod, static_cast<int>(IPFSResolveMethodTypes::IPFS_LOCAL));
+  auto* service = ipfs_service(profile());
+  ASSERT_TRUE(service);
+  service->SetSkipGetConnectedPeersCallbackForTest(true);
+
+  service->SetAllowIpfsLaunchForTest(true);
+  service->RunLaunchDaemonCallbackForTest(true);
+
+  bool was_navigation_resumed1 = false;
+  auto throttle1 = CreateDeferredNavigation(
+      service,
+      base::BindLambdaForTesting([&]() { was_navigation_resumed1 = true; }));
+
+  bool was_navigation_resumed2 = false;
+  auto throttle2 = CreateDeferredNavigation(
+      service,
+      base::BindLambdaForTesting([&]() { was_navigation_resumed2 = true; }));
+  EXPECT_FALSE(was_navigation_resumed1);
+  EXPECT_FALSE(was_navigation_resumed2);
+
+  auto peers = std::vector<std::string>();
+  throttle1->OnGetConnectedPeers(true, peers);
+  EXPECT_FALSE(was_navigation_resumed1);
+  EXPECT_FALSE(was_navigation_resumed2);
+
+  throttle2->OnGetConnectedPeers(true, peers);
+  EXPECT_FALSE(was_navigation_resumed1);
+  EXPECT_FALSE(was_navigation_resumed2);
+
+  peers = std::vector<std::string>{
+      "/ip4/101.101.101.101/tcp/4001/p2p/"
+      "QmaCpDMGvV2BGHeYERUEnRQAwe3N8SzbUtfsmvsqQLuvuJ"};
+  throttle1->OnGetConnectedPeers(true, peers);
+  EXPECT_TRUE(was_navigation_resumed1);
+  EXPECT_FALSE(was_navigation_resumed2);
+
+  throttle2->OnGetConnectedPeers(true, peers);
+  EXPECT_TRUE(was_navigation_resumed1);
+  EXPECT_TRUE(was_navigation_resumed2);
+}
+
 TEST_F(IpfsNavigationThrottleUnitTest, DeferUntilIpfsProcessLaunched) {
   profile()->GetPrefs()->SetInteger(
       kIPFSResolveMethod, static_cast<int>(IPFSResolveMethodTypes::IPFS_LOCAL));