Make DOM Distiller's font and theme code more resilient.

The existing code relies on the document body's class list following a
specific format. This CL allows for an arbitrary class list, which in
turn makes it easier to update the theme on other elements. (This will
be needed for the appearance settings, assuming the settings are put in
the web contents.)

This CL also reworks how the JS font/theme maps to the CSS font/theme.
The JS and CSS strings are always the same, so we can just verify that
the JS string is one of the expected ones before returning it.

The associated browser tests are also modified to no longer expect a
specific class order. Monospace is used instead of serif to make
substring matching less complicated.

Bug: 952034
Change-Id: I99d61dd3a3edcfe66f52169309105d1e235b937a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1684251Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Commit-Queue: Aran Gilman <gilmanmh@google.com>
Cr-Commit-Position: refs/heads/master@{#676064}

chrome/browser/dom_distiller/dom_distiller_viewer_source_browsertest.cc

@@ -44,6 +44,7 @@
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/common/isolated_world_ids.h"
 #include "content/public/test/browser_test_utils.h"
+#include "testing/gmock/include/gmock/gmock-matchers.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -102,6 +103,16 @@ ArticleEntry CreateEntry(const std::string& entry_id,
   return entry;
 }
 
+void ExpectBodyHasThemeAndFont(content::WebContents* contents,
+                               const std::string& expected_theme,
+                               const std::string& expected_font) {
+  std::string result;
+  EXPECT_TRUE(
+      content::ExecuteScriptAndExtractString(contents, kGetBodyClass, &result));
+  EXPECT_THAT(result, HasSubstr(expected_theme));
+  EXPECT_THAT(result, HasSubstr(expected_font));
+}
+
 }  // namespace
 
 class DomDistillerViewerSourceBrowserTest : public InProcessBrowserTest {
@@ -557,10 +568,7 @@ void DomDistillerViewerSourceBrowserTest::PrefTest(bool is_error_page) {
       browser()->tab_strip_model()->GetActiveWebContents();
   ViewSingleDistilledPage(url, "text/html");
   content::WaitForLoadStop(contents);
-  std::string result;
-  EXPECT_TRUE(
-      content::ExecuteScriptAndExtractString(contents, kGetBodyClass, &result));
-  EXPECT_EQ("light sans-serif", result);
+  ExpectBodyHasThemeAndFont(contents, "light", "sans-serif");
 
   DistilledPagePrefs* distilled_page_prefs =
       DomDistillerServiceFactory::GetForBrowserContext(browser()->profile())
@@ -569,21 +577,19 @@ void DomDistillerViewerSourceBrowserTest::PrefTest(bool is_error_page) {
   // Test theme.
   distilled_page_prefs->SetTheme(DistilledPagePrefs::THEME_DARK);
   base::RunLoop().RunUntilIdle();
-  EXPECT_TRUE(
-      content::ExecuteScriptAndExtractString(contents, kGetBodyClass, &result));
-  EXPECT_EQ("dark sans-serif", result);
+  ExpectBodyHasThemeAndFont(contents, "dark", "sans-serif");
 
   // Verify that the theme color for the tab is updated as well.
   EXPECT_EQ(kDarkToolbarThemeColor, contents->GetThemeColor());
 
   // Test font family.
-  distilled_page_prefs->SetFontFamily(DistilledPagePrefs::FONT_FAMILY_SERIF);
+  distilled_page_prefs->SetFontFamily(
+      DistilledPagePrefs::FONT_FAMILY_MONOSPACE);
   base::RunLoop().RunUntilIdle();
-  EXPECT_TRUE(
-      content::ExecuteScriptAndExtractString(contents, kGetBodyClass, &result));
-  EXPECT_EQ("dark serif", result);
+  ExpectBodyHasThemeAndFont(contents, "dark", "monospace");
 
   // Test font scaling.
+  std::string result;
   EXPECT_TRUE(
       content::ExecuteScriptAndExtractString(contents, kGetFontSize, &result));
   double oldFontSize;
@@ -621,13 +627,13 @@ IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest, PrefPersist) {
   // Set preference.
   const double kScale = 1.23;
   distilled_page_prefs->SetTheme(DistilledPagePrefs::THEME_DARK);
-  distilled_page_prefs->SetFontFamily(DistilledPagePrefs::FONT_FAMILY_SERIF);
+  distilled_page_prefs->SetFontFamily(
+      DistilledPagePrefs::FONT_FAMILY_MONOSPACE);
   distilled_page_prefs->SetFontScaling(kScale);
 
   base::RunLoop().RunUntilIdle();
-  EXPECT_TRUE(
-      content::ExecuteScriptAndExtractString(contents, kGetBodyClass, &result));
-  EXPECT_EQ("dark serif", result);
+  ExpectBodyHasThemeAndFont(contents, "dark", "monospace");
+
   EXPECT_EQ(kDarkToolbarThemeColor, contents->GetThemeColor());
   EXPECT_TRUE(
       content::ExecuteScriptAndExtractString(contents, kGetFontSize, &result));
@@ -643,7 +649,7 @@ IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest, PrefPersist) {
   base::RunLoop().RunUntilIdle();
   EXPECT_TRUE(
       content::ExecuteScriptAndExtractString(contents, kGetBodyClass, &result));
-  EXPECT_EQ("dark serif", result);
+  ExpectBodyHasThemeAndFont(contents, "dark", "monospace");
   EXPECT_EQ(kDarkToolbarThemeColor, contents->GetThemeColor());
 
   EXPECT_TRUE(

components/dom_distiller/core/javascript/dom_distiller_viewer.js

@@ -58,46 +58,53 @@ function setTextDirection(direction) {
   document.body.setAttribute('dir', direction);
 }
 
-// Maps JS Font Family to CSS class and then changes body class name.
-// CSS classes must agree with distilledpage.css.
-function useFontFamily(fontFamily) {
-  var cssClass;
-  if (fontFamily == 'serif') {
-    cssClass = 'serif';
-  } else if (fontFamily == 'monospace') {
-    cssClass = 'monospace';
-  } else {
-    cssClass = 'sans-serif';
+function removeAll(source, elementsToRemove) {
+  elementsToRemove.forEach(function(element) {
+    source.remove(element)
+  });
+}
+
+// These classes must agree with the font classes in distilledpage.css.
+const fontFamilyClasses = ['sans-serif', 'serif', 'monospace'];
+function getFontFamilyClass(fontFamily) {
+  if (fontFamilyClasses.includes(fontFamily)) {
+    return fontFamily;
   }
-  // Relies on the classname order of the body being Theme class, then Font
-  // Family class.
-  var themeClass = document.body.className.split(' ')[0];
-  document.body.className = themeClass + ' ' + cssClass;
+  return fontFamilyClasses[0];
 }
 
-// Maps JS theme to CSS class and then changes body class name.
-// CSS classes must agree with distilledpage.css.
-function useTheme(theme) {
-  var cssClass;
-  if (theme == 'sepia') {
-    cssClass = 'sepia';
-  } else if (theme == 'dark') {
-    cssClass = 'dark';
-  } else {
-    cssClass = 'light';
+function useFontFamily(fontFamily) {
+  removeAll(document.body.classList, fontFamilyClasses);
+  document.body.classList.add(getFontFamilyClass(fontFamily));
+}
+
+// These classes must agree with the theme classes in distilledpage.css.
+const themeClasses = ['light', 'dark', 'sepia'];
+function getThemeClass(theme) {
+  if (themeClasses.includes(theme)) {
+    return theme;
   }
-  // Relies on the classname order of the body being Theme class, then Font
-  // Family class.
-  var fontFamilyClass = document.body.className.split(' ')[1];
-  document.body.className = cssClass + ' ' + fontFamilyClass;
+  return themeClasses[0];
+}
 
+function useTheme(theme) {
+  removeAll(document.body.classList, themeClasses);
+  document.body.classList.add(getThemeClass(theme));
   updateToolbarColor();
 }
 
+function getThemeFromElement(element) {
+  var foundTheme = themeClasses[0];
+  themeClasses.forEach(function(theme) {
+    if (element.classList.contains(theme)) {
+      foundTheme = theme;
+    }
+  });
+  return foundTheme;
+}
+
 function updateToolbarColor() {
-  // Relies on the classname order of the body being Theme class, then Font
-  // Family class.
-  var themeClass = document.body.className.split(' ')[0];
+  var themeClass = getThemeFromElement(document.body);
 
   var toolbarColor;
   if (themeClass == 'sepia') {