Crash fix for back button around newtab

browser/ui/webui/basic_ui.cc

@@ -6,6 +6,7 @@
 
 #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_ui_data_source.h"
 #include "content/public/browser/web_ui_message_handler.h"
 
@@ -69,3 +70,11 @@ BasicUI::BasicUI(content::WebUI* web_ui,
 
 BasicUI::~BasicUI() {
 }
+
+content::RenderViewHost* BasicUI::GetRenderViewHost() {
+  auto* web_contents = web_ui()->GetWebContents();
+  if (web_contents) {
+    return web_contents->GetRenderViewHost();
+  }
+  return nullptr;
+}

browser/ui/webui/basic_ui.h

@@ -12,6 +12,7 @@
 #include "content/public/browser/web_ui_controller.h"
 
 namespace content {
+class RenderViewHost;
 class WebUIDataSource;
 class WebUI;
 }
@@ -22,6 +23,9 @@ class BasicUI : public content::WebUIController {
       const std::string& js_file, int js_resource_id, int html_resource_id);
   ~BasicUI() override;
 
+ protected:
+  content::RenderViewHost* GetRenderViewHost();
+
  private:
   DISALLOW_COPY_AND_ASSIGN(BasicUI);
 };

browser/ui/webui/brave_adblock_ui.cc

@@ -14,7 +14,6 @@
 #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_contents.h"
 #include "content/public/browser/web_ui_data_source.h"
 #include "content/public/browser/web_ui_message_handler.h"
 #include "content/public/common/bindings_policy.h"
@@ -33,32 +32,28 @@ BraveAdblockUI::BraveAdblockUI(content::WebUI* web_ui, const std::string& name)
 BraveAdblockUI::~BraveAdblockUI() {
 }
 
-void BraveAdblockUI::CustomizeWebUIProperties() {
+void BraveAdblockUI::CustomizeWebUIProperties(content::RenderViewHost* render_view_host) {
   Profile* profile = Profile::FromWebUI(web_ui());
   PrefService* prefs = profile->GetPrefs();
-  auto* web_contents = web_ui()->GetWebContents();
-  if (web_contents) {
-    auto* render_view_host = web_contents->GetRenderViewHost();
-    if (render_view_host) {
-      render_view_host->SetWebUIProperty("adsBlockedStat", std::to_string(prefs->GetUint64(kAdsBlocked)));
-      render_view_host->SetWebUIProperty("regionalAdBlockEnabled",
-          std::to_string(
-            g_brave_browser_process->ad_block_regional_service()->IsInitialized()));
-      render_view_host->SetWebUIProperty("regionalAdBlockTitle",
-          g_brave_browser_process->ad_block_regional_service()->GetTitle());
-    }
+  if (render_view_host) {
+    render_view_host->SetWebUIProperty("adsBlockedStat", std::to_string(prefs->GetUint64(kAdsBlocked)));
+    render_view_host->SetWebUIProperty("regionalAdBlockEnabled",
+        std::to_string(
+          g_brave_browser_process->ad_block_regional_service()->IsInitialized()));
+    render_view_host->SetWebUIProperty("regionalAdBlockTitle",
+        g_brave_browser_process->ad_block_regional_service()->GetTitle());
   }
 }
 
 void BraveAdblockUI::RenderFrameCreated(content::RenderFrameHost* render_frame_host) {
   if (0 != (web_ui()->GetBindings() & content::BINDINGS_POLICY_WEB_UI)) {
-    CustomizeWebUIProperties();
+    CustomizeWebUIProperties(render_frame_host->GetRenderViewHost());
   }
 }
 
 void BraveAdblockUI::OnPreferenceChanged() {
   if (0 != (web_ui()->GetBindings() & content::BINDINGS_POLICY_WEB_UI)) {
-    CustomizeWebUIProperties();
+    CustomizeWebUIProperties(GetRenderViewHost());
     web_ui()->CallJavascriptFunctionUnsafe("brave_adblock.statsUpdated");
   }
 }

browser/ui/webui/brave_adblock_ui.h

@@ -16,8 +16,8 @@ class BraveAdblockUI : public BasicUI {
   ~BraveAdblockUI() override;
 
  private:
-  void CustomizeWebUIProperties();
   void RenderFrameCreated(content::RenderFrameHost* render_frame_host) override;
+  void CustomizeWebUIProperties(content::RenderViewHost* render_view_host);
   void OnPreferenceChanged();
 
   std::unique_ptr<PrefChangeRegistrar> pref_change_registrar_;

browser/ui/webui/brave_new_tab_ui.cc

@@ -13,7 +13,6 @@
 #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_contents.h"
 #include "content/public/browser/web_ui_data_source.h"
 #include "content/public/browser/web_ui_message_handler.h"
 #include "content/public/common/bindings_policy.h"
@@ -63,45 +62,41 @@ BraveNewTabUI::BraveNewTabUI(content::WebUI* web_ui, const std::string& name)
 BraveNewTabUI::~BraveNewTabUI() {
 }
 
-void BraveNewTabUI::CustomizeNewTabWebUIProperties() {
+void BraveNewTabUI::CustomizeNewTabWebUIProperties(content::RenderViewHost* render_view_host) {
   Profile* profile = Profile::FromWebUI(web_ui());
   PrefService* prefs = profile->GetPrefs();
-  auto* web_contents = web_ui()->GetWebContents();
-  if (web_contents) {
-    auto* render_view_host = web_contents->GetRenderViewHost();
-    if (render_view_host) {
-      render_view_host->SetWebUIProperty(
-          "adsBlockedStat",
-          std::to_string(prefs->GetUint64(kAdsBlocked)));
-      render_view_host->SetWebUIProperty(
-          "trackersBlockedStat",
-          std::to_string(prefs->GetUint64(kTrackersBlocked)));
-      render_view_host->SetWebUIProperty(
-          "javascriptBlockedStat",
-          std::to_string(prefs->GetUint64(kJavascriptBlocked)));
-      render_view_host->SetWebUIProperty(
-          "httpsUpgradesStat",
-          std::to_string(prefs->GetUint64(kHttpsUpgrades)));
-      render_view_host->SetWebUIProperty(
-          "fingerprintingBlockedStat",
-          std::to_string(prefs->GetUint64(kFingerprintingBlocked)));
-      render_view_host->SetWebUIProperty(
-          "useAlternativePrivateSearchEngine",
-          prefs->GetBoolean(kUseAlternatePrivateSearchEngine) ? "true"
-                                                              : "false");
-    }
+  if (render_view_host) {
+    render_view_host->SetWebUIProperty(
+        "adsBlockedStat",
+        std::to_string(prefs->GetUint64(kAdsBlocked)));
+    render_view_host->SetWebUIProperty(
+        "trackersBlockedStat",
+        std::to_string(prefs->GetUint64(kTrackersBlocked)));
+    render_view_host->SetWebUIProperty(
+        "javascriptBlockedStat",
+        std::to_string(prefs->GetUint64(kJavascriptBlocked)));
+    render_view_host->SetWebUIProperty(
+        "httpsUpgradesStat",
+        std::to_string(prefs->GetUint64(kHttpsUpgrades)));
+    render_view_host->SetWebUIProperty(
+        "fingerprintingBlockedStat",
+        std::to_string(prefs->GetUint64(kFingerprintingBlocked)));
+    render_view_host->SetWebUIProperty(
+        "useAlternativePrivateSearchEngine",
+        prefs->GetBoolean(kUseAlternatePrivateSearchEngine) ? "true"
+                                                            : "false");
   }
 }
 
 void BraveNewTabUI::RenderFrameCreated(content::RenderFrameHost* render_frame_host) {
   if (0 != (web_ui()->GetBindings() & content::BINDINGS_POLICY_WEB_UI)) {
-    CustomizeNewTabWebUIProperties();
+    CustomizeNewTabWebUIProperties(render_frame_host->GetRenderViewHost());
   }
 }
 
 void BraveNewTabUI::OnPreferenceChanged() {
   if (0 != (web_ui()->GetBindings() & content::BINDINGS_POLICY_WEB_UI)) {
-    CustomizeNewTabWebUIProperties();
+    CustomizeNewTabWebUIProperties(GetRenderViewHost());
     web_ui()->CallJavascriptFunctionUnsafe("brave_new_tab.statsUpdated");
   }
 }

browser/ui/webui/brave_new_tab_ui.h

@@ -16,8 +16,8 @@ class BraveNewTabUI : public BasicUI {
   ~BraveNewTabUI() override;
 
  private:
-  void CustomizeNewTabWebUIProperties();
   void RenderFrameCreated(content::RenderFrameHost* render_frame_host) override;
+  void CustomizeNewTabWebUIProperties(content::RenderViewHost* render_view_host);
   void OnPreferenceChanged();
 
   std::unique_ptr<PrefChangeRegistrar> pref_change_registrar_;

browser/ui/webui/brave_new_tab_ui_browsertest.cc

@@ -0,0 +1,77 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "base/path_service.h"
+#include "brave/common/brave_paths.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/webui_url_constants.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/child_process_termination_info.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_types.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/browser/render_process_host_observer.h"
+#include "content/public/test/browser_test_utils.h"
+
+using namespace content;
+
+namespace {
+class ObserverLogger : public RenderProcessHostObserver {
+ public:
+  explicit ObserverLogger(RenderProcessHost* observed_host) :
+      observed_host_(observed_host) {
+    observed_host->AddObserver(this);
+  }
+
+ protected:
+  // Make sure we aren't exiting because of a crash
+  void RenderProcessExited(RenderProcessHost* host,
+                           const ChildProcessTerminationInfo& info) override {
+    observed_host_->RemoveObserver(this);
+    EXPECT_EQ(info.exit_code, 0);
+  }
+  RenderProcessHost* observed_host_;
+};
+
+}  // namespace
+
+class BraveNewTabUIBrowserTest : public InProcessBrowserTest {
+ public:
+  void SetUpOnMainThread() override {
+    InProcessBrowserTest::SetUpOnMainThread();
+
+    brave::RegisterPathProvider();
+    base::FilePath test_data_dir;
+    base::PathService::Get(brave::DIR_TEST_DATA, &test_data_dir);
+    embedded_test_server()->ServeFilesFromDirectory(test_data_dir);
+    ASSERT_TRUE(embedded_test_server()->Start());
+  }
+  void GoBack(WebContents* web_contents) {
+    WindowedNotificationObserver load_stop_observer(
+        NOTIFICATION_LOAD_STOP,
+        NotificationService::AllSources());
+    web_contents->GetController().GoBack();
+    load_stop_observer.Wait();
+  }
+};
+
+// Test that properties are set on the correct RenderViewHost.
+IN_PROC_BROWSER_TEST_F(BraveNewTabUIBrowserTest, StartupURLTest) {
+  auto* contents = browser()->tab_strip_model()->GetActiveWebContents();
+  RenderProcessHost* host = contents->GetMainFrame()->GetProcess();
+  ObserverLogger observer_logger(host);
+
+  GURL new_tab_url(chrome::kChromeUINewTabURL);
+  ui_test_utils::NavigateToURL(browser(), new_tab_url);
+  WaitForLoadStop(contents);
+
+  GURL simple_url = embedded_test_server()->GetURL("/simple.html");
+  ui_test_utils::NavigateToURL(browser(), simple_url);
+  WaitForLoadStop(contents);
+
+  GoBack(contents);
+  WaitForLoadStop(contents);
+}

test/BUILD.gn

@@ -160,6 +160,7 @@ test("brave_browser_tests") {
     "//brave/browser/extensions/api/brave_shields_api_browsertest.cc",
     "//brave/browser/ui/content_settings/brave_autoplay_blocked_image_model_browsertest.cc",
     "//brave/browser/ui/content_settings/brave_widevine_blocked_image_model_browsertest.cc",
+    "//brave/browser/ui/webui/brave_new_tab_ui_browsertest.cc",
     "//brave/browser/ui/webui/brave_welcome_ui_browsertest.cc",
     "//brave/common/brave_channel_info_browsertest.cc",
     "//brave/components/brave_shields/browser/ad_block_service_browsertest.cc",