Refactor to propagate access type through page load metrics.

This change renames OnDomStorageAccess to OnStorageAccess from
MetricsWebContentObserver->PageLoadMetricsObserver. It adds a new
parameter access_type denoting what storage medium is being accessed.

This change is a precursor change to new metrics measuring third party
storage usage. OnStorageAccess is currently used for LocalStorage and
SessionStorage but will be expanded to cover other types of storage
in future changes.

BUG=1059915

Change-Id: I8fe5e6ed8d934723b1d31b6459bd77c4b8baf700
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2094213Reviewed-by: Balazs Engedy <engedy@chromium.org>
Reviewed-by: John Delaney <johnidel@chromium.org>
Reviewed-by: Yao Xiao <yaoxia@chromium.org>
Commit-Queue: Justin Miron <justinmiron@google.com>
Cr-Commit-Position: refs/heads/master@{#749681}

chrome/browser/content_settings/content_settings_manager_impl.cc

@@ -10,6 +10,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "components/content_settings/core/browser/cookie_settings.h"
 #include "components/page_load_metrics/browser/metrics_web_contents_observer.h"
+#include "components/page_load_metrics/browser/page_load_metrics_observer.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "extensions/buildflags/buildflags.h"
@@ -28,7 +29,8 @@ void OnDomStorageAccessed(int process_id,
                           const GURL& origin_url,
                           const GURL& top_origin_url,
                           bool local,
-                          bool blocked_by_policy) {
+                          bool blocked_by_policy,
+                          page_load_metrics::StorageType storage_type) {
   content::RenderFrameHost* frame =
       content::RenderFrameHost::FromID(process_id, frame_id);
   content::WebContents* web_contents =
@@ -45,8 +47,8 @@ void OnDomStorageAccessed(int process_id,
       page_load_metrics::MetricsWebContentsObserver::FromWebContents(
           web_contents);
   if (metrics_observer)
-    metrics_observer->OnDomStorageAccessed(origin_url, top_origin_url, local,
-                                           blocked_by_policy);
+    metrics_observer->OnStorageAccessed(origin_url, top_origin_url,
+                                        blocked_by_policy, storage_type);
 }
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
@@ -120,11 +122,13 @@ void ContentSettingsManagerImpl::AllowStorageAccess(
       break;
     case StorageType::LOCAL_STORAGE:
       OnDomStorageAccessed(render_process_id_, render_frame_id, url,
-                           top_frame_origin.GetURL(), true, !allowed);
+                           top_frame_origin.GetURL(), true, !allowed,
+                           page_load_metrics::StorageType::kLocalStorage);
       break;
     case StorageType::SESSION_STORAGE:
       OnDomStorageAccessed(render_process_id_, render_frame_id, url,
-                           top_frame_origin.GetURL(), false, !allowed);
+                           top_frame_origin.GetURL(), false, !allowed,
+                           page_load_metrics::StorageType::kSessionStorage);
       break;
     case StorageType::FILE_SYSTEM:
 #if BUILDFLAG(ENABLE_EXTENSIONS)

chrome/browser/page_load_metrics/observers/third_party_metrics_observer.cc

@@ -6,6 +6,7 @@
 
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/strcat.h"
+#include "components/page_load_metrics/browser/page_load_metrics_observer.h"
 #include "components/page_load_metrics/browser/page_load_metrics_util.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
@@ -49,6 +50,9 @@ ThirdPartyMetricsObserver::AccessedTypes::AccessedTypes(
     case AccessType::kSessionStorage:
       session_storage = true;
       break;
+    case AccessType::kUnknown:
+      NOTREACHED();
+      break;
   }
 }
 
@@ -101,14 +105,13 @@ void ThirdPartyMetricsObserver::OnCookieChange(
                           AccessType::kCookieWrite);
 }
 
-void ThirdPartyMetricsObserver::OnDomStorageAccessed(
+void ThirdPartyMetricsObserver::OnStorageAccessed(
     const GURL& url,
     const GURL& first_party_url,
-    bool local,
-    bool blocked_by_policy) {
-  OnCookieOrStorageAccess(
-      url, first_party_url, blocked_by_policy,
-      local ? AccessType::kLocalStorage : AccessType::kSessionStorage);
+    bool blocked_by_policy,
+    page_load_metrics::StorageType storage_type) {
+  OnCookieOrStorageAccess(url, first_party_url, blocked_by_policy,
+                          StorageTypeToAccessType(storage_type));
 }
 
 void ThirdPartyMetricsObserver::OnDidFinishSubFrameNavigation(
@@ -228,6 +231,9 @@ void ThirdPartyMetricsObserver::OnCookieOrStorageAccess(
       case AccessType::kSessionStorage:
         it->second.session_storage = true;
         break;
+      case AccessType::kUnknown:
+        NOTREACHED();
+        break;
     }
     return;
   }
@@ -281,3 +287,17 @@ void ThirdPartyMetricsObserver::RecordMetrics(
         all_frames_largest_contentful_paint.Time().value());
   }
 }
+
+ThirdPartyMetricsObserver::AccessType
+ThirdPartyMetricsObserver::StorageTypeToAccessType(
+    page_load_metrics::StorageType storage_type) {
+  switch (storage_type) {
+    case page_load_metrics::StorageType::kLocalStorage:
+      return AccessType::kLocalStorage;
+    case page_load_metrics::StorageType::kSessionStorage:
+      return AccessType::kSessionStorage;
+    default:
+      NOTREACHED();
+      return AccessType::kUnknown;
+  }
+}

chrome/browser/page_load_metrics/observers/third_party_metrics_observer.h

@@ -36,10 +36,10 @@ class ThirdPartyMetricsObserver
                       const GURL& first_party_url,
                       const net::CanonicalCookie& cookie,
                       bool blocked_by_policy) override;
-  void OnDomStorageAccessed(const GURL& url,
-                            const GURL& first_party_url,
-                            bool local,
-                            bool blocked_by_policy) override;
+  void OnStorageAccessed(const GURL& url,
+                         const GURL& first_party_url,
+                         bool blocked_by_policy,
+                         page_load_metrics::StorageType storage_type) override;
   void OnDidFinishSubFrameNavigation(
       content::NavigationHandle* navigation_handle) override;
   void OnFrameDeleted(content::RenderFrameHost* render_frame_host) override;
@@ -52,7 +52,8 @@ class ThirdPartyMetricsObserver
     kCookieRead,
     kCookieWrite,
     kLocalStorage,
-    kSessionStorage
+    kSessionStorage,
+    kUnknown,
   };
 
   struct AccessedTypes {
@@ -70,6 +71,9 @@ class ThirdPartyMetricsObserver
   void RecordMetrics(
       const page_load_metrics::mojom::PageLoadTiming& main_frame_timing);
 
+  AccessType StorageTypeToAccessType(
+      page_load_metrics::StorageType storage_type);
+
   // A map of third parties that have read or written cookies, or have
   // accessed local storage or session storage on this page.
   //

chrome/browser/page_load_metrics/observers/third_party_metrics_observer_unittest.cc

@@ -426,12 +426,14 @@ TEST_F(ThirdPartyMetricsObserverTest,
        LocalAndSessionStorageAccess_BothRecorded) {
   NavigateAndCommit(GURL("https://top.com"));
 
-  tester()->SimulateDomStorageAccess(GURL("https://a.com"),
-                                     GURL("https://top.com"), true /* local */,
-                                     false /* blocked_by_policy */);
-  tester()->SimulateDomStorageAccess(GURL("https://a.com"),
-                                     GURL("https://top.com"), false /* local */,
-                                     false /* blocked_by_policy */);
+  tester()->SimulateStorageAccess(
+      GURL("https://a.com"), GURL("https://top.com"),
+      false /* blocked_by_policy */,
+      page_load_metrics::StorageType::kLocalStorage /* storage_type */);
+  tester()->SimulateStorageAccess(
+      GURL("https://a.com"), GURL("https://top.com"),
+      false /* blocked_by_policy */,
+      page_load_metrics::StorageType::kSessionStorage /* storage_type */);
   tester()->NavigateToUntrackedUrl();
 
   tester()->histogram_tester().ExpectUniqueSample(kAccessLocalStorageHistogram,
@@ -554,6 +556,11 @@ class ThirdPartyDomStorageAccessMetricsObserverTest
  public:
   bool IsLocal() const { return GetParam(); }
 
+  page_load_metrics::StorageType StorageType() const {
+    return IsLocal() ? page_load_metrics::StorageType::kLocalStorage
+                     : page_load_metrics::StorageType::kSessionStorage;
+  }
+
   const char* DomStorageHistogramName() const {
     return IsLocal() ? kAccessLocalStorageHistogram
                      : kAccessSessionStorageHistogram;
@@ -565,12 +572,12 @@ TEST_P(ThirdPartyDomStorageAccessMetricsObserverTest, Blocked_NotRecorded) {
 
   // If there are any blocked_by_policy access, nothing should be recorded. Even
   // if there are subsequent non-blocked third-party access.
-  tester()->SimulateDomStorageAccess(GURL("https://a.com"),
-                                     GURL("https://top.com"), IsLocal(),
-                                     true /* blocked_by_policy */);
-  tester()->SimulateDomStorageAccess(GURL("https://a.com"),
-                                     GURL("https://top.com"), IsLocal(),
-                                     false /* blocked_by_policy */);
+  tester()->SimulateStorageAccess(GURL("https://a.com"),
+                                  GURL("https://top.com"),
+                                  true /* blocked_by_policy */, StorageType());
+  tester()->SimulateStorageAccess(GURL("https://a.com"),
+                                  GURL("https://top.com"),
+                                  false /* blocked_by_policy */, StorageType());
 
   tester()->NavigateToUntrackedUrl();
 
@@ -581,9 +588,9 @@ TEST_P(ThirdPartyDomStorageAccessMetricsObserverTest,
        NoRegistrableDomainNoHost_NotRecorded) {
   NavigateAndCommit(GURL("https://top.com"));
 
-  tester()->SimulateDomStorageAccess(GURL("data:,Hello%2C%20World!"),
-                                     GURL("https://top.com"), IsLocal(),
-                                     false /* blocked_by_policy */);
+  tester()->SimulateStorageAccess(GURL("data:,Hello%2C%20World!"),
+                                  GURL("https://top.com"),
+                                  false /* blocked_by_policy */, StorageType());
   tester()->NavigateToUntrackedUrl();
 
   tester()->histogram_tester().ExpectUniqueSample(DomStorageHistogramName(), 0,
@@ -594,9 +601,9 @@ TEST_P(ThirdPartyDomStorageAccessMetricsObserverTest,
        NoRegistrableDomainWithHost_OneRecorded) {
   NavigateAndCommit(GURL("https://top.com"));
 
-  tester()->SimulateDomStorageAccess(GURL("https://127.0.0.1"),
-                                     GURL("https://top.com"), IsLocal(),
-                                     false /* blocked_by_policy */);
+  tester()->SimulateStorageAccess(GURL("https://127.0.0.1"),
+                                  GURL("https://top.com"),
+                                  false /* blocked_by_policy */, StorageType());
   tester()->NavigateToUntrackedUrl();
 
   tester()->histogram_tester().ExpectUniqueSample(DomStorageHistogramName(), 1,
@@ -606,9 +613,9 @@ TEST_P(ThirdPartyDomStorageAccessMetricsObserverTest,
 TEST_P(ThirdPartyDomStorageAccessMetricsObserverTest, SameOrigin_NotRecorded) {
   NavigateAndCommit(GURL("https://top.com"));
 
-  tester()->SimulateDomStorageAccess(GURL("https://top.com"),
-                                     GURL("https://top.com"), IsLocal(),
-                                     false /* blocked_by_policy */);
+  tester()->SimulateStorageAccess(GURL("https://top.com"),
+                                  GURL("https://top.com"),
+                                  false /* blocked_by_policy */, StorageType());
   tester()->NavigateToUntrackedUrl();
 
   tester()->histogram_tester().ExpectUniqueSample(DomStorageHistogramName(), 0,
@@ -619,9 +626,9 @@ TEST_P(ThirdPartyDomStorageAccessMetricsObserverTest,
        DifferentOrigin_OneRecorded) {
   NavigateAndCommit(GURL("https://top.com"));
 
-  tester()->SimulateDomStorageAccess(GURL("https://a.com"),
-                                     GURL("https://top.com"), IsLocal(),
-                                     false /* blocked_by_policy */);
+  tester()->SimulateStorageAccess(GURL("https://a.com"),
+                                  GURL("https://top.com"),
+                                  false /* blocked_by_policy */, StorageType());
   tester()->NavigateToUntrackedUrl();
 
   tester()->histogram_tester().ExpectUniqueSample(DomStorageHistogramName(), 1,
@@ -632,9 +639,9 @@ TEST_P(ThirdPartyDomStorageAccessMetricsObserverTest,
        DifferentSchemeSameRegistrableDomain_OneRecorded) {
   NavigateAndCommit(GURL("http://top.com"));
 
-  tester()->SimulateDomStorageAccess(GURL("https://top.com"),
-                                     GURL("http://top.com"), IsLocal(),
-                                     false /* blocked_by_policy */);
+  tester()->SimulateStorageAccess(GURL("https://top.com"),
+                                  GURL("http://top.com"),
+                                  false /* blocked_by_policy */, StorageType());
   tester()->NavigateToUntrackedUrl();
 
   tester()->histogram_tester().ExpectUniqueSample(DomStorageHistogramName(), 1,
@@ -646,12 +653,12 @@ TEST_P(
     TwoAccesses_BothSameSchemeAndRegistrableDomainDifferentOrigin_OneRecorded) {
   NavigateAndCommit(GURL("https://top.com"));
 
-  tester()->SimulateDomStorageAccess(GURL("https://a.com"),
-                                     GURL("https://top.com"), IsLocal(),
-                                     false /* blocked_by_policy */);
-  tester()->SimulateDomStorageAccess(GURL("https://sub.a.com"),
-                                     GURL("https://top.com"), IsLocal(),
-                                     false /* blocked_by_policy */);
+  tester()->SimulateStorageAccess(GURL("https://a.com"),
+                                  GURL("https://top.com"),
+                                  false /* blocked_by_policy */, StorageType());
+  tester()->SimulateStorageAccess(GURL("https://sub.a.com"),
+                                  GURL("https://top.com"),
+                                  false /* blocked_by_policy */, StorageType());
 
   tester()->NavigateToUntrackedUrl();
 
@@ -665,15 +672,15 @@ TEST_P(ThirdPartyDomStorageAccessMetricsObserverTest,
 
   // Simulate third-party DOM storage access from two different
   // origins.
-  tester()->SimulateDomStorageAccess(GURL("https://a.com"),
-                                     GURL("https://top.com"), IsLocal(),
-                                     false /* blocked_by_policy */);
-  tester()->SimulateDomStorageAccess(GURL("https://a.com"),
-                                     GURL("https://top.com"), IsLocal(),
-                                     false /* blocked_by_policy */);
-  tester()->SimulateDomStorageAccess(GURL("https://b.com"),
-                                     GURL("https://top.com"), IsLocal(),
-                                     false /* blocked_by_policy */);
+  tester()->SimulateStorageAccess(GURL("https://a.com"),
+                                  GURL("https://top.com"),
+                                  false /* blocked_by_policy */, StorageType());
+  tester()->SimulateStorageAccess(GURL("https://a.com"),
+                                  GURL("https://top.com"),
+                                  false /* blocked_by_policy */, StorageType());
+  tester()->SimulateStorageAccess(GURL("https://b.com"),
+                                  GURL("https://top.com"),
+                                  false /* blocked_by_policy */, StorageType());
   tester()->NavigateToUntrackedUrl();
 
   tester()->histogram_tester().ExpectUniqueSample(DomStorageHistogramName(), 2,

components/page_load_metrics/browser/metrics_web_contents_observer.cc

@@ -383,14 +383,13 @@ void MetricsWebContentsObserver::OnCookieChange(
                                     blocked_by_policy);
 }
 
-void MetricsWebContentsObserver::OnDomStorageAccessed(
-    const GURL& url,
-    const GURL& first_party_url,
-    bool local,
-    bool blocked_by_policy) {
+void MetricsWebContentsObserver::OnStorageAccessed(const GURL& url,
+                                                   const GURL& first_party_url,
+                                                   bool blocked_by_policy,
+                                                   StorageType storage_type) {
   if (committed_load_)
-    committed_load_->OnDomStorageAccessed(url, first_party_url, local,
-                                          blocked_by_policy);
+    committed_load_->OnStorageAccessed(url, first_party_url, blocked_by_policy,
+                                       storage_type);
 }
 
 const PageLoadMetricsObserverDelegate&

components/page_load_metrics/browser/metrics_web_contents_observer.h

@@ -131,11 +131,10 @@ class MetricsWebContentsObserver
                       const net::CanonicalCookie& cookie,
                       bool blocked_by_policy) override;
 
-  // These methods are forwarded from the ChromeRenderMessageFilter.
-  void OnDomStorageAccessed(const GURL& url,
-                            const GURL& first_party_url,
-                            bool local,
-                            bool blocked_by_policy);
+  void OnStorageAccessed(const GURL& url,
+                         const GURL& first_party_url,
+                         bool blocked_by_policy,
+                         StorageType storage_type);
 
   // These methods are forwarded from the MetricsNavigationThrottle.
   void WillStartNavigationRequest(content::NavigationHandle* navigation_handle);

components/page_load_metrics/browser/observers/page_load_metrics_observer_tester.cc

@@ -286,13 +286,13 @@ void PageLoadMetricsObserverTester::SimulateCookieChange(
                                                  blocked_by_policy);
 }
 
-void PageLoadMetricsObserverTester::SimulateDomStorageAccess(
+void PageLoadMetricsObserverTester::SimulateStorageAccess(
     const GURL& url,
     const GURL& first_party_url,
-    bool local,
-    bool blocked_by_policy) {
-  metrics_web_contents_observer_->OnDomStorageAccessed(
-      url, first_party_url, local, blocked_by_policy);
+    bool blocked_by_policy,
+    StorageType storage_type) {
+  metrics_web_contents_observer_->OnStorageAccessed(
+      url, first_party_url, blocked_by_policy, storage_type);
 }
 
 const PageLoadMetricsObserverDelegate&

components/page_load_metrics/browser/observers/page_load_metrics_observer_tester.h

@@ -141,10 +141,10 @@ class PageLoadMetricsObserverTester : public test::WeakMockTimerProvider {
                             bool blocked_by_policy);
 
   // Simulate accessing the local storage or session storage.
-  void SimulateDomStorageAccess(const GURL& url,
-                                const GURL& first_party_url,
-                                bool local,
-                                bool blocked_by_policy);
+  void SimulateStorageAccess(const GURL& url,
+                             const GURL& first_party_url,
+                             bool blocked_by_policy,
+                             StorageType storage_type);
 
   MetricsWebContentsObserver* metrics_web_contents_observer() {
     return metrics_web_contents_observer_;

components/page_load_metrics/browser/page_load_metrics_observer.h

@@ -29,6 +29,10 @@ class RenderFrameHost;
 
 namespace page_load_metrics {
 
+// Storage types reported to page load metrics observers on storage
+// accesses.
+enum class StorageType { kLocalStorage, kSessionStorage };
+
 // Information related to failed provisional loads.
 struct FailedProvisionalLoadInfo {
   FailedProvisionalLoadInfo(base::TimeDelta interval, net::Error error);
@@ -426,12 +430,13 @@ class PageLoadMetricsObserver {
                               const net::CanonicalCookie& cookie,
                               bool blocked_by_policy) {}
 
-  // Called when a DOM storage is accessed via Window.localStorage or
-  // Window.sessionStorage.
-  virtual void OnDomStorageAccessed(const GURL& url,
-                                    const GURL& first_party_url,
-                                    bool local,
-                                    bool blocked_by_policy) {}
+  // Called when a storage access attempt by the origin |url| to |storage_type|
+  // is checked by the content settings manager. |blocked_by_policy| is false
+  // when cookie access is not allowed for |url|.
+  virtual void OnStorageAccessed(const GURL& url,
+                                 const GURL& first_party_url,
+                                 bool blocked_by_policy,
+                                 StorageType access_type) {}
 
   // Called when the event corresponding to |event_key| occurs in this page
   // load.

components/page_load_metrics/browser/page_load_tracker.cc

@@ -14,6 +14,7 @@
 #include "base/time/default_tick_clock.h"
 #include "base/trace_event/trace_event.h"
 #include "components/page_load_metrics/browser/page_load_metrics_embedder_interface.h"
+#include "components/page_load_metrics/browser/page_load_metrics_observer.h"
 #include "components/page_load_metrics/browser/page_load_metrics_util.h"
 #include "components/page_load_metrics/common/page_load_timing.h"
 #include "content/public/browser/navigation_details.h"
@@ -479,13 +480,13 @@ void PageLoadTracker::OnCookieChange(const GURL& url,
   }
 }
 
-void PageLoadTracker::OnDomStorageAccessed(const GURL& url,
-                                           const GURL& first_party_url,
-                                           bool local,
-                                           bool blocked_by_policy) {
+void PageLoadTracker::OnStorageAccessed(const GURL& url,
+                                        const GURL& first_party_url,
+                                        bool blocked_by_policy,
+                                        StorageType access_type) {
   for (const auto& observer : observers_) {
-    observer->OnDomStorageAccessed(url, first_party_url, local,
-                                   blocked_by_policy);
+    observer->OnStorageAccessed(url, first_party_url, blocked_by_policy,
+                                access_type);
   }
 }
 

components/page_load_metrics/browser/page_load_tracker.h

@@ -277,10 +277,10 @@ class PageLoadTracker : public PageLoadMetricsUpdateDispatcher::Client,
                       const net::CanonicalCookie& cookie,
                       bool blocked_by_policy);
 
-  void OnDomStorageAccessed(const GURL& url,
-                            const GURL& first_party_url,
-                            bool local,
-                            bool blocked_by_policy);
+  void OnStorageAccessed(const GURL& url,
+                         const GURL& first_party_url,
+                         bool blocked_by_policy,
+                         StorageType access_type);
 
   // Signals that we should stop tracking metrics for the associated page load.
   // We may stop tracking a page load if it doesn't meet the criteria for