Cleanup RenderFrameHost interaction during ShieldsSettings passing.

brave · Sep 13, 2024 · 31f115f · 31f115f
1 parent dce67ae
commit 31f115f
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 105 deletions.
diff --git a/browser/brave_content_browser_client.cc b/browser/brave_content_browser_client.cc
@@ -688,15 +688,17 @@ BraveContentBrowserClient::WorkerGetBraveShieldSettings(
     auto fingerprinting_type = brave_shields::GetFingerprintingControlType(
         host_content_settings_map, url);
     switch (fingerprinting_type) {
-      case ControlType::BLOCK:
-        farbling_level = brave_shields::mojom::FarblingLevel::MAXIMUM;
-        break;
       case ControlType::ALLOW:
         farbling_level = brave_shields::mojom::FarblingLevel::OFF;
         break;
+      case ControlType::BLOCK:
+        farbling_level = brave_shields::mojom::FarblingLevel::MAXIMUM;
+        break;
       case ControlType::BLOCK_THIRD_PARTY:
-      case ControlType::DEFAULT:
         NOTREACHED();
+      case ControlType::DEFAULT:
+        farbling_level = brave_shields::mojom::FarblingLevel::BALANCED;
+        break;
     }
   } else {
     farbling_level = brave_shields::mojom::FarblingLevel::OFF;

diff --git a/browser/brave_shields/brave_shields_web_contents_observer.cc b/browser/brave_shields/brave_shields_web_contents_observer.cc
@@ -49,9 +49,7 @@ BraveShieldsWebContentsObserver* g_receiver_impl_for_testing = nullptr;
 
 }  // namespace
 
-BraveShieldsWebContentsObserver::~BraveShieldsWebContentsObserver() {
-  brave_shields_remotes_.clear();
-}
+BraveShieldsWebContentsObserver::~BraveShieldsWebContentsObserver() = default;
 
 BraveShieldsWebContentsObserver::BraveShieldsWebContentsObserver(
     WebContents* web_contents)
@@ -60,72 +58,6 @@ BraveShieldsWebContentsObserver::BraveShieldsWebContentsObserver(
           *web_contents),
       receivers_(web_contents, this) {}
 
-void BraveShieldsWebContentsObserver::RenderFrameCreated(RenderFrameHost* rfh) {
-  SendShieldsSettingsToFrame(rfh, nullptr);
-}
-
-void BraveShieldsWebContentsObserver::SendShieldsSettingsToFrame(
-    RenderFrameHost* rfh,
-    NavigationHandle* navigation_handle) {
-  DCHECK(rfh);
-  DCHECK(!navigation_handle || rfh == navigation_handle->GetRenderFrameHost());
-
-  const GURL& primary_url =
-      navigation_handle
-          ? (navigation_handle->GetParentFrameOrOuterDocument()
-                 ? navigation_handle->GetParentFrameOrOuterDocument()
-                       ->GetOutermostMainFrame()
-                       ->GetLastCommittedURL()
-                 : navigation_handle->GetURL())
-          : rfh->GetLastCommittedURL();
-
-  brave_shields::mojom::FarblingLevel farbling_level =
-      brave_shields::mojom::FarblingLevel::BALANCED;
-  HostContentSettingsMap* host_content_settings_map =
-      HostContentSettingsMapFactory::GetForProfile(rfh->GetBrowserContext());
-  const bool shields_up = brave_shields::GetBraveShieldsEnabled(
-      host_content_settings_map, primary_url);
-  if (shields_up) {
-    auto fingerprinting_type = brave_shields::GetFingerprintingControlType(
-        host_content_settings_map, primary_url);
-    switch (fingerprinting_type) {
-      case ControlType::BLOCK:
-        farbling_level = brave_shields::mojom::FarblingLevel::MAXIMUM;
-        break;
-      case ControlType::ALLOW:
-        farbling_level = brave_shields::mojom::FarblingLevel::OFF;
-        break;
-      case ControlType::BLOCK_THIRD_PARTY:
-      case ControlType::DEFAULT:
-        NOTREACHED();
-    }
-  } else {
-    farbling_level = brave_shields::mojom::FarblingLevel::OFF;
-  }
-  PrefService* pref_service =
-      user_prefs::UserPrefs::Get(rfh->GetBrowserContext());
-
-  GetBraveShieldsRemote(rfh)->SetShieldsSettings(
-      brave_shields::mojom::ShieldsSettings::New(
-          farbling_level, allowed_scripts_,
-          brave_shields::IsReduceLanguageEnabledForProfile(pref_service)));
-}
-
-void BraveShieldsWebContentsObserver::RenderFrameDeleted(RenderFrameHost* rfh) {
-  brave_shields_remotes_.erase(rfh);
-}
-
-void BraveShieldsWebContentsObserver::RenderFrameHostChanged(
-    RenderFrameHost* old_host,
-    RenderFrameHost* new_host) {
-  if (old_host) {
-    RenderFrameDeleted(old_host);
-  }
-  if (new_host) {
-    RenderFrameCreated(new_host);
-  }
-}
-
 bool BraveShieldsWebContentsObserver::IsBlockedSubresource(
     const std::string& subresource) {
   return blocked_url_paths_.find(subresource) != blocked_url_paths_.end();
@@ -301,10 +233,14 @@ void BraveShieldsWebContentsObserver::RegisterProfilePrefs(
 
 void BraveShieldsWebContentsObserver::ReadyToCommitNavigation(
     NavigationHandle* navigation_handle) {
-  // when the main frame navigate away
+  // Ignore same document navigations.
+  if (navigation_handle->IsSameDocument()) {
+    return;
+  }
+
+  // When the main frame navigates away.
   content::ReloadType reload_type = navigation_handle->GetReloadType();
-  if (navigation_handle->IsInMainFrame() &&
-      !navigation_handle->IsSameDocument()) {
+  if (navigation_handle->IsInMainFrame()) {
     if (reload_type == content::ReloadType::NONE) {
       // For new loads, we reset the counters for both blocked scripts and URLs.
       allowed_scripts_.clear();
@@ -317,8 +253,7 @@ void BraveShieldsWebContentsObserver::ReadyToCommitNavigation(
     }
   }
 
-  RenderFrameHost* rfh = navigation_handle->GetRenderFrameHost();
-  SendShieldsSettingsToFrame(rfh, navigation_handle);
+  SendShieldsSettings(navigation_handle);
 }
 
 void BraveShieldsWebContentsObserver::BlockAllowedScripts(
@@ -347,25 +282,60 @@ void BraveShieldsWebContentsObserver::SetReceiverImplForTesting(
   g_receiver_impl_for_testing = impl;
 }
 
+void BraveShieldsWebContentsObserver::SendShieldsSettings(
+    NavigationHandle* navigation_handle) {
+  DCHECK(navigation_handle);
+  RenderFrameHost* rfh = navigation_handle->GetRenderFrameHost();
+
+  const GURL& primary_url =
+      navigation_handle->GetParentFrameOrOuterDocument()
+          ? navigation_handle->GetParentFrameOrOuterDocument()
+                ->GetOutermostMainFrame()
+                ->GetLastCommittedURL()
+          : navigation_handle->GetURL();
+
+  brave_shields::mojom::FarblingLevel farbling_level =
+      brave_shields::mojom::FarblingLevel::BALANCED;
+  HostContentSettingsMap* host_content_settings_map =
+      HostContentSettingsMapFactory::GetForProfile(rfh->GetBrowserContext());
+  const bool shields_up = brave_shields::GetBraveShieldsEnabled(
+      host_content_settings_map, primary_url);
+  if (shields_up) {
+    auto fingerprinting_type = brave_shields::GetFingerprintingControlType(
+        host_content_settings_map, primary_url);
+    switch (fingerprinting_type) {
+      case ControlType::ALLOW:
+        farbling_level = brave_shields::mojom::FarblingLevel::OFF;
+        break;
+      case ControlType::BLOCK:
+        farbling_level = brave_shields::mojom::FarblingLevel::MAXIMUM;
+        break;
+      case ControlType::BLOCK_THIRD_PARTY:
+        NOTREACHED();
+      case ControlType::DEFAULT:
+        farbling_level = brave_shields::mojom::FarblingLevel::BALANCED;
+        break;
+    }
+  } else {
+    farbling_level = brave_shields::mojom::FarblingLevel::OFF;
+  }
+  PrefService* pref_service =
+      user_prefs::UserPrefs::Get(rfh->GetBrowserContext());
+
+  mojo::AssociatedRemote<brave_shields::mojom::BraveShields> agent;
+  rfh->GetRemoteAssociatedInterfaces()->GetInterface(&agent);
+  agent->SetShieldsSettings(brave_shields::mojom::ShieldsSettings::New(
+      farbling_level, allowed_scripts_,
+      brave_shields::IsReduceLanguageEnabledForProfile(pref_service)));
+}
+
 void BraveShieldsWebContentsObserver::BindReceiver(
     mojo::PendingAssociatedReceiver<brave_shields::mojom::BraveShieldsHost>
         receiver,
     content::RenderFrameHost* rfh) {
   receivers_.Bind(rfh, std::move(receiver));
 }
 
-mojo::AssociatedRemote<brave_shields::mojom::BraveShields>&
-BraveShieldsWebContentsObserver::GetBraveShieldsRemote(
-    content::RenderFrameHost* rfh) {
-  if (!brave_shields_remotes_.contains(rfh)) {
-    rfh->GetRemoteAssociatedInterfaces()->GetInterface(
-        &brave_shields_remotes_[rfh]);
-  }
-
-  DCHECK(brave_shields_remotes_[rfh].is_bound());
-  return brave_shields_remotes_[rfh];
-}
-
 WEB_CONTENTS_USER_DATA_KEY_IMPL(BraveShieldsWebContentsObserver);
 
 }  // namespace brave_shields
diff --git a/browser/brave_shields/brave_shields_web_contents_observer.h b/browser/brave_shields/brave_shields_web_contents_observer.h
@@ -67,10 +67,6 @@ class BraveShieldsWebContentsObserver
 
  protected:
   // content::WebContentsObserver overrides.
-  void RenderFrameCreated(content::RenderFrameHost* host) override;
-  void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) override;
-  void RenderFrameHostChanged(content::RenderFrameHost* old_host,
-                              content::RenderFrameHost* new_host) override;
   void ReadyToCommitNavigation(
       content::NavigationHandle* navigation_handle) override;
 
@@ -92,20 +88,16 @@ class BraveShieldsWebContentsObserver
   // other than this own class, for testing purposes only.
   static void SetReceiverImplForTesting(BraveShieldsWebContentsObserver* impl);
 
-  void SendShieldsSettingsToFrame(content::RenderFrameHost* rfh,
-                                  content::NavigationHandle* navigation_handle);
-
   // Only used from the BindBraveShieldsHost() static method, useful to bind
   // the mojo receiver of brave_shields::mojom::BraveShieldsHost to a
   // different implementor when needed, for testing purposes.
   void BindReceiver(mojo::PendingAssociatedReceiver<
                         brave_shields::mojom::BraveShieldsHost> receiver,
                     content::RenderFrameHost* rfh);
 
-  // Return an already bound remote for the brave_shields::mojom::BraveShields
-  // mojo interface. It is an error to call this method with an invalid |rfh|.
-  mojo::AssociatedRemote<brave_shields::mojom::BraveShields>&
-  GetBraveShieldsRemote(content::RenderFrameHost* rfh);
+  // Sends the current shields settings to the renderer process bound to the
+  // given |navigation_handle|.
+  void SendShieldsSettings(content::NavigationHandle* navigation_handle);
 
   std::vector<std::string> allowed_scripts_;
   // We keep a set of the current page's blocked URLs in case the page
@@ -115,10 +107,6 @@ class BraveShieldsWebContentsObserver
   content::RenderFrameHostReceiverSet<brave_shields::mojom::BraveShieldsHost>
       receivers_;
 
-  // Map of remote endpoints for the brave_shields::mojom::BraveShields mojo
-  // interface, to prevent binding a new remote each time it's used.
-  BraveShieldsRemotesMap brave_shields_remotes_;
-
   WEB_CONTENTS_USER_DATA_KEY_DECL();
 };