Speedreader communicate result to tab_helper

browser/brave_browser_main_parts.cc

@@ -13,6 +13,7 @@
 #include "brave/common/brave_constants.h"
 #include "brave/common/pref_names.h"
 #include "brave/components/brave_sync/features.h"
+#include "brave/components/speedreader/buildflags.h"
 #include "brave/components/tor/buildflags/buildflags.h"
 #include "chrome/common/chrome_features.h"
 #include "components/prefs/pref_service.h"
@@ -21,6 +22,11 @@
 #include "extensions/buildflags/buildflags.h"
 #include "media/base/media_switches.h"
 
+#if BUILDFLAG(ENABLE_SPEEDREADER)
+#include "brave/components/speedreader/speedreader_extended_info_handler.h"
+#include "components/sessions/content/content_serialized_navigation_driver.h"
+#endif
+
 #if BUILDFLAG(ENABLE_TOR)
 #include <string>
 #include "base/files/file_util.h"
@@ -59,6 +65,19 @@
 #include "extensions/browser/extension_system.h"
 #endif
 
+void BraveBrowserMainParts::PreBrowserStart() {
+#if BUILDFLAG(ENABLE_SPEEDREADER)
+  // Register() must be called after the SerializedNavigationDriver is
+  // initialized, but before any calls to
+  // ContentSerializedNavigationBuilder::ToNavigationEntries()
+  //
+  // TODO(keur): Can we DCHECK the latter condition?
+  DCHECK(sessions::ContentSerializedNavigationDriver::GetInstance());
+  speedreader::SpeedreaderExtendedInfoHandler::Register();
+#endif
+  ChromeBrowserMainParts::PreBrowserStart();
+}
+
 void BraveBrowserMainParts::PostBrowserStart() {
   ChromeBrowserMainParts::PostBrowserStart();
 

browser/brave_browser_main_parts.h

@@ -14,6 +14,7 @@ class BraveBrowserMainParts : public ChromeBrowserMainParts {
   using ChromeBrowserMainParts::ChromeBrowserMainParts;
   ~BraveBrowserMainParts() override = default;
 
+  void PreBrowserStart() override;
   void PostBrowserStart() override;
   void PreShutdown() override;
   void PreProfileInit() override;

browser/brave_content_browser_client.cc

@@ -48,6 +48,7 @@
 #include "brave/components/gemini/browser/buildflags/buildflags.h"
 #include "brave/components/ipfs/buildflags/buildflags.h"
 #include "brave/components/speedreader/buildflags.h"
+#include "brave/components/speedreader/speedreader_util.h"
 #include "brave/components/tor/buildflags/buildflags.h"
 #include "brave/grit/brave_generated_resources.h"
 #include "chrome/browser/browser_process.h"
@@ -492,14 +493,14 @@ std::vector<std::unique_ptr<blink::URLLoaderThrottle>>
 BraveContentBrowserClient::CreateURLLoaderThrottles(
     const network::ResourceRequest& request,
     content::BrowserContext* browser_context,
-    const base::RepeatingCallback<content::WebContents*()>& wc_getter,
+    const content::WebContents::Getter& wc_getter,
     content::NavigationUIData* navigation_ui_data,
     int frame_tree_node_id) {
   auto result = ChromeContentBrowserClient::CreateURLLoaderThrottles(
       request, browser_context, wc_getter, navigation_ui_data,
       frame_tree_node_id);
 #if BUILDFLAG(ENABLE_SPEEDREADER)
-  using DistillState = speedreader::SpeedreaderTabHelper::DistillState;
+  using DistillState = speedreader::DistillState;
   content::WebContents* contents = wc_getter.Run();
   if (!contents) {
     return result;
@@ -508,15 +509,18 @@ BraveContentBrowserClient::CreateURLLoaderThrottles(
       speedreader::SpeedreaderTabHelper::FromWebContents(contents);
   if (tab_helper) {
     const auto state = tab_helper->PageDistillState();
-    if (speedreader::SpeedreaderTabHelper::PageStateIsDistilled(state) &&
+    if (speedreader::PageWantsDistill(state) &&
         request.resource_type ==
             static_cast<int>(blink::mojom::ResourceType::kMainFrame)) {
+      // Only check for disabled sites if we are in Speedreader mode
+      const bool check_disabled_sites =
+          state == DistillState::kSpeedreaderModePending;
       std::unique_ptr<speedreader::SpeedReaderThrottle> throttle =
           speedreader::SpeedReaderThrottle::MaybeCreateThrottleFor(
               g_brave_browser_process->speedreader_rewriter_service(),
               HostContentSettingsMapFactory::GetForProfile(
                   Profile::FromBrowserContext(browser_context)),
-              request.url, state == DistillState::kSpeedreaderMode,
+              tab_helper->GetWeakPtr(), request.url, check_disabled_sites,
               base::ThreadTaskRunnerHandle::Get());
       if (throttle)
         result.push_back(std::move(throttle));

browser/speedreader/speedreader_browsertest.cc

@@ -12,17 +12,26 @@
 #include "brave/components/speedreader/features.h"
 #include "brave/components/speedreader/speedreader_service.h"
 #include "brave/components/speedreader/speedreader_switches.h"
+#include "chrome/browser/profiles/profile_keep_alive_types.h"
+#include "chrome/browser/profiles/scoped_profile_keep_alive.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_commands.h"
+#include "chrome/browser/ui/browser_list.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "components/keep_alive_registry/keep_alive_types.h"
+#include "components/keep_alive_registry/scoped_keep_alive.h"
 #include "components/network_session_configurator/common/network_switches.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/browser_test_utils.h"
+#include "content/public/test/test_navigation_observer.h"
+#include "content/public/test/test_utils.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 
 const char kTestHost[] = "theguardian.com";
-const char kTestPage[] = "/guardian.html";
+const char kTestPageSimple[] = "/simple.html";
+const char kTestPageReadable[] = "/articles/guardian.html";
 const base::FilePath::StringPieceType kTestWhitelist =
     FILE_PATH_LITERAL("speedreader_whitelist.json");
 
@@ -65,33 +74,96 @@ class SpeedReaderBrowserTest : public InProcessBrowserTest {
     host_resolver()->AddRule("*", "127.0.0.1");
   }
 
+  void TearDownOnMainThread() override { DisableSpeedreader(); }
+
   content::WebContents* ActiveWebContents() {
     return browser()->tab_strip_model()->GetActiveWebContents();
   }
 
+  speedreader::SpeedreaderTabHelper* tab_helper() {
+    return speedreader::SpeedreaderTabHelper::FromWebContents(
+        ActiveWebContents());
+  }
+
   speedreader::SpeedreaderService* speedreader_service() {
     return speedreader::SpeedreaderServiceFactory::GetForProfile(
         browser()->profile());
   }
 
   void ToggleSpeedreader() {
-    auto* speedreader_service =
-        speedreader::SpeedreaderServiceFactory::GetForProfile(
-            browser()->profile());
-    speedreader_service->ToggleSpeedreader();
+    speedreader_service()->ToggleSpeedreader();
     ActiveWebContents()->GetController().Reload(content::ReloadType::NORMAL,
                                                 false);
   }
 
+  void DisableSpeedreader() {
+    speedreader_service()->DisableSpeedreaderForTest();
+  }
+
+  void GoBack(Browser* browser) {
+    content::TestNavigationObserver observer(ActiveWebContents());
+    chrome::GoBack(browser, WindowOpenDisposition::CURRENT_TAB);
+    observer.Wait();
+  }
+
+  void NavigateToPageSynchronously(
+      const char* path,
+      WindowOpenDisposition disposition =
+          WindowOpenDisposition::NEW_FOREGROUND_TAB) {
+    const GURL url = https_server_.GetURL(kTestHost, path);
+    ui_test_utils::NavigateToURLWithDisposition(
+        browser(), url, disposition,
+        ui_test_utils::BROWSER_TEST_WAIT_FOR_LOAD_STOP);
+  }
+
  protected:
   base::test::ScopedFeatureList feature_list_;
   net::EmbeddedTestServer https_server_;
 };
 
+IN_PROC_BROWSER_TEST_F(SpeedReaderBrowserTest, RestoreSpeedreaderPage) {
+  ToggleSpeedreader();
+  NavigateToPageSynchronously(kTestPageReadable);
+  EXPECT_TRUE(
+      speedreader::PageStateIsDistilled(tab_helper()->PageDistillState()));
+
+  Profile* profile = browser()->profile();
+
+  ScopedKeepAlive test_keep_alive(KeepAliveOrigin::PANEL_VIEW,
+                                  KeepAliveRestartOption::DISABLED);
+  ScopedProfileKeepAlive test_profile_keep_alive(
+      profile, ProfileKeepAliveOrigin::kBrowserWindow);
+  CloseBrowserSynchronously(browser());
+
+  EXPECT_EQ(0u, BrowserList::GetInstance()->size());
+  chrome::OpenWindowWithRestoredTabs(profile);
+  EXPECT_EQ(1u, BrowserList::GetInstance()->size());
+  SelectFirstBrowser();
+  EXPECT_TRUE(
+      speedreader::PageStateIsDistilled(tab_helper()->PageDistillState()));
+}
+
+IN_PROC_BROWSER_TEST_F(SpeedReaderBrowserTest, NavigationNostickTest) {
+  ToggleSpeedreader();
+  NavigateToPageSynchronously(kTestPageSimple);
+  EXPECT_FALSE(
+      speedreader::PageStateIsDistilled(tab_helper()->PageDistillState()));
+  NavigateToPageSynchronously(kTestPageReadable,
+                              WindowOpenDisposition::CURRENT_TAB);
+  EXPECT_TRUE(
+      speedreader::PageStateIsDistilled(tab_helper()->PageDistillState()));
+
+  // Ensure distill state doesn't stick when we back-navigate from a readable
+  // page to a non-readable one.
+  GoBack(browser());
+  EXPECT_FALSE(
+      speedreader::PageStateIsDistilled(tab_helper()->PageDistillState()));
+}
+
 // disabled in https://github.com/brave/brave-browser/issues/11328
 IN_PROC_BROWSER_TEST_F(SpeedReaderBrowserTest, DISABLED_SmokeTest) {
   ToggleSpeedreader();
-  const GURL url = https_server_.GetURL(kTestHost, kTestPage);
+  const GURL url = https_server_.GetURL(kTestHost, kTestPageReadable);
   ui_test_utils::NavigateToURL(browser(), url);
   content::WebContents* contents = ActiveWebContents();
   content::RenderFrameHost* rfh = contents->GetMainFrame();
@@ -116,9 +188,6 @@ IN_PROC_BROWSER_TEST_F(SpeedReaderBrowserTest, DISABLED_SmokeTest) {
 IN_PROC_BROWSER_TEST_F(SpeedReaderBrowserTest, P3ATest) {
   base::HistogramTester tester;
 
-  const GURL url = https_server_.GetURL(kTestHost, kTestPage);
-  ui_test_utils::NavigateToURL(browser(), url);
-
   // SpeedReader never enabled
   EXPECT_FALSE(speedreader_service()->IsEnabled());
   tester.ExpectBucketCount(kSpeedreaderEnabledUMAHistogramName, 0, 1);

browser/speedreader/speedreader_tab_helper.cc

@@ -9,6 +9,7 @@
 #include "brave/browser/speedreader/speedreader_service_factory.h"
 #include "brave/browser/ui/brave_browser_window.h"
 #include "brave/browser/ui/speedreader/speedreader_bubble_view.h"
+#include "brave/components/speedreader/speedreader_extended_info_handler.h"
 #include "brave/components/speedreader/speedreader_rewriter_service.h"
 #include "brave/components/speedreader/speedreader_service.h"
 #include "brave/components/speedreader/speedreader_test_whitelist.h"
@@ -17,6 +18,8 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
+#include "content/public/browser/navigation_details.h"
+#include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/web_contents.h"
 
@@ -26,6 +29,10 @@ SpeedreaderTabHelper::~SpeedreaderTabHelper() {
   HideBubble();
 }
 
+base::WeakPtr<SpeedreaderTabHelper> SpeedreaderTabHelper::GetWeakPtr() {
+  return weak_factory_.GetWeakPtr();
+}
+
 SpeedreaderTabHelper::SpeedreaderTabHelper(content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents) {}
 
@@ -85,13 +92,37 @@ void SpeedreaderTabHelper::SingleShotSpeedreader() {
   }
 }
 
+bool SpeedreaderTabHelper::MaybeUpdateCachedState(
+    content::NavigationHandle* handle) {
+  auto* entry = handle->GetNavigationEntry();
+  if (!entry) {
+    return false;
+  }
+  Profile* profile =
+      Profile::FromBrowserContext(web_contents()->GetBrowserContext());
+  DCHECK(profile);
+  auto* speedreader_service = SpeedreaderServiceFactory::GetForProfile(profile);
+
+  bool cached = false;
+  DistillState state =
+      SpeedreaderExtendedInfoHandler::GetCachedMode(entry, speedreader_service);
+  if (state != DistillState::kUnknown) {
+    cached = true;
+    distill_state_ = state;
+  }
+  if (!cached) {
+    SpeedreaderExtendedInfoHandler::ClearPersistedData(entry);
+  }
+  return cached;
+}
+
 void SpeedreaderTabHelper::UpdateActiveState(
     content::NavigationHandle* handle) {
   DCHECK(handle);
   DCHECK(handle->IsInMainFrame());
 
   if (single_shot_next_request_) {
-    SetNextRequestState(DistillState::kReaderMode);
+    SetNextRequestState(DistillState::kReaderModePending);
     return;
   }
 
@@ -108,7 +139,7 @@ void SpeedreaderTabHelper::UpdateActiveState(
       } else if (!IsEnabledForSite(handle->GetURL())) {
         SetNextRequestState(DistillState::kSpeedreaderOnDisabledPage);
       } else {
-        SetNextRequestState(DistillState::kSpeedreaderMode);
+        SetNextRequestState(DistillState::kSpeedreaderModePending);
       }
       return;
     }
@@ -124,14 +155,18 @@ void SpeedreaderTabHelper::SetNextRequestState(DistillState state) {
 void SpeedreaderTabHelper::DidStartNavigation(
     content::NavigationHandle* navigation_handle) {
   if (navigation_handle->IsInMainFrame()) {
-    UpdateActiveState(navigation_handle);
+    if (!MaybeUpdateCachedState(navigation_handle)) {
+      UpdateActiveState(navigation_handle);
+    }
   }
 }
 
 void SpeedreaderTabHelper::DidRedirectNavigation(
     content::NavigationHandle* navigation_handle) {
   if (navigation_handle->IsInMainFrame()) {
-    UpdateActiveState(navigation_handle);
+    if (!MaybeUpdateCachedState(navigation_handle)) {
+      UpdateActiveState(navigation_handle);
+    }
   }
 }
 
@@ -173,6 +208,26 @@ void SpeedreaderTabHelper::HideBubble() {
   }
 }
 
+void SpeedreaderTabHelper::OnDistillComplete() {
+  // Perform a state transition
+  if (distill_state_ == DistillState::kSpeedreaderModePending) {
+    distill_state_ = DistillState::kSpeedreaderMode;
+  } else if (distill_state_ == DistillState::kReaderModePending) {
+    distill_state_ = DistillState::kReaderMode;
+  } else {
+    // We got here via an already cached page.
+    DCHECK(distill_state_ == DistillState::kSpeedreaderMode ||
+           distill_state_ == DistillState::kReaderMode);
+  }
+}
+
+void SpeedreaderTabHelper::DidStopLoading() {
+  auto* entry = web_contents()->GetController().GetLastCommittedEntry();
+  if (entry) {
+    SpeedreaderExtendedInfoHandler::PersistMode(entry, distill_state_);
+  }
+}
+
 WEB_CONTENTS_USER_DATA_KEY_IMPL(SpeedreaderTabHelper)
 
 }  // namespace speedreader