Revert "Reland "Persist Reader Mode theme and font family settings on desktop.""

This reverts commit d1173dbd.

Reason for revert: DomDistillerViewerSourceBrowserTest.UISetsPrefs flaky

Original change's description:
> Reland "Persist Reader Mode theme and font family settings on desktop."
> 
> This reverts commit 23979013, which
> reverted an earlier implementation of this CL in commit
> a18fd063 due to
> https://crbug.com/1065341.
> 
> The main functional difference between this and the original version is
> where the JavaScript code calls the pref-storing functions. They were
> previously called inside useTheme() and useFontFamily() instead of the
> UI callbacks. Changes to the synced prefs result in a series of
> observers being notified, which ultimately led to the useTheme() and
> useFontFamily() methods being called again, potentially leading to an
> infinite loop and the behavior seen in the bug.
> 
> The updateThemeSelection() method from the original CL was excluded in
> this reimplementation because it was unintentionally never called in the
> original CL. Omitting it helps reduce the scope of the CL and highlight
> the important changes.
> 
> Bug: 1016615, 1065341
> Change-Id: Ifd0e71027afd196267419ed81f7bdc0d78978a98
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2152901
> Reviewed-by: Emily Stark <estark@chromium.org>
> Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
> Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
> Commit-Queue: Aran Gilman <gilmanmh@google.com>
> Cr-Commit-Position: refs/heads/master@{#762524}

TBR=dmazzoni@chromium.org,estark@chromium.org,wychen@chromium.org,gilmanmh@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1016615, 1065341, 1075439
Change-Id: I222e95940848b68fa8158fd64aaca76048ed96db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2167751Reviewed-by: Alexander Hendrich <hendrich@chromium.org>
Commit-Queue: Alexander Hendrich <hendrich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#762756}

chrome/browser/chrome_browser_interface_binders.cc

@@ -253,9 +253,7 @@ void BindDistillerJavaScriptService(
       distiller_ui_handle)
       ->set_render_frame_host(frame_host);
 #endif
-  auto* distilled_page_prefs = dom_distiller_service->GetDistilledPagePrefs();
-  CreateDistillerJavaScriptService(distiller_ui_handle, distilled_page_prefs,
-                                   std::move(receiver));
+  CreateDistillerJavaScriptService(distiller_ui_handle, std::move(receiver));
 }
 
 void BindPrerenderCanceler(

chrome/browser/dom_distiller/dom_distiller_viewer_source_browsertest.cc

@@ -601,61 +601,4 @@ IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest, PrefPersist) {
   ASSERT_FLOAT_EQ(kScale, fontSize / oldFontSize);
 }
 
-IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest, UISetsPrefs) {
-  content::WebContents* contents =
-      browser()->tab_strip_model()->GetActiveWebContents();
-
-  // Load the distilled page
-  GURL view_url;
-  expect_distillation_ = true;
-  expect_distiller_page_ = true;
-  GURL original_url("http://www.example.com/1");
-  view_url = url_utils::GetDistillerViewUrlFromUrl(kDomDistillerScheme,
-                                                   original_url, "Title");
-  ViewSingleDistilledPage(view_url, "text/html");
-  content::WaitForLoadStop(contents);
-
-  DistilledPagePrefs* distilled_page_prefs =
-      DomDistillerServiceFactory::GetForBrowserContext(browser()->profile())
-          ->GetDistilledPagePrefs();
-
-  // Verify that the initial preferences aren't the same as those set below.
-  ExpectBodyHasThemeAndFont(contents, "light", "sans-serif");
-  EXPECT_NE(mojom::Theme::kDark, distilled_page_prefs->GetTheme());
-  EXPECT_NE(mojom::FontFamily::kMonospace,
-            distilled_page_prefs->GetFontFamily());
-
-  // 'Click' the associated UI elements for changing each preference.
-  const std::string script = R"(
-      (() => {
-        const observer = new MutationObserver((mutationsList, observer) => {
-          const classes = document.body.classList;
-          if (classes.contains('dark') && classes.contains('monospace')) {
-            observer.disconnect();
-            window.domAutomationController.send(document.body.className);
-          }
-        });
-
-        observer.observe(document.body, {
-          attributes: true,
-          attributeFilter: [ 'class' ]
-        });
-
-        document.querySelector('.theme-option .dark')
-          .dispatchEvent(new MouseEvent('click'));
-        document.querySelector(
-            '#font-family-selection option[value="monospace"]')
-          .dispatchEvent(new Event("change", { bubbles: true }));
-      })();)";
-  content::DOMMessageQueue queue(contents);
-  std::string result;
-  content::ExecuteScriptAsync(contents, script);
-  EXPECT_TRUE(queue.WaitForMessage(&result));
-
-  // Verify that the preferences changed in the browser-side DistilledPagePrefs.
-  EXPECT_EQ(mojom::Theme::kDark, distilled_page_prefs->GetTheme());
-  EXPECT_EQ(mojom::FontFamily::kMonospace,
-            distilled_page_prefs->GetFontFamily());
-}
-
 }  // namespace dom_distiller

components/dom_distiller/content/browser/distiller_javascript_service_impl.cc

@@ -9,10 +9,8 @@
 namespace dom_distiller {
 
 DistillerJavaScriptServiceImpl::DistillerJavaScriptServiceImpl(
-    DistillerUIHandle* distiller_ui_handle,
-    DistilledPagePrefs* distilled_page_prefs)
-    : distiller_ui_handle_(distiller_ui_handle),
-      distilled_page_prefs_(distilled_page_prefs) {}
+    DistillerUIHandle* distiller_ui_handle)
+    : distiller_ui_handle_(distiller_ui_handle) {}
 
 DistillerJavaScriptServiceImpl::~DistillerJavaScriptServiceImpl() = default;
 
@@ -24,22 +22,12 @@ void DistillerJavaScriptServiceImpl::HandleDistillerOpenSettingsCall() {
   distiller_ui_handle_->OpenSettings();
 }
 
-void DistillerJavaScriptServiceImpl::HandleStoreThemePref(mojom::Theme theme) {
-  distilled_page_prefs_->SetTheme(theme);
-}
-
-void DistillerJavaScriptServiceImpl::HandleStoreFontFamilyPref(
-    mojom::FontFamily font_family) {
-  distilled_page_prefs_->SetFontFamily(font_family);
-}
-
 void CreateDistillerJavaScriptService(
     DistillerUIHandle* distiller_ui_handle,
-    DistilledPagePrefs* distilled_page_prefs,
     mojo::PendingReceiver<mojom::DistillerJavaScriptService> receiver) {
-  mojo::MakeSelfOwnedReceiver(std::make_unique<DistillerJavaScriptServiceImpl>(
-                                  distiller_ui_handle, distilled_page_prefs),
-                              std::move(receiver));
+  mojo::MakeSelfOwnedReceiver(
+      std::make_unique<DistillerJavaScriptServiceImpl>(distiller_ui_handle),
+      std::move(receiver));
 }
 
 }  // namespace dom_distiller

components/dom_distiller/content/browser/distiller_javascript_service_impl.h

@@ -6,21 +6,17 @@
 #define COMPONENTS_DOM_DISTILLER_CONTENT_BROWSER_DISTILLER_JAVASCRIPT_SERVICE_IMPL_H_
 
 #include "components/dom_distiller/content/common/mojom/distiller_javascript_service.mojom.h"
-#include "components/dom_distiller/core/distilled_page_prefs.h"
 #include "components/dom_distiller/core/distiller_ui_handle.h"
-#include "components/dom_distiller/core/mojom/distilled_page_prefs.mojom.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 
 namespace dom_distiller {
 
-class DistilledPagePrefs;
-
 // This is the receiving end of "distiller" JavaScript object calls.
 class DistillerJavaScriptServiceImpl
     : public mojom::DistillerJavaScriptService {
  public:
-  DistillerJavaScriptServiceImpl(DistillerUIHandle* distiller_ui_handle,
-                                 DistilledPagePrefs* distilled_page_prefs);
+  explicit DistillerJavaScriptServiceImpl(
+      DistillerUIHandle* distiller_ui_handle);
   ~DistillerJavaScriptServiceImpl() override;
 
   // Mojo mojom::DistillerJavaScriptService implementation.
@@ -28,9 +24,6 @@ class DistillerJavaScriptServiceImpl
   // Show the Android view containing Reader Mode settings.
   void HandleDistillerOpenSettingsCall() override;
 
-  void HandleStoreThemePref(mojom::Theme theme) override;
-  void HandleStoreFontFamilyPref(mojom::FontFamily font_family) override;
-
   DistillerJavaScriptServiceImpl(const DistillerJavaScriptServiceImpl&) =
       delete;
   DistillerJavaScriptServiceImpl& operator=(
@@ -38,13 +31,11 @@ class DistillerJavaScriptServiceImpl
 
  private:
   DistillerUIHandle* distiller_ui_handle_;
-  DistilledPagePrefs* distilled_page_prefs_;
 };
 
 // static
 void CreateDistillerJavaScriptService(
     DistillerUIHandle* distiller_ui_handle,
-    DistilledPagePrefs* distilled_page_prefs,
     mojo::PendingReceiver<mojom::DistillerJavaScriptService> receiver);
 
 }  // namespace dom_distiller

components/dom_distiller/content/common/mojom/BUILD.gn

@@ -10,5 +10,4 @@ mojom("mojom") {
     "distiller_javascript_service.mojom",
     "distiller_page_notifier_service.mojom",
   ]
-  deps = [ "//components/dom_distiller/core/mojom" ]
 }

components/dom_distiller/content/common/mojom/distiller_javascript_service.mojom

@@ -4,16 +4,10 @@
 
 module dom_distiller.mojom;
 
-import "components/dom_distiller/core/mojom/distilled_page_prefs.mojom";
-
 // This service is implemented by the browser process and is used by the
 // renderer when a distiller JavaScript function is called.
 interface DistillerJavaScriptService {
   // Open the Android view containing settings for Reader Mode; the
   // "distiller.openSettings" function.
   HandleDistillerOpenSettingsCall();
-
-  // Store updated appearance settings as synced prefs.
-  HandleStoreThemePref(Theme theme);
-  HandleStoreFontFamilyPref(FontFamily font_family);
 };

components/dom_distiller/content/renderer/BUILD.gn

@@ -19,7 +19,6 @@ static_library("renderer") {
     "//base",
     "//components/dom_distiller/content/common/mojom",
     "//components/dom_distiller/core",
-    "//components/dom_distiller/core/mojom",
     "//content/public/common",
     "//content/public/renderer",
     "//gin",

components/dom_distiller/content/renderer/distiller_native_javascript.cc

@@ -9,8 +9,6 @@
 #include "base/bind.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/dom_distiller/content/common/mojom/distiller_javascript_service.mojom.h"
-#include "components/dom_distiller/core/distilled_page_prefs.h"
-#include "components/dom_distiller/core/mojom/distilled_page_prefs.mojom.h"
 #include "content/public/renderer/render_frame.h"
 #include "gin/arguments.h"
 #include "gin/function_template.h"
@@ -26,20 +24,6 @@ DistillerNativeJavaScript::DistillerNativeJavaScript(
 
 DistillerNativeJavaScript::~DistillerNativeJavaScript() = default;
 
-void DistillerNativeJavaScript::StoreIntTheme(int int_theme) {
-  auto theme = static_cast<mojom::Theme>(int_theme);
-  if (!mojom::IsKnownEnumValue(theme))
-    return;
-  distiller_js_service_->HandleStoreThemePref(theme);
-}
-
-void DistillerNativeJavaScript::StoreIntFontFamily(int int_font_family) {
-  auto font_family = static_cast<mojom::FontFamily>(int_font_family);
-  if (!mojom::IsKnownEnumValue(font_family))
-    return;
-  distiller_js_service_->HandleStoreFontFamilyPref(font_family);
-}
-
 void DistillerNativeJavaScript::AddJavaScriptObjectToFrame(
     v8::Local<v8::Context> context) {
   v8::Isolate* isolate = blink::MainThreadIsolate();
@@ -62,15 +46,6 @@ void DistillerNativeJavaScript::AddJavaScriptObjectToFrame(
       base::BindRepeating(
           &mojom::DistillerJavaScriptService::HandleDistillerOpenSettingsCall,
           base::Unretained(distiller_js_service_.get())));
-
-  BindFunctionToObject(isolate, distiller_obj, "storeThemePref",
-                       base::Bind(&DistillerNativeJavaScript::StoreIntTheme,
-                                  base::Unretained(this)));
-
-  BindFunctionToObject(
-      isolate, distiller_obj, "storeFontFamilyPref",
-      base::Bind(&DistillerNativeJavaScript::StoreIntFontFamily,
-                 base::Unretained(this)));
 }
 
 template <typename Sig>

components/dom_distiller/content/renderer/distiller_native_javascript.h

@@ -36,11 +36,6 @@ class DistillerNativeJavaScript {
   // Make sure the mojo service is connected.
   void EnsureServiceConnected();
 
-  // Wrappers to convert integer representations of the pref enums, then send
-  // the enum values to the browser process.
-  void StoreIntTheme(int theme);
-  void StoreIntFontFamily(int font_family);
-
   content::RenderFrame* render_frame_;
   mojo::Remote<mojom::DistillerJavaScriptService> distiller_js_service_;
 };

components/dom_distiller/core/javascript/dom_distiller_viewer.js

@@ -454,13 +454,9 @@ $('close-settings-button').addEventListener('click', (e) => {
 });
 
 $('theme-selection').addEventListener('change', (e) => {
-  const newTheme = e.target.value;
-  useTheme(newTheme);
-  distiller.storeThemePref(themeClasses.indexOf(newTheme));
+  useTheme(e.target.value);
 });
 
 $('font-family-selection').addEventListener('change', (e) => {
-  const newFontFamily = e.target.value;
-  useFontFamily(newFontFamily);
-  distiller.storeFontFamilyPref(fontFamilyClasses.indexOf(newFontFamily));
+  useFontFamily(e.target.value);
 });