Merge pull request #7836 from brave/pr7811_fix_renderer_crash_from_ad…

…block_page_1.21.x

Fix renderer crash from adblock page(removed stats from adblock webui) (uplift to 1.21.x)

browser/ui/webui/basic_ui.cc

@@ -7,11 +7,10 @@
 
 #include "brave/browser/ui/webui/brave_webui_source.h"
 #include "chrome/browser/profiles/profile.h"
-#include "content/public/browser/web_contents.h"
-#include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_ui.h"
 #include "content/public/browser/web_ui_data_source.h"
 #include "content/public/common/bindings_policy.h"
+#include "services/network/public/mojom/content_security_policy.mojom.h"
 #include "ui/resources/grit/webui_resources_map.h"
 
 content::WebUIDataSource* CreateBasicUIHTMLSource(
@@ -50,65 +49,18 @@ content::WebUIDataSource* CreateBasicUIHTMLSource(
   return source;
 }
 
-// This is used to know proper timing for setting webui properties.
-// So far, content::WebUIController::RenderFrameCreated() is used.
-// However, it doesn't get called sometimes when reloading, or called when
-// RenderFrameHost is not prepared during renderer process is in changing.
-class BasicUI::BasicUIWebContentsObserver
-    : public content::WebContentsObserver {
- public:
-  BasicUIWebContentsObserver(BasicUI* host, content::WebContents* web_contents)
-      : WebContentsObserver(web_contents),
-        host_(host) {
-  }
-  ~BasicUIWebContentsObserver() override {}
-
-  // content::WebContentsObserver overrides:
-  void RenderViewReady() override {
-    host_->UpdateWebUIProperties();
-  }
-
- private:
-  BasicUI* host_;
-
-  DISALLOW_COPY_AND_ASSIGN(BasicUIWebContentsObserver);
-};
-
 BasicUI::BasicUI(content::WebUI* web_ui,
                  const std::string& name,
                  const GritResourceMap* resource_map,
                  size_t resource_map_size,
                  int html_resource_id,
                  bool disable_trusted_types_csp)
     : WebUIController(web_ui) {
-  observer_.reset(
-      new BasicUIWebContentsObserver(this, web_ui->GetWebContents()));
   Profile* profile = Profile::FromWebUI(web_ui);
   content::WebUIDataSource* source = CreateBasicUIHTMLSource(profile, name,
       resource_map, resource_map_size, html_resource_id,
       disable_trusted_types_csp);
   content::WebUIDataSource::Add(profile, source);
 }
 
-BasicUI::~BasicUI() {
-}
-
-content::RenderFrameHost* BasicUI::GetRenderFrameHost() {
-  auto* web_contents = web_ui()->GetWebContents();
-  if (web_contents) {
-    return web_contents->GetMainFrame();
-  }
-  return nullptr;
-}
-
-bool BasicUI::IsSafeToSetWebUIProperties() const {
-  // Allow `web_ui()->CanCallJavascript()` to be false.
-  // Allow `web_ui()->CanCallJavascript()` to be true if
-  // `(web_ui()->GetBindings() & content::BINDINGS_POLICY_WEB_UI) != 0`
-  // Disallow `web_ui()->CanCallJavascript()` to be true if
-  // `(web_ui()->GetBindings() & content::BINDINGS_POLICY_WEB_UI) == 0`
-  DCHECK(!web_ui()->CanCallJavascript() ||
-         (web_ui()->GetBindings() & content::BINDINGS_POLICY_WEB_UI));
-  return web_ui()->CanCallJavascript() &&
-         (web_ui()->GetBindings() & content::BINDINGS_POLICY_WEB_UI);
-}
+BasicUI::~BasicUI() = default;

browser/ui/webui/basic_ui.h

@@ -6,19 +6,16 @@
 #ifndef BRAVE_BROWSER_UI_WEBUI_BASIC_UI_H_
 #define BRAVE_BROWSER_UI_WEBUI_BASIC_UI_H_
 
-#include <memory>
-
 #include <string>
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "content/public/browser/web_ui_controller.h"
 
 namespace content {
-class RenderFrameHost;
 class WebUIDataSource;
 class WebUI;
-}
+}  // namespace content
 
 class Profile;
 
@@ -42,18 +39,7 @@ class BasicUI : public content::WebUIController {
           bool disable_trusted_types_csp = false);
   ~BasicUI() override;
 
-  // Called when subclass can set its webui properties.
-  virtual void UpdateWebUIProperties() {}
-
- protected:
-  bool IsSafeToSetWebUIProperties() const;
-  content::RenderFrameHost* GetRenderFrameHost();
-
  private:
-  class BasicUIWebContentsObserver;
-
-  std::unique_ptr<BasicUIWebContentsObserver> observer_;
-
   DISALLOW_COPY_AND_ASSIGN(BasicUI);
 };
 

browser/ui/webui/brave_adblock_ui.cc

@@ -5,19 +5,14 @@
 
 #include "brave/browser/ui/webui/brave_adblock_ui.h"
 
+#include <memory>
+
 #include "brave/browser/brave_browser_process_impl.h"
-#include "brave/common/pref_names.h"
 #include "brave/common/webui_url_constants.h"
 #include "brave/components/brave_adblock/resources/grit/brave_adblock_generated_map.h"
 #include "brave/components/brave_shields/browser/ad_block_custom_filters_service.h"
 #include "brave/components/brave_shields/browser/ad_block_regional_service_manager.h"
-#include "chrome/browser/profiles/profile.h"
 #include "components/grit/brave_components_resources.h"
-#include "components/prefs/pref_change_registrar.h"
-#include "components/prefs/pref_service.h"
-#include "content/public/browser/render_frame_host.h"
-#include "content/public/browser/render_view_host.h"
-#include "content/public/browser/web_ui_data_source.h"
 #include "content/public/browser/web_ui_message_handler.h"
 
 namespace {
@@ -111,37 +106,7 @@ void AdblockDOMHandler::HandleUpdateCustomFilters(const base::ListValue* args) {
 BraveAdblockUI::BraveAdblockUI(content::WebUI* web_ui, const std::string& name)
     : BasicUI(web_ui, name, kBraveAdblockGenerated,
         kBraveAdblockGeneratedSize, IDR_BRAVE_ADBLOCK_HTML) {
-  Profile* profile = Profile::FromWebUI(web_ui);
-  PrefService* prefs = profile->GetPrefs();
-  pref_change_registrar_ = std::make_unique<PrefChangeRegistrar>();
-  pref_change_registrar_->Init(prefs);
-  pref_change_registrar_->Add(kAdsBlocked,
-    base::Bind(&BraveAdblockUI::OnPreferenceChanged, base::Unretained(this)));
   web_ui->AddMessageHandler(std::make_unique<AdblockDOMHandler>());
 }
 
-BraveAdblockUI::~BraveAdblockUI() {
-}
-
-void BraveAdblockUI::CustomizeWebUIProperties(
-    content::RenderFrameHost* render_frame_host) {
-  DCHECK(IsSafeToSetWebUIProperties());
-  Profile* profile = Profile::FromWebUI(web_ui());
-  PrefService* prefs = profile->GetPrefs();
-  if (render_frame_host) {
-    render_frame_host->SetWebUIProperty(
-        "adsBlockedStat", std::to_string(prefs->GetUint64(kAdsBlocked) +
-            prefs->GetUint64(kTrackersBlocked)));
-  }
-}
-
-void BraveAdblockUI::UpdateWebUIProperties() {
-  if (IsSafeToSetWebUIProperties()) {
-    CustomizeWebUIProperties(GetRenderFrameHost());
-    web_ui()->CallJavascriptFunctionUnsafe("brave_adblock.statsUpdated");
-  }
-}
-
-void BraveAdblockUI::OnPreferenceChanged() {
-  UpdateWebUIProperties();
-}
+BraveAdblockUI::~BraveAdblockUI() = default;

browser/ui/webui/brave_adblock_ui.h

@@ -6,27 +6,16 @@
 #ifndef BRAVE_BROWSER_UI_WEBUI_BRAVE_ADBLOCK_UI_H_
 #define BRAVE_BROWSER_UI_WEBUI_BRAVE_ADBLOCK_UI_H_
 
-#include <memory>
 #include <string>
 
 #include "brave/browser/ui/webui/basic_ui.h"
 
-class PrefChangeRegistrar;
-
 class BraveAdblockUI : public BasicUI {
  public:
   BraveAdblockUI(content::WebUI* web_ui, const std::string& host);
   ~BraveAdblockUI() override;
 
  private:
-  // BasicUI overrides:
-  void UpdateWebUIProperties() override;
-
-  void CustomizeWebUIProperties(content::RenderFrameHost* render_frame_host);
-  void OnPreferenceChanged();
-
-  std::unique_ptr<PrefChangeRegistrar> pref_change_registrar_;
-
   DISALLOW_COPY_AND_ASSIGN(BraveAdblockUI);
 };
 

components/brave_adblock_ui/actions/adblock_actions.ts

@@ -27,8 +27,6 @@ export const onGetRegionalLists = (regionalLists: AdBlock.FilterList[]) =>
     regionalLists
   })
 
-export const statsUpdated = () => action(types.ADBLOCK_STATS_UPDATED)
-
 export const updateCustomFilters = (customFilters: string) =>
   action(types.ADBLOCK_UPDATE_CUSTOM_FILTERS, {
     customFilters

components/brave_adblock_ui/brave_adblock.tsx

@@ -48,16 +48,10 @@ window.cr.define('brave_adblock', function () {
     actions.onGetRegionalLists(regionalLists)
   }
 
-  function statsUpdated () {
-    const actions = bindActionCreators(adblockActions, store.dispatch.bind(store))
-    actions.statsUpdated()
-  }
-
   return {
     initialize,
     onGetCustomFilters,
-    onGetRegionalLists,
-    statsUpdated
+    onGetRegionalLists
   }
 })
 

components/brave_adblock_ui/components/app.tsx

@@ -9,7 +9,6 @@ import { connect } from 'react-redux'
 // Components
 import { AdBlockItemList } from './adBlockItemList'
 import { CustomFilters } from './customFilters'
-import { NumBlockedStat } from './numBlockedStat'
 
 // Utils
 import * as adblockActions from '../actions/adblock_actions'
@@ -32,7 +31,6 @@ export class AdblockPage extends React.Component<Props, {}> {
     const { actions, adblockData } = this.props
     return (
       <div id='adblockPage'>
-        <NumBlockedStat adsBlockedStat={adblockData.stats.adsBlockedStat || 0} />
         <AdBlockItemList
           actions={actions}
           resources={adblockData.settings.regionalLists}

components/brave_adblock_ui/constants/adblock_types.ts

@@ -8,6 +8,5 @@ export const enum types {
   ADBLOCK_GET_REGIONAL_LISTS = '@@adblock/ADBLOCK_GET_REGIONAL_LISTS',
   ADBLOCK_ON_GET_CUSTOM_FILTERS = '@@adblock/ADBLOCK_ON_GET_CUSTOM_FILTERS',
   ADBLOCK_ON_GET_REGIONAL_LISTS = '@@adblock/ADBLOCK_ON_GET_REGIONAL_LISTS',
-  ADBLOCK_STATS_UPDATED = '@@adblock/ADBLOCK_STATS_UPDATED',
   ADBLOCK_UPDATE_CUSTOM_FILTERS = '@@adblock/ADBLOCK_UPDATE_CUSTOM_FILTERS'
 }

components/brave_adblock_ui/reducers/adblock_reducer.ts

@@ -46,9 +46,6 @@ const adblockReducer: Reducer<AdBlock.State | undefined> = (state: AdBlock.State
     case types.ADBLOCK_ON_GET_REGIONAL_LISTS:
       state = { ...state, settings: { ...state.settings, regionalLists: action.payload.regionalLists } }
       break
-    case types.ADBLOCK_STATS_UPDATED:
-      state = storage.getLoadTimeData(state)
-      break
     case types.ADBLOCK_UPDATE_CUSTOM_FILTERS:
       state = { ...state, settings: { ...state.settings, customFilters: action.payload.customFilters } }
       updateCustomFilters(state.settings.customFilters)

components/brave_adblock_ui/storage.ts

@@ -10,28 +10,9 @@ export const defaultState: AdBlock.State = {
   settings: {
     customFilters: '',
     regionalLists: []
-  },
-  stats: {
-    numBlocked: 0
   }
 }
 
-export const getLoadTimeData = (state: AdBlock.State): AdBlock.State => {
-  state = { ...state }
-  state.stats = defaultState.stats
-
-  // Expected to be numbers
-  ;['adsBlockedStat'].forEach((stat) => {
-    state.stats[stat] = parseInt(chrome.getVariableValue(stat), 10)
-  })
-
-  return state
-}
-
-export const cleanData = (state: AdBlock.State): AdBlock.State => {
-  return getLoadTimeData(state)
-}
-
 export const load = (): AdBlock.State => {
   const data = window.localStorage.getItem(keyName)
   let state: AdBlock.State = defaultState
@@ -42,11 +23,11 @@ export const load = (): AdBlock.State => {
       console.error('Could not parse local storage data: ', e)
     }
   }
-  return cleanData(state)
+  return state
 }
 
 export const debouncedSave = debounce((data: AdBlock.State) => {
   if (data) {
-    window.localStorage.setItem(keyName, JSON.stringify(cleanData(data)))
+    window.localStorage.setItem(keyName, JSON.stringify(data))
   }
 }, 50)

components/definitions/adBlock.d.ts

@@ -7,10 +7,6 @@ declare namespace AdBlock {
     settings: {
       customFilters: string
       regionalLists: FilterList[]
-    },
-    stats: {
-      adsBlockedStat?: number
-      numBlocked: number
     }
   }
 

components/test/brave_adblock_ui/components/app_test.tsx

@@ -1,11 +1,9 @@
 import * as React from 'react'
 import { shallow } from 'enzyme'
-import { types } from '../../../brave_adblock_ui/constants/adblock_types'
 import { adblockInitialState } from '../../testData'
 import {
   AdblockPage,
-  mapStateToProps,
-  mapDispatchToProps
+  mapStateToProps
 } from '../../../brave_adblock_ui/components/app'
 
 describe('adblockPage component', () => {
@@ -15,21 +13,6 @@ describe('adblockPage component', () => {
     })
   })
 
-  describe('mapDispatchToProps', () => {
-    it('should fire statsUpdated', () => {
-      const dispatch = jest.fn()
-
-      // For the `mapDispatchToProps`, call it directly but pass in
-      // a mock function and check the arguments passed in are as expected
-      mapDispatchToProps(dispatch).actions.statsUpdated()
-      expect(dispatch.mock.calls[0][0]).toEqual({
-        type: types.ADBLOCK_STATS_UPDATED,
-        meta: undefined,
-        payload: undefined
-      })
-    })
-  })
-
   describe('adblockPage dumb component', () => {
     it('renders the component', () => {
       const wrapper = shallow(