BackForwardCache: Refactoring: Create independent files for CanStoreDocumentResult

This change is pure refactoring. This is preparation for letting
RenderFrameHostImpl::MarkNotRestoredWithReason accept
CanStoreDocumentResult.

We'd like to pass not only multiple reasons and blocklisted
features but also disallowed reasons at
MarkNotRestoredWithReason.

As CanStoreDocumentResult is an inner class, forward
declaration is impossible, then this CL moves the class to
an independent file.

Bug: 1004676
Change-Id: I80a59f8035bc0093f87840b0b85ef7f6c31441d2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1877502
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712905}

content/browser/BUILD.gn

@@ -850,6 +850,8 @@ jumbo_source_set("browser") {
     "font_list_async.cc",
     "frame_host/ancestor_throttle.cc",
     "frame_host/ancestor_throttle.h",
+    "frame_host/back_forward_cache_can_store_document_result.cc",
+    "frame_host/back_forward_cache_can_store_document_result.h",
     "frame_host/back_forward_cache_impl.cc",
     "frame_host/back_forward_cache_impl.h",
     "frame_host/back_forward_cache_metrics.cc",

content/browser/frame_host/back_forward_cache_can_store_document_result.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/browser/frame_host/back_forward_cache_can_store_document_result.h"
+
+#include "base/strings/string_util.h"
+#include "third_party/blink/public/common/scheduler/web_scheduler_tracked_feature.h"
+
+namespace content {
+
+namespace {
+
+using blink::scheduler::WebSchedulerTrackedFeature;
+
+std::string DescribeFeatures(uint64_t blocklisted_features) {
+  std::vector<std::string> features;
+  for (size_t i = 0;
+       i <= static_cast<size_t>(WebSchedulerTrackedFeature::kMaxValue); ++i) {
+    if (blocklisted_features & (1 << i)) {
+      features.push_back(blink::scheduler::FeatureToString(
+          static_cast<WebSchedulerTrackedFeature>(i)));
+    }
+  }
+  return base::JoinString(features, ", ");
+}
+
+}  // namespace
+
+bool BackForwardCacheCanStoreDocumentResult::CanStore() const {
+  return not_stored_reasons_.none();
+}
+
+std::string BackForwardCacheCanStoreDocumentResult::ToString() const {
+  using Reason = BackForwardCacheMetrics::NotRestoredReason;
+
+  if (CanStore())
+    return "Yes";
+
+  std::vector<std::string> reason_strs;
+
+  for (int i = 0; i <= static_cast<int>(Reason::kMaxValue); i++) {
+    if (!not_stored_reasons_.test(static_cast<size_t>(i)))
+      continue;
+
+    reason_strs.push_back(NotRestoredReasonToString(static_cast<Reason>(i)));
+  }
+
+  return "No: " + base::JoinString(reason_strs, ", ");
+}
+
+std::string BackForwardCacheCanStoreDocumentResult::NotRestoredReasonToString(
+    BackForwardCacheMetrics::NotRestoredReason reason) const {
+  using Reason = BackForwardCacheMetrics::NotRestoredReason;
+
+  switch (reason) {
+    case Reason::kNotMainFrame:
+      return "not a main frame";
+    case Reason::kBackForwardCacheDisabled:
+      return "BackForwardCache disabled";
+    case Reason::kRelatedActiveContentsExist:
+      return "related active contents exist";
+    case Reason::kHTTPStatusNotOK:
+      return "HTTP status is not OK";
+    case Reason::kSchemeNotHTTPOrHTTPS:
+      return "scheme is not HTTP or HTTPS";
+    case Reason::kLoading:
+      return "frame is not fully loaded";
+    case Reason::kWasGrantedMediaAccess:
+      return "frame was granted microphone or camera access";
+    case Reason::kBlocklistedFeatures:
+      return "blocklisted features: " + DescribeFeatures(blocklisted_features_);
+    case Reason::kDisableForRenderFrameHostCalled:
+      return "BackForwardCache::DisableForRenderFrameHost() was called";
+    case Reason::kDomainNotAllowed:
+      return "This domain is not allowed to be stored in BackForwardCache";
+    case Reason::kHTTPMethodNotGET:
+      return "HTTP method is not GET";
+    case Reason::kSubframeIsNavigating:
+      return "subframe navigation is in progress";
+    case Reason::kTimeout:
+      return "timeout";
+    case Reason::kCacheLimit:
+      return "cache limit";
+    case Reason::kJavaScriptExecution:
+      return "JavaScript execution";
+    case Reason::kRendererProcessKilled:
+      return "renderer process is killed";
+    case Reason::kRendererProcessCrashed:
+      return "renderer process crashed";
+    case Reason::kDialog:
+      return "dialog";
+    case Reason::kGrantedMediaStreamAccess:
+      return "granted media stream access";
+    case Reason::kSchedulerTrackedFeatureUsed:
+      return "scheduler tracked feature is used";
+    case Reason::kConflictingBrowsingInstance:
+      return "conflicting BrowsingInstance";
+    case Reason::kCacheFlushed:
+      return "cache flushed";
+    case Reason::kServiceWorkerVersionActivation:
+      return "service worker version is activated";
+  }
+}
+
+void BackForwardCacheCanStoreDocumentResult::No(
+    BackForwardCacheMetrics::NotRestoredReason reason) {
+  not_stored_reasons_.set(static_cast<size_t>(reason));
+}
+
+void BackForwardCacheCanStoreDocumentResult::NoDueToFeatures(
+    uint64_t features) {
+  not_stored_reasons_.set(static_cast<size_t>(
+      BackForwardCacheMetrics::NotRestoredReason::kBlocklistedFeatures));
+  blocklisted_features_ |= features;
+}
+
+BackForwardCacheCanStoreDocumentResult::BackForwardCacheCanStoreDocumentResult(
+    const NotStoredReasons& reasons)
+    : not_stored_reasons_(reasons) {}
+
+BackForwardCacheCanStoreDocumentResult::
+    BackForwardCacheCanStoreDocumentResult() = default;
+BackForwardCacheCanStoreDocumentResult::BackForwardCacheCanStoreDocumentResult(
+    BackForwardCacheCanStoreDocumentResult&&) = default;
+BackForwardCacheCanStoreDocumentResult& BackForwardCacheCanStoreDocumentResult::
+operator=(BackForwardCacheCanStoreDocumentResult&&) = default;
+BackForwardCacheCanStoreDocumentResult::
+    ~BackForwardCacheCanStoreDocumentResult() = default;
+
+}  // namespace content

content/browser/frame_host/back_forward_cache_can_store_document_result.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CONTENT_BROWSER_FRAME_HOST_BACK_FORWARD_CACHE_CAN_STORE_DOCUMENT_RESULT_H_
+#define CONTENT_BROWSER_FRAME_HOST_BACK_FORWARD_CACHE_CAN_STORE_DOCUMENT_RESULT_H_
+
+#include <bitset>
+
+#include "content/browser/frame_host/back_forward_cache_metrics.h"
+
+namespace content {
+
+// Represents the result whether the page could be stored in the back-forward
+// cache with the reasons.
+class BackForwardCacheCanStoreDocumentResult {
+ public:
+  using NotStoredReasons =
+      std::bitset<static_cast<size_t>(
+                      BackForwardCacheMetrics::NotRestoredReason::kMaxValue) +
+                  1ul>;
+
+  BackForwardCacheCanStoreDocumentResult();
+  BackForwardCacheCanStoreDocumentResult(const NotStoredReasons& reasons);
+
+  BackForwardCacheCanStoreDocumentResult(
+      BackForwardCacheCanStoreDocumentResult&&);
+  BackForwardCacheCanStoreDocumentResult& operator=(
+      BackForwardCacheCanStoreDocumentResult&&);
+  ~BackForwardCacheCanStoreDocumentResult();
+
+  void No(BackForwardCacheMetrics::NotRestoredReason reason);
+  void NoDueToFeatures(uint64_t features);
+
+  bool CanStore() const;
+  operator bool() const { return CanStore(); }
+
+  const NotStoredReasons& not_stored_reasons() const {
+    return not_stored_reasons_;
+  }
+  uint64_t blocklisted_features() const { return blocklisted_features_; }
+
+  std::string ToString() const;
+
+ private:
+  std::string NotRestoredReasonToString(
+      BackForwardCacheMetrics::NotRestoredReason reason) const;
+
+  NotStoredReasons not_stored_reasons_;
+  uint64_t blocklisted_features_ = 0;
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_FRAME_HOST_BACK_FORWARD_CACHE_CAN_STORE_DOCUMENT_RESULT_H_

content/browser/frame_host/back_forward_cache_impl.cc

@@ -8,7 +8,6 @@
 #include <string>
 
 #include "base/metrics/field_trial_params.h"
-#include "base/strings/string_split.h"
 #include "content/browser/frame_host/frame_tree_node.h"
 #include "content/browser/frame_host/render_frame_host_delegate.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
@@ -136,18 +135,6 @@ uint64_t GetDisallowedFeatures(RenderFrameHostImpl* rfh) {
   return result;
 }
 
-std::string DescribeFeatures(uint64_t blocklisted_features) {
-  std::vector<std::string> features;
-  for (size_t i = 0;
-       i <= static_cast<size_t>(WebSchedulerTrackedFeature::kMaxValue); ++i) {
-    if (blocklisted_features & (1 << i)) {
-      features.push_back(blink::scheduler::FeatureToString(
-          static_cast<WebSchedulerTrackedFeature>(i)));
-    }
-  }
-  return base::JoinString(features, ", ");
-}
-
 // The BackForwardCache feature is controlled via an experiment. This function
 // returns the allowed URLs where it is enabled. To enter the BackForwardCache
 // the URL of a document must have a host and a path matching with at least
@@ -178,102 +165,6 @@ BackForwardCacheImpl::Entry::Entry(
       render_view_hosts(std::move(render_view_hosts)) {}
 BackForwardCacheImpl::Entry::~Entry() {}
 
-bool BackForwardCacheImpl::CanStoreDocumentResult::CanStore() const {
-  return not_stored_reasons.none();
-}
-
-std::string BackForwardCacheImpl::CanStoreDocumentResult::ToString() const {
-  using Reason = BackForwardCacheMetrics::NotRestoredReason;
-
-  if (CanStore())
-    return "Yes";
-
-  std::vector<std::string> reason_strs;
-
-  for (int i = 0; i <= static_cast<int>(Reason::kMaxValue); i++) {
-    if (!not_stored_reasons.test(static_cast<size_t>(i)))
-      continue;
-
-    reason_strs.push_back(NotRestoredReasonToString(static_cast<Reason>(i)));
-  }
-
-  return "No: " + base::JoinString(reason_strs, ", ");
-}
-
-std::string
-BackForwardCacheImpl::CanStoreDocumentResult::NotRestoredReasonToString(
-    BackForwardCacheMetrics::NotRestoredReason reason) const {
-  using Reason = BackForwardCacheMetrics::NotRestoredReason;
-
-  switch (reason) {
-    case Reason::kNotMainFrame:
-      return "not a main frame";
-    case Reason::kBackForwardCacheDisabled:
-      return "BackForwardCache disabled";
-    case Reason::kRelatedActiveContentsExist:
-      return "related active contents exist";
-    case Reason::kHTTPStatusNotOK:
-      return "HTTP status is not OK";
-    case Reason::kSchemeNotHTTPOrHTTPS:
-      return "scheme is not HTTP or HTTPS";
-    case Reason::kLoading:
-      return "frame is not fully loaded";
-    case Reason::kWasGrantedMediaAccess:
-      return "frame was granted microphone or camera access";
-    case Reason::kBlocklistedFeatures:
-      return "blocklisted features: " + DescribeFeatures(blocklisted_features);
-    case Reason::kDisableForRenderFrameHostCalled:
-      return "BackForwardCache::DisableForRenderFrameHost() was called";
-    case Reason::kDomainNotAllowed:
-      return "This domain is not allowed to be stored in BackForwardCache";
-    case Reason::kHTTPMethodNotGET:
-      return "HTTP method is not GET";
-    case Reason::kSubframeIsNavigating:
-      return "subframe navigation is in progress";
-    case Reason::kTimeout:
-      return "timeout";
-    case Reason::kCacheLimit:
-      return "cache limit";
-    case Reason::kJavaScriptExecution:
-      return "JavaScript execution";
-    case Reason::kRendererProcessKilled:
-      return "renderer process is killed";
-    case Reason::kRendererProcessCrashed:
-      return "renderer process crashed";
-    case Reason::kDialog:
-      return "dialog";
-    case Reason::kGrantedMediaStreamAccess:
-      return "granted media stream access";
-    case Reason::kSchedulerTrackedFeatureUsed:
-      return "scheduler tracked feature is used";
-    case Reason::kConflictingBrowsingInstance:
-      return "conflicting BrowsingInstance";
-    case Reason::kCacheFlushed:
-      return "cache flushed";
-    case Reason::kServiceWorkerVersionActivation:
-      return "service worker version is activated";
-  }
-}
-
-void BackForwardCacheImpl::CanStoreDocumentResult::No(
-    BackForwardCacheMetrics::NotRestoredReason reason) {
-  not_stored_reasons.set(static_cast<size_t>(reason));
-}
-
-void BackForwardCacheImpl::CanStoreDocumentResult::NoDueToFeatures(
-    uint64_t features) {
-  not_stored_reasons.set(static_cast<size_t>(
-      BackForwardCacheMetrics::NotRestoredReason::kBlocklistedFeatures));
-  blocklisted_features |= features;
-}
-
-BackForwardCacheImpl::CanStoreDocumentResult::CanStoreDocumentResult() =
-    default;
-BackForwardCacheImpl::CanStoreDocumentResult::CanStoreDocumentResult(
-    const CanStoreDocumentResult&) = default;
-BackForwardCacheImpl::CanStoreDocumentResult::~CanStoreDocumentResult() =
-    default;
-
 BackForwardCacheTestDelegate::BackForwardCacheTestDelegate() {
   DCHECK(!g_bfcache_disabled_test_observer);
   g_bfcache_disabled_test_observer = this;
@@ -305,9 +196,9 @@ base::TimeDelta BackForwardCacheImpl::GetTimeToLiveInBackForwardCache() {
       kDefaultTimeToLiveInBackForwardCacheInSeconds));
 }
 
-BackForwardCacheImpl::CanStoreDocumentResult
-BackForwardCacheImpl::CanStoreDocument(RenderFrameHostImpl* rfh) {
-  CanStoreDocumentResult result;
+BackForwardCacheCanStoreDocumentResult BackForwardCacheImpl::CanStoreDocument(
+    RenderFrameHostImpl* rfh) {
+  BackForwardCacheCanStoreDocumentResult result;
 
   // Use the BackForwardCache only for the main frame.
   if (rfh->GetParent())
@@ -360,7 +251,7 @@ BackForwardCacheImpl::CanStoreDocument(RenderFrameHostImpl* rfh) {
 // Recursively checks whether this RenderFrameHost and all child frames
 // can be cached.
 void BackForwardCacheImpl::CanStoreRenderFrameHost(
-    CanStoreDocumentResult* result,
+    BackForwardCacheCanStoreDocumentResult* result,
     RenderFrameHostImpl* rfh) {
   if (!rfh->dom_content_loaded())
     result->No(BackForwardCacheMetrics::NotRestoredReason::kLoading);

content/browser/frame_host/back_forward_cache_impl.h

@@ -5,7 +5,6 @@
 #ifndef CONTENT_BROWSER_FRAME_HOST_BACK_FORWARD_CACHE_IMPL_H_
 #define CONTENT_BROWSER_FRAME_HOST_BACK_FORWARD_CACHE_IMPL_H_
 
-#include <bitset>
 #include <list>
 #include <memory>
 #include <unordered_map>
@@ -17,7 +16,7 @@
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
-#include "content/browser/frame_host/back_forward_cache_metrics.h"
+#include "content/browser/frame_host/back_forward_cache_can_store_document_result.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/back_forward_cache.h"
 #include "content/public/browser/global_routing_id.h"
@@ -74,37 +73,10 @@ class CONTENT_EXPORT BackForwardCacheImpl : public BackForwardCache {
   BackForwardCacheImpl();
   ~BackForwardCacheImpl();
 
-  struct CanStoreDocumentResult {
-    using NotStoredReasons =
-        std::bitset<static_cast<size_t>(
-                        BackForwardCacheMetrics::NotRestoredReason::kMaxValue) +
-                    1ul>;
-
-    CanStoreDocumentResult();
-    CanStoreDocumentResult(const CanStoreDocumentResult&);
-    ~CanStoreDocumentResult();
-
-    NotStoredReasons not_stored_reasons;
-    uint64_t blocklisted_features = 0;
-
-    bool CanStore() const;
-
-    std::string ToString() const;
-
-    void No(BackForwardCacheMetrics::NotRestoredReason reason);
-    void NoDueToFeatures(uint64_t features);
-
-    operator bool() const { return CanStore(); }
-
-   private:
-    std::string NotRestoredReasonToString(
-        BackForwardCacheMetrics::NotRestoredReason reason) const;
-  };
-
   // Returns whether a RenderFrameHost can be stored into the
   // BackForwardCache. Depends on the |render_frame_host| and its children's
   // state.
-  CanStoreDocumentResult CanStoreDocument(
+  BackForwardCacheCanStoreDocumentResult CanStoreDocument(
       RenderFrameHostImpl* render_frame_host);
 
   // Moves the specified BackForwardCache entry into the BackForwardCache. It
@@ -185,7 +157,7 @@ class CONTENT_EXPORT BackForwardCacheImpl : public BackForwardCache {
   void DestroyEvictedFrames();
 
   // Helper for recursively checking each child.
-  void CanStoreRenderFrameHost(CanStoreDocumentResult* result,
+  void CanStoreRenderFrameHost(BackForwardCacheCanStoreDocumentResult* result,
                                RenderFrameHostImpl* render_frame_host);
 
   // Contains the set of stored Entries.

content/browser/frame_host/back_forward_cache_metrics.cc

@@ -179,14 +179,10 @@ void BackForwardCacheMetrics::CollectFeatureUsageFromSubtree(
   }
 }
 
-void BackForwardCacheMetrics::MarkNotRestoredWithReasons(
-    const BackForwardCacheMetrics::NotRestoredReasons& reasons) {
-  not_restored_reasons_ |= reasons;
-}
-
-void BackForwardCacheMetrics::MarkNotRestoredWithBlocklistedFeatures(
-    uint64_t blocklisted_features) {
-  blocklisted_features_ |= blocklisted_features;
+void BackForwardCacheMetrics::MarkNotRestoredWithReason(
+    const BackForwardCacheCanStoreDocumentResult& can_store) {
+  not_restored_reasons_ |= can_store.not_stored_reasons();
+  blocklisted_features_ |= can_store.blocklisted_features();
 }
 
 void BackForwardCacheMetrics::MarkDisableForRenderFrameHost(

content/browser/frame_host/back_forward_cache_metrics.h

@@ -21,6 +21,7 @@ class Origin;
 }
 
 namespace content {
+class BackForwardCacheCanStoreDocumentResult;
 class NavigationEntryImpl;
 class NavigationRequest;
 class RenderFrameHostImpl;
@@ -132,8 +133,8 @@ class BackForwardCacheMetrics
 
   // Marks when the page is not cached, or evicted. This information is useful
   // e.g., to prioritize the tasks to improve cache-hit rate.
-  void MarkNotRestoredWithReasons(const NotRestoredReasons& reasons);
-  void MarkNotRestoredWithBlocklistedFeatures(uint64_t blocklisted_features);
+  void MarkNotRestoredWithReason(
+      const BackForwardCacheCanStoreDocumentResult& can_store);
 
   // Marks the frame disabled the back forward cache with the reason.
   void MarkDisableForRenderFrameHost(const base::StringPiece& reason);

content/browser/frame_host/render_frame_host_impl.cc

@@ -3742,8 +3742,10 @@ void RenderFrameHostImpl::EvictFromBackForwardCacheWithReasons(
   // TODO(hajimehoshi): Record the 'race condition' by JavaScript execution when
   // |is_in_back_forward_cache()| is false.
   BackForwardCacheMetrics* metrics = top_document->GetBackForwardCacheMetrics();
-  if (is_in_back_forward_cache() && metrics)
-    metrics->MarkNotRestoredWithReasons(reasons);
+  if (is_in_back_forward_cache() && metrics) {
+    BackForwardCacheCanStoreDocumentResult can_store(reasons);
+    metrics->MarkNotRestoredWithReason(can_store);
+  }
 
   if (!in_back_forward_cache) {
     BackForwardCacheMetrics::RecordEvictedAfterDocumentRestored(
@@ -7872,7 +7874,7 @@ void RenderFrameHostImpl::MaybeEvictFromBackForwardCache() {
 
   // TODO(hajimehoshi): Pass and record the other information of |can_store|
   // like blocklisted features.
-  EvictFromBackForwardCacheWithReasons(can_store.not_stored_reasons);
+  EvictFromBackForwardCacheWithReasons(can_store.not_stored_reasons());
 }
 
 void RenderFrameHostImpl::LogCannotCommitOriginCrashKeys(

content/browser/frame_host/render_frame_host_manager.cc

@@ -534,16 +534,8 @@ void RenderFrameHostManager::SwapOutOldFrame(
       return;
     }
 
-    if (old_page_back_forward_cache_metrics) {
-      // TODO(hajimehoshi): For code readability, BackForwardCacheMetrics should
-      // take CanStoreDocumentResult directly and rename
-      // MarkNotRestoredWithReason to DidNotStoreDocument.
-      old_page_back_forward_cache_metrics->MarkNotRestoredWithReasons(
-          can_store.not_stored_reasons);
-      old_page_back_forward_cache_metrics
-          ->MarkNotRestoredWithBlocklistedFeatures(
-              can_store.blocklisted_features);
-    }
+    if (old_page_back_forward_cache_metrics)
+      old_page_back_forward_cache_metrics->MarkNotRestoredWithReason(can_store);
   }
 
   // Create a replacement proxy for the old RenderFrameHost. (There should not