Count blocked resources at most once

Fix brave/brave-browser#1311
brave · Nov 20, 2018 · 2564f30 · 2564f30
1 parent 838d82d
commit 2564f30
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 23 deletions.
diff --git a/components/brave_shields/browser/brave_shields_web_contents_observer.cc b/components/brave_shields/browser/brave_shields_web_contents_observer.cc
@@ -182,6 +182,17 @@ GURL BraveShieldsWebContentsObserver::GetTabURLFromRenderFrameInfo(
   return GURL();
 }
 
+bool BraveShieldsWebContentsObserver::IsBlockedSubresource(
+    const std::string& subresource) {
+  return blocked_url_paths_.find(subresource) != blocked_url_paths_.end();
+}
+
+void BraveShieldsWebContentsObserver::AddBlockedSubresource(
+    const std::string& subresource) {
+  blocked_url_paths_.insert(subresource);
+}
+
+// static
 void BraveShieldsWebContentsObserver::DispatchBlockedEvent(
     std::string block_type,
     std::string subresource,
@@ -195,28 +206,35 @@ void BraveShieldsWebContentsObserver::DispatchBlockedEvent(
   DispatchBlockedEventForWebContents(block_type, subresource, web_contents);
 
   if (web_contents) {
-    PrefService* prefs = Profile::FromBrowserContext(
-        web_contents->GetBrowserContext())->
-        GetOriginalProfile()->
-        GetPrefs();
-
-    if (block_type == kAds) {
-      prefs->SetUint64(kAdsBlocked, prefs->GetUint64(kAdsBlocked) + 1);
-    } else if (block_type == kTrackers) {
-      prefs->SetUint64(kTrackersBlocked,
-          prefs->GetUint64(kTrackersBlocked) + 1);
-    } else if (block_type == kHTTPUpgradableResources) {
-      prefs->SetUint64(kHttpsUpgrades, prefs->GetUint64(kHttpsUpgrades) + 1);
-    } else if (block_type == kJavaScript) {
-      prefs->SetUint64(kJavascriptBlocked,
-          prefs->GetUint64(kJavascriptBlocked) + 1);
-    } else if (block_type == kFingerprinting) {
-      prefs->SetUint64(kFingerprintingBlocked,
-          prefs->GetUint64(kFingerprintingBlocked) + 1);
+    BraveShieldsWebContentsObserver* observer =
+        BraveShieldsWebContentsObserver::FromWebContents(web_contents);
+    if (observer &&
+        !observer->IsBlockedSubresource(subresource)) {
+      observer->AddBlockedSubresource(subresource);
+      PrefService* prefs = Profile::FromBrowserContext(
+          web_contents->GetBrowserContext())->
+          GetOriginalProfile()->
+          GetPrefs();
+
+      if (block_type == kAds) {
+        prefs->SetUint64(kAdsBlocked, prefs->GetUint64(kAdsBlocked) + 1);
+      } else if (block_type == kTrackers) {
+        prefs->SetUint64(kTrackersBlocked,
+            prefs->GetUint64(kTrackersBlocked) + 1);
+      } else if (block_type == kHTTPUpgradableResources) {
+        prefs->SetUint64(kHttpsUpgrades, prefs->GetUint64(kHttpsUpgrades) + 1);
+      } else if (block_type == kJavaScript) {
+        prefs->SetUint64(kJavascriptBlocked,
+            prefs->GetUint64(kJavascriptBlocked) + 1);
+      } else if (block_type == kFingerprinting) {
+        prefs->SetUint64(kFingerprintingBlocked,
+            prefs->GetUint64(kFingerprintingBlocked) + 1);
+      }
     }
   }
 }
 
+// static
 void BraveShieldsWebContentsObserver::DispatchBlockedEventForWebContents(
     const std::string& block_type, const std::string& subresource,
     WebContents* web_contents) {
@@ -295,9 +313,11 @@ void BraveShieldsWebContentsObserver::ReadyToCommitNavigation(
     content::NavigationHandle* navigation_handle) {
   // when the main frame navigate away
   if (navigation_handle->IsInMainFrame() &&
-      !navigation_handle->IsSameDocument() &&
-      navigation_handle->GetReloadType() == content::ReloadType::NONE) {
+      !navigation_handle->IsSameDocument()) {
     allowed_script_origins_.clear();
+    if (navigation_handle->GetReloadType() == content::ReloadType::NONE) {
+      blocked_url_paths_.clear();
+    }
   }
 
   navigation_handle->GetWebContents()->SendToAllFrames(

diff --git a/components/brave_shields/browser/brave_shields_web_contents_observer.h b/components/brave_shields/browser/brave_shields_web_contents_observer.h
@@ -38,6 +38,8 @@ class BraveShieldsWebContentsObserver : public content::WebContentsObserver,
   static GURL GetTabURLFromRenderFrameInfo(int render_process_id, int render_frame_id);
   void AllowScriptsOnce(const std::vector<std::string>& origins,
                         content::WebContents* web_contents);
+  bool IsBlockedSubresource(const std::string& subresource);
+  void AddBlockedSubresource(const std::string& subresource);
 
  protected:
     // A set of identifiers that uniquely identifies a RenderFrame.
@@ -78,9 +80,12 @@ class BraveShieldsWebContentsObserver : public content::WebContentsObserver,
   // UI thread and read on the IO thread.
   static base::Lock frame_data_map_lock_;
 
-  private:
-    friend class content::WebContentsUserData<BraveShieldsWebContentsObserver>;
-    std::vector<std::string> allowed_script_origins_;
+ private:
+  friend class content::WebContentsUserData<BraveShieldsWebContentsObserver>;
+  std::vector<std::string> allowed_script_origins_;
+  // We keep a set of the current page's blocked URLs in case the page
+  // continually tries to load the same blocked URLs.
+  std::set<std::string> blocked_url_paths_;
 
   DISALLOW_COPY_AND_ASSIGN(BraveShieldsWebContentsObserver);
 };