Don't send uninitialized ThemeBackgroundInfo to New Tab page.

Also, start using assert() in NTP code. This is helpful for compile time
typechecking and runtime enforcement.

Based on https://crrev.com/c/1763345 by Evan Stade <estade@chromium.org>

R=gayane@chromium.org
BUG=996209

Change-Id: I6ab3f92e93ae52249bafd722733aa4bb63289930
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1764794
Commit-Queue: Dan Beam <dbeam@chromium.org>
Reviewed-by: Gayane Petrosyan <gayane@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690161}

chrome/browser/resources/local_ntp/BUILD.gn

@@ -23,5 +23,8 @@ js_library("local_ntp") {
     "utils.js",
     "voice.js",
   ]
+  deps = [
+    "//ui/webui/resources/js:assert",
+  ]
   externs_list = [ "externs.js" ]
 }

chrome/browser/resources/local_ntp/customize.js

@@ -1424,7 +1424,7 @@ customize.richerPicker_preselectBackgroundOption = function() {
 
   customize.preselectedOptions.backgroundsMenuTile = null;
 
-  const themeInfo = ntpApiHandle.themeBackgroundInfo;
+  const themeInfo = assert(ntpApiHandle.themeBackgroundInfo);
   if (!themeInfo.customBackgroundConfigured) {
     // Default.
     customize.preselectedOptions.backgroundsMenuTile =
@@ -2302,7 +2302,7 @@ customize.loadColorsMenu = function() {
  */
 customize.colorsMenuOnThemeChange = function() {
   // Update webstore theme information.
-  const themeInfo = ntpApiHandle.themeBackgroundInfo;
+  const themeInfo = assert(ntpApiHandle.themeBackgroundInfo);
   if (themeInfo.themeId && themeInfo.themeName) {
     $(customize.IDS.COLORS_THEME).classList.add(customize.CLASSES.VISIBLE);
     $(customize.IDS.COLORS_THEME_NAME).innerHTML = themeInfo.themeName;
@@ -2329,7 +2329,7 @@ customize.colorsMenuOnThemeChange = function() {
  * Preselect Colors menu tile according to the theme info.
  */
 customize.colorsMenuPreselectTile = function() {
-  const themeInfo = ntpApiHandle.themeBackgroundInfo;
+  const themeInfo = assert(ntpApiHandle.themeBackgroundInfo);
 
   let tile;
   if (themeInfo.usingDefaultTheme) {

chrome/browser/resources/local_ntp/externs.js

@@ -303,8 +303,38 @@ window.chrome.embeddedSearch.newTabPage.resetBackgroundInfo;
 window.chrome.embeddedSearch.newTabPage.setBackgroundInfo;
 
 /**
- * @return {Object} theme_background_info
- */
+ * @typedef {{
+ *   alternateLogo: boolean,
+ *   attribution1: (string|undefined),
+ *   attribution2: (string|undefined),
+ *   attributionActionUrl: (string|undefined),
+ *   attributionUrl: (string|undefined),
+ *   backgroundColorRgba: !Array<number>,
+ *   collectionId: (string|undefined),
+ *   colorDark: (!Array<number>|undefined),
+ *   colorId: (number|undefined),
+ *   colorLight: (!Array<number>|undefined),
+ *   colorPicked: (!Array<number>|undefined),
+ *   customBackgroundConfigured: boolean,
+ *   iconBackgroundColor: !Array<number>,
+ *   imageHorizontalAlignment: (string|undefined),
+ *   imageTiling: (string|undefined),
+ *   imageUrl: (string|undefined),
+ *   imageVerticalAlignment: (string|undefined),
+ *   isNtpBackgroundDark: boolean,
+ *   logoColor: (!Array<number>|undefined),
+ *   textColorLightRgba: !Array<number>,
+ *   textColorRgba: !Array<number>,
+ *   themeId: (string|undefined),
+ *   themeName: (string|undefined),
+ *   useTitleContainer: boolean,
+ *   useWhiteAddIcon: boolean,
+ *   usingDefaultTheme: boolean,
+ * }}
+ */
+let ThemeBackgroundInfo;
+
+/** @type {?ThemeBackgroundInfo} */
 window.chrome.embeddedSearch.newTabPage.themeBackgroundInfo;
 
 /**

chrome/browser/resources/local_ntp/local_ntp.html

@@ -14,6 +14,8 @@
   $i18nRaw{bgPreloader}
   <meta http-equiv="Content-Security-Policy"
       content="$i18nRaw{contentSecurityPolicy}">
+  <script src="chrome-search://local-ntp/assert.js"
+      integrity="$i18n{assertIntegrity}"></script>
   <script src="chrome-search://local-ntp/animations.js"
       integrity="$i18n{animationsIntegrity}"></script>
   <script src="chrome-search://local-ntp/config.js"

chrome/browser/resources/local_ntp/local_ntp.js

@@ -50,13 +50,13 @@ function disableIframesAndVoiceSearchForTesting() {
  *   theme.
  *
  * @type {{
- *   backgroundColor: Array<number>,
- *   darkBackgroundColor: Array<number>,
- *   iconBackgroundColor: Array<number>,
- *   iconDarkBackgroundColor: Array<number>,
+ *   backgroundColor: !Array<number>,
+ *   darkBackgroundColor: !Array<number>,
+ *   iconBackgroundColor: !Array<number>,
+ *   iconDarkBackgroundColor: !Array<number>,
  *   numTitleLines: number,
- *   titleColor: Array<number>,
- *   titleColorAgainstDark: Array<number>,
+ *   titleColor: !Array<number>,
+ *   titleColorAgainstDark: !Array<number>,
  * }}
  */
 const NTP_DESIGN = {
@@ -276,6 +276,7 @@ function overrideExecutableTimeoutForTesting(timeout) {
  * the page has notheme set, returns a fallback light-colored theme (or dark-
  * colored theme if dark mode is enabled). This is used when the doodle is
  * displayed after clicking the notifier.
+ * @return {?ThemeBackgroundInfo}
  */
 function getThemeBackgroundInfo() {
   if (history.state && history.state.notheme) {
@@ -295,7 +296,6 @@ function getThemeBackgroundInfo() {
                                NTP_DESIGN.titleColor),
       useTitleContainer: false,
       useWhiteAddIcon: isDarkModeEnabled,
-      usingDarkMode: isDarkModeEnabled,
       usingDefaultTheme: true,
     };
   }
@@ -376,9 +376,10 @@ function renderTheme() {
 
   if (info.customBackgroundConfigured) {
     // Do anything only if the custom background changed.
-    if (!$(IDS.CUSTOM_BG).style.backgroundImage.includes(info.imageUrl)) {
+    const imageUrl = assert(info.imageUrl);
+    if (!$(IDS.CUSTOM_BG).style.backgroundImage.includes(imageUrl)) {
       const imageWithOverlay = [
-        customize.CUSTOM_BACKGROUND_OVERLAY, 'url(' + info.imageUrl + ')'
+        customize.CUSTOM_BACKGROUND_OVERLAY, 'url(' + imageUrl + ')'
       ].join(',').trim();
       // If the theme update is because of uploading a local image then we
       // should close the customization menu. Closing the menu before the image
@@ -396,11 +397,12 @@ function renderTheme() {
       image.onload = function() {
         $(IDS.CUSTOM_BG).style.opacity = '1';
       };
-      image.src = info.imageUrl;
+      image.src = imageUrl;
 
       customize.clearAttribution();
       customize.setAttribution(
-          info.attribution1, info.attribution2, info.attributionActionUrl);
+          '' + info.attribution1, '' + info.attribution2,
+          '' + info.attributionActionUrl);
     }
   } else {
     $(IDS.CUSTOM_BG).style.opacity = '0';
@@ -466,7 +468,8 @@ function renderOneGoogleBarTheme() {
     const oneGoogleBarPromise = oneGoogleBarApi.bf();
     oneGoogleBarPromise.then(function(oneGoogleBar) {
       const setForegroundStyle = oneGoogleBar.pc.bind(oneGoogleBar);
-      setForegroundStyle(getThemeBackgroundInfo().isNtpBackgroundDark ? 1 : 0);
+      const themeInfo = getThemeBackgroundInfo();
+      setForegroundStyle(themeInfo && themeInfo.isNtpBackgroundDark ? 1 : 0);
     });
   } catch (err) {
     console.log('Failed setting OneGoogleBar theme:\n' + err);
@@ -505,8 +508,8 @@ function setCustomThemeStyle(themeInfo) {
 
 /**
  * Renders the attribution if the URL is present, otherwise hides it.
- * @param {string} url The URL of the attribution image, if any.
- * @param {string} themeBackgroundAlignment The alignment of the theme
+ * @param {string|undefined} url The URL of the attribution image, if any.
+ * @param {string|undefined} themeBackgroundAlignment The alignment of the theme
  *     background image. This is used to compute the attribution's alignment.
  */
 function updateThemeAttribution(url, themeBackgroundAlignment) {

chrome/browser/resources/local_ntp/most_visited_util.js

@@ -175,7 +175,7 @@ function getTextColor(params, isTitle) {
   // For backward compatibility with server-side NTP, look at themes directly
   // and use param.c for non-title or as fallback.
   const apiHandle = chrome.embeddedSearch.newTabPage;
-  const themeInfo = apiHandle.themeBackgroundInfo;
+  const themeInfo = assert(apiHandle.themeBackgroundInfo);
   let c = '#777';
   if (isTitle && themeInfo && !themeInfo.usingDefaultTheme) {
     // Read from theme directly

chrome/browser/search/BUILD.gn

@@ -8,6 +8,7 @@ action("local_ntp_code_generate") {
   script = "tools/generate_integrity_header.py"
   header_path = "$target_gen_dir/local_ntp_js_integrity.h"
   animations_js = local_ntp_resources + "/animations.js"
+  assert_js = "//ui/webui/resources/js/assert.js"
   customize_js = local_ntp_resources + "/customize.js"
   doodles_js = local_ntp_resources + "/doodles.js"
   local_ntp_js = local_ntp_resources + "/local_ntp.js"
@@ -16,6 +17,7 @@ action("local_ntp_code_generate") {
 
   inputs = [
     animations_js,
+    assert_js,
     customize_js,
     doodles_js,
     local_ntp_js,
@@ -30,6 +32,7 @@ action("local_ntp_code_generate") {
   args = [
     "--output_path=" + rebase_path(header_path, root_build_dir),
     rebase_path(animations_js, root_build_dir),
+    rebase_path(assert_js, root_build_dir),
     rebase_path(customize_js, root_build_dir),
     rebase_path(doodles_js, root_build_dir),
     rebase_path(local_ntp_js, root_build_dir),

chrome/browser/search/local_ntp_source.cc

@@ -76,6 +76,7 @@
 #include "ui/base/template_expressions.h"
 #include "ui/base/webui/web_ui_util.h"
 #include "ui/resources/grit/ui_resources.h"
+#include "ui/resources/grit/webui_resources.h"
 #include "url/gurl.h"
 
 using search_provider_logos::EncodedLogo;
@@ -113,6 +114,7 @@ const struct Resource{
 } kResources[] = {
     {"animations.css", IDR_LOCAL_NTP_ANIMATIONS_CSS, "text/css"},
     {"animations.js", IDR_LOCAL_NTP_ANIMATIONS_JS, "application/javascript"},
+    {"assert.js", IDR_WEBUI_JS_ASSERT, "application/javascript"},
     {"local-ntp-common.css", IDR_LOCAL_NTP_COMMON_CSS, "text/css"},
     {"customize.css", IDR_LOCAL_NTP_CUSTOMIZE_CSS, "text/css"},
     {"customize.js", IDR_LOCAL_NTP_CUSTOMIZE_JS, "application/javascript"},
@@ -958,6 +960,8 @@ void LocalNtpSource::StartDataRequest(
 
     replacements["animationsIntegrity"] =
         base::StrCat({kSha256, ANIMATIONS_JS_INTEGRITY});
+    replacements["assertIntegrity"] =
+        base::StrCat({kSha256, ASSERT_JS_INTEGRITY});
     replacements["configDataIntegrity"] = base::StrCat(
         {kSha256, search_config_provider_->config_data_integrity()});
     replacements["localNtpCustomizeIntegrity"] =
@@ -1098,9 +1102,10 @@ std::string LocalNtpSource::GetContentSecurityPolicy() {
   // 'strict-dynamic' allows those scripts to load dependencies not listed here.
   std::string script_src_csp = base::StringPrintf(
       "script-src 'strict-dynamic' 'sha256-%s' 'sha256-%s' 'sha256-%s' "
-      "'sha256-%s' 'sha256-%s' 'sha256-%s' 'sha256-%s';",
-      ANIMATIONS_JS_INTEGRITY, CUSTOMIZE_JS_INTEGRITY, DOODLES_JS_INTEGRITY,
-      LOCAL_NTP_JS_INTEGRITY, UTILS_JS_INTEGRITY, VOICE_JS_INTEGRITY,
+      "'sha256-%s' 'sha256-%s' 'sha256-%s' 'sha256-%s' 'sha256-%s';",
+      ANIMATIONS_JS_INTEGRITY, ASSERT_JS_INTEGRITY, CUSTOMIZE_JS_INTEGRITY,
+      DOODLES_JS_INTEGRITY, LOCAL_NTP_JS_INTEGRITY, UTILS_JS_INTEGRITY,
+      VOICE_JS_INTEGRITY,
       search_config_provider_->config_data_integrity().c_str());
 
   return GetContentSecurityPolicyObjectSrc() +

chrome/common/search/instant_types.cc

@@ -6,7 +6,10 @@
 
 ThemeBackgroundInfo::ThemeBackgroundInfo() = default;
 
-ThemeBackgroundInfo::~ThemeBackgroundInfo() {}
+ThemeBackgroundInfo::ThemeBackgroundInfo(const ThemeBackgroundInfo& other) =
+    default;
+
+ThemeBackgroundInfo::~ThemeBackgroundInfo() = default;
 
 bool ThemeBackgroundInfo::operator==(const ThemeBackgroundInfo& rhs) const {
   return using_default_theme == rhs.using_default_theme &&

chrome/common/search/instant_types.h

@@ -47,6 +47,7 @@ enum ThemeBackgroundImageTiling {
 // Theme background settings for the NTP.
 struct ThemeBackgroundInfo {
   ThemeBackgroundInfo();
+  ThemeBackgroundInfo(const ThemeBackgroundInfo& other);
   ~ThemeBackgroundInfo();
 
   bool operator==(const ThemeBackgroundInfo& rhs) const;

chrome/renderer/searchbox/searchbox.cc

@@ -11,6 +11,7 @@
 
 #include "base/bind.h"
 #include "base/logging.h"
+#include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
@@ -273,8 +274,8 @@ bool SearchBox::GetMostVisitedItemWithID(
                                                            item);
 }
 
-const ThemeBackgroundInfo& SearchBox::GetThemeBackgroundInfo() const {
-  return theme_info_;
+const ThemeBackgroundInfo* SearchBox::GetThemeBackgroundInfo() const {
+  return base::OptionalOrNullptr(theme_info_);
 }
 
 void SearchBox::Paste(const base::string16& text) {

chrome/renderer/searchbox/searchbox.h

@@ -10,6 +10,7 @@
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "base/optional.h"
 #include "base/strings/string16.h"
 #include "chrome/common/search.mojom.h"
 #include "chrome/common/search/instant_types.h"
@@ -93,7 +94,8 @@ class SearchBox : public content::RenderFrameObserver,
   // Sends PasteAndOpenDropdown to the browser.
   void Paste(const base::string16& text);
 
-  const ThemeBackgroundInfo& GetThemeBackgroundInfo() const;
+  // Will return null if the theme info hasn't been set yet.
+  const ThemeBackgroundInfo* GetThemeBackgroundInfo() const;
 
   // Sends FocusOmnibox(OMNIBOX_FOCUS_INVISIBLE) to the browser.
   void StartCapturingKeyStrokes();
@@ -240,7 +242,7 @@ class SearchBox : public content::RenderFrameObserver,
   // comparing most visited items.
   InstantMostVisitedInfo most_visited_info_;
   bool has_received_most_visited_;
-  ThemeBackgroundInfo theme_info_;
+  base::Optional<ThemeBackgroundInfo> theme_info_;
 
   base::WeakPtrFactory<SearchBox> weak_ptr_factory_{this};
 

chrome/renderer/searchbox/searchbox_extension.cc

@@ -895,8 +895,10 @@ v8::Local<v8::Value> NewTabPageBindings::GetThemeBackgroundInfo(
   const SearchBox* search_box = GetSearchBoxForCurrentContext();
   if (!search_box)
     return v8::Null(isolate);
-  const ThemeBackgroundInfo& theme_info = search_box->GetThemeBackgroundInfo();
-  return GenerateThemeBackgroundInfo(isolate, theme_info);
+  const ThemeBackgroundInfo* theme_info = search_box->GetThemeBackgroundInfo();
+  if (!theme_info)
+    return v8::Null(isolate);
+  return GenerateThemeBackgroundInfo(isolate, *theme_info);
 }
 
 // static

chrome/test/data/local_ntp/local_ntp_browsertest.html

@@ -12,6 +12,7 @@
   <link rel="stylesheet" href="chrome-search://local-ntp/local-ntp.css"></link>
   <link rel="stylesheet" href="chrome-search://local-ntp/voice.css"></link>
   <script src="chrome-search://local-ntp/animations.js" charset="utf-8"></script>
+  <script src="chrome-search://local-ntp/assert.js" charset="utf-8"></script>
   <script src="chrome-search://local-ntp/config.js" charset="utf-8"></script>
   <script src="chrome-search://local-ntp/customize.js" charset="utf-8"></script>
   <script src="chrome-search://local-ntp/doodles.js" charset="utf-8"></script>

chrome/test/data/local_ntp/local_ntp_browsertest.js

@@ -64,21 +64,21 @@ test.localNtp.testDoesNotShowFakeboxIfNotGoogle = function() {
 test.localNtp.testMostVisitedContents = function() {
   // Check that the API is available and properly hooked up, so that it returns
   // some data (see history::PrepopulatedPageList for the default contents).
-  assert(window.chrome.embeddedSearch.newTabPage.mostVisited.length > 0);
+  assertTrue(window.chrome.embeddedSearch.newTabPage.mostVisited.length > 0);
 
   // Check that the items have the required fields: We expect a "restricted ID"
   // (rid), but there mustn't be url, title, etc. Those are only available
   // through getMostVisitedItemData(rid).
   for (var mvItem of window.chrome.embeddedSearch.newTabPage.mostVisited) {
     assertTrue(isFinite(mvItem.rid));
-    assert(!mvItem.url);
-    assert(!mvItem.title);
-    assert(!mvItem.domain);
+    assertTrue(!mvItem.url);
+    assertTrue(!mvItem.title);
+    assertTrue(!mvItem.domain);
   }
 
   // Try to get an item's details via getMostVisitedItemData. This should fail,
   // because that API is only available to the MV iframe.
-  assert(!window.chrome.embeddedSearch.newTabPage.getMostVisitedItemData(
+  assertTrue(!window.chrome.embeddedSearch.newTabPage.getMostVisitedItemData(
       window.chrome.embeddedSearch.newTabPage.mostVisited[0].rid));
 };
 

chrome/test/data/local_ntp/test_utils.js

@@ -34,17 +34,6 @@ function setUpPage(templateId) {
   document.body.appendChild($(templateId).content.cloneNode(true));
 }
 
-/**
- * Aborts a test if a condition is not met.
- * @param {boolean} condition The condition that must be true.
- * @param {string=} opt_message A message to log if the condition is not met.
- */
-function assert(condition, opt_message) {
-  if (!condition) {
-    throw new Error(opt_message || 'Assertion failed');
-  }
-}
-
 /**
  * Aborts a test if |got| !== true.
  * @param {boolean} got The boolean that must be true.
@@ -271,7 +260,7 @@ function MockClock() {
  * @param {!number} msec The time in milliseconds.
  */
 MockClock.prototype.setTime = function(msec) {
-  assert(this.time <= msec);
+  assertTrue(this.time <= msec);
   this.time = msec;
 };
 
@@ -280,7 +269,7 @@ MockClock.prototype.setTime = function(msec) {
  * @param {!number} msec The length of time to advance the current time by.
  */
 MockClock.prototype.advanceTime = function(msec) {
-  assert(msec >= 0);
+  assertTrue(msec >= 0);
   this.time += msec;
 };
 

chrome/test/data/local_ntp/voice_speech_browsertest.js

@@ -181,7 +181,7 @@ test.speech.testSpeechRecognitionInitSettings = function() {
   assertFalse(speech.recognition_.continuous);
   assertEquals('en-ZA', speech.recognition_.lang);
   assertEquals(1, speech.recognition_.maxAlternatives);
-  assert(!!speech.recognition_);
+  assertTrue(!!speech.recognition_);
   test.speech.validateInactive();
 };
 
@@ -269,12 +269,12 @@ test.speech.testClickHandlingWithUnitializedSpeechRecognition = function() {
   test.speech.initSpeech();
   speech.recognition_ = undefined;
   assertEquals(speech.State_.READY, speech.currentState_);
-  assert(!speech.recognition_);
+  assertTrue(!speech.recognition_);
   speech.start();
 
   assertEquals(1, test.speech.recognitionActiveCount);
   assertEquals(1, test.speech.viewActiveCount);
-  assert(!!speech.recognition_);
+  assertTrue(!!speech.recognition_);
 };
 
 /**
@@ -350,7 +350,7 @@ test.speech.testHandleFinalSpeechResponse = function() {
   speech.recognition_.onresult(responseEvent);
 
   test.speech.validateInactive();
-  assert(!!test.speech.locationUrl);
+  assertTrue(!!test.speech.locationUrl);
   assertEquals(
       test.speech.TEST_BASE_URL + 'search?q=high&gs_ivs=1',
       test.speech.locationUrl.href);
@@ -585,7 +585,7 @@ test.speech.testEnterToSubmit = function() {
   speech.onKeyDown(keyEvent);
 
   test.speech.validateInactive();
-  assert(!!test.speech.locationUrl);
+  assertTrue(!!test.speech.locationUrl);
   assertEquals(
       test.speech.TEST_BASE_URL + 'search?q=test+query&gs_ivs=1',
       test.speech.locationUrl.href);
@@ -607,7 +607,7 @@ test.speech.testClickToSubmit = function() {
       /*submitQuery=*/true, /*shouldRetry=*/false, /*navigatingAway=*/false);
 
   test.speech.validateInactive();
-  assert(!!test.speech.locationUrl);
+  assertTrue(!!test.speech.locationUrl);
   assertEquals(
       test.speech.TEST_BASE_URL + 'search?q=test+query&gs_ivs=1',
       test.speech.locationUrl.href);
@@ -891,7 +891,7 @@ test.speech.testIdleTimeoutWithConfidentSpeechResults = function() {
   test.speech.clock.pendingTimeouts.shift().callback();
 
   test.speech.validateInactive();
-  assert(!!test.speech.locationUrl);
+  assertTrue(!!test.speech.locationUrl);
   assertEquals(
       test.speech.TEST_BASE_URL + 'search?q=high&gs_ivs=1',
       test.speech.locationUrl.href);
@@ -934,7 +934,7 @@ test.speech.testIdleTimeoutWithNonConfidentSpeechResults = function() {
   test.speech.clock.advanceTime(3000);
   test.speech.clock.pendingTimeouts.shift().callback();
   test.speech.validateInactive();
-  assert(
+  assertTrue(
       !test.speech.locationUrl ||
       !test.speech.locationUrl.href.startsWith(test.speech.TEST_BASE_URL));
 };
@@ -950,7 +950,7 @@ test.speech.testQueryEncoding = function() {
   speech.finalResult_ = '🔍t&qôr 文字+weird*chär%?s?';
   speech.submitFinalResult_();
 
-  assert(!!test.speech.locationUrl);
+  assertTrue(!!test.speech.locationUrl);
   // To encode query: encodeURIComponent(queryText).replace(/%20/g, '+')
   assertEquals(
       test.speech.TEST_BASE_URL + 'search?q=%F0%9F%94%8Dt%26q%C3%B4r+' +

chrome/test/data/local_ntp/voice_view_browsertest.js

@@ -358,7 +358,7 @@ test.view.testClickMicButtonWithResults = function() {
   assertFalse(test.view.state.shouldRetry);
   assertTrue(test.view.state.shouldSubmit);
   assertFalse(test.view.state.navigatingAway);
-  assert(!test.view.state.lastEvent);
+  assertTrue(!test.view.state.lastEvent);
 };
 
 /**