ambient: Change Settings topic source picker behaviors

The new  behaviors are:
1. When going to ambient mode page, keep user previous selection or
auto-select Art if no album or selected album is deleted.
2. Click Art row will always select Art row.
3. Click any place of `Google Photos' (GP) row, will not change the
selection status yet, but goes to the GP album-selection page. When user
makes any change on GP album-selection page, we will either select GP
row, or fall back to select Art row based on if any albums is available
or un/selected.
4. Any actives on the album-selection page without first going to the
ambient mode page, will trigger same behaviors as 2 & 3.
5. Click the topic source row and right arrow in that row will behave
the same.

Bug: b/161928422
Test: modified handler and js tests
Change-Id: Id5b33ca9a7cd712c899091796cbab461e9970141
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2364167
Commit-Queue: Tao Wu <wutao@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Jimmy Gong <jimmyxgong@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803781}

chrome/browser/resources/settings/chromeos/ambient_mode_page/ambient_mode_browser_proxy.js

@@ -36,12 +36,6 @@ cr.define('settings', function() {
      */
     setSelectedTemperatureUnit(temperatureUnit) {}
 
-    /**
-     * Updates the selected topic source to server.
-     * @param {!AmbientModeTopicSource} topicSource the selected topic source.
-     */
-    setSelectedTopicSource(topicSource) {}
-
     /**
      * Updates the selected albums of Google Photos or art categories to server.
      * @param {!AmbientModeSettings} settings the selected albums or categeries.
@@ -66,11 +60,6 @@ cr.define('settings', function() {
       chrome.send('setSelectedTemperatureUnit', [temperatureUnit]);
     }
 
-    /** @override */
-    setSelectedTopicSource(topicSource) {
-      chrome.send('setSelectedTopicSource', [topicSource]);
-    }
-
     /** @override */
     setSelectedAlbums(settings) {
       chrome.send('setSelectedAlbums', [settings]);

chrome/browser/resources/settings/chromeos/ambient_mode_page/ambient_mode_page.html

@@ -69,7 +69,8 @@
     <template is="dom-if" if="[[prefs.settings.ambient_mode.enabled.value]]">
       <div id="topicSourceListDiv" class="layout vertical flex">
         <topic-source-list topic-sources="[[topicSources_]]"
-            selected-item="[[selectedTopicSource_]]"
+            selected-topic-source="[[selectedTopicSource_]]"
+            has-google-photos-albums="[[hasGooglePhotosAlbums_]]"
             disabled="[[!isValidTopicSource_(selectedTopicSource_)]]">
         </topic-source-list>
       </div>

chrome/browser/resources/settings/chromeos/ambient_mode_page/ambient_mode_page.js

@@ -51,6 +51,9 @@ Polymer({
       value: AmbientModeTopicSource.UNKNOWN,
     },
 
+    /** @private */
+    hasGooglePhotosAlbums_: Boolean,
+
     /** @private {!AmbientModeTemperatureUnit} */
     selectedTemperatureUnit_: {
       type: AmbientModeTemperatureUnit,
@@ -72,7 +75,6 @@ Polymer({
   },
 
   listeners: {
-    'selected-topic-source-changed': 'onSelectedTopicSourceChanged_',
     'show-albums': 'onShowAlbums_',
   },
 
@@ -88,8 +90,9 @@ Polymer({
   ready() {
     this.addWebUIListener(
         'topic-source-changed',
-        (/** @type {!AmbientModeTopicSource} */ topicSource) => {
-          this.selectedTopicSource_ = topicSource;
+        (/** @type {!TopicSourceItem} */ topicSourceItem) => {
+          this.selectedTopicSource_ = topicSourceItem.topicSource;
+          this.hasGooglePhotosAlbums_ = topicSourceItem.hasGooglePhotosAlbums;
         },
     );
     this.addWebUIListener(
@@ -181,15 +184,6 @@ Polymer({
     }
   },
 
-  /**
-   * @param {!CustomEvent<{item: !AmbientModeTopicSource}>} event
-   * @private
-   */
-  onSelectedTopicSourceChanged_(event) {
-    this.browserProxy_.setSelectedTopicSource(
-        /** @type {!AmbientModeTopicSource} */ (event.detail));
-  },
-
   /**
    * Open ambientMode/photos subpage.
    * @param {!CustomEvent<{item: !AmbientModeTopicSource}>} event

chrome/browser/resources/settings/chromeos/ambient_mode_page/ambient_mode_photos_page.html

@@ -18,12 +18,19 @@
     <style include="settings-shared">
     </style>
     <settings-localized-link class="cr-row first"
-        localized-string="[[getTitleInnerHtml_(topicSource_)]]">
+        localized-string="[[getTitleInnerHtml_(topicSource)]]">
     </settings-localized-link>
 
+    <template is="dom-if" if="[[hasNoAlbums_(albums)]]">
+      <settings-localized-link class="cr-row first"
+          localized-string=
+              $i18nPolymer{ambientModeAlbumsSubpageGooglePhotosNoAlbum}
+      </settings-localized-link>
+    </template>
+
     <!-- Text based album selection. -->
     <template is="dom-if" if="[[!photoPreviewEnabled]]">
-      <iron-list id="albums" class="list-frame" items="[[albums_]]">
+      <iron-list id="albums" class="list-frame" items="[[albums]]">
         <template>
           <cr-checkbox class="list-item"
               checked="[[item.checked]]"
@@ -38,7 +45,7 @@
 
     <!-- Photo based album selection. -->
     <template is="dom-if" if="[[photoPreviewEnabled]]">
-      <album-list albums="{{albums_}}" topic-source="[[topicSource_]]">
+      <album-list albums="{{albums}}" topic-source="[[topicSource]]">
       </album-list>
     </template>
   </template>

chrome/browser/resources/settings/chromeos/ambient_mode_page/ambient_mode_photos_page.js

@@ -13,7 +13,6 @@ Polymer({
       [I18nBehavior, settings.RouteObserverBehavior, WebUIListenerBehavior],
 
   properties: {
-    /** @private */
     photoPreviewEnabled: {
       type: Boolean,
       value() {
@@ -22,16 +21,14 @@ Polymer({
       readOnly: true,
     },
 
-    /** @private {!AmbientModeTopicSource} */
-    topicSource_: {
+    /** @type {!AmbientModeTopicSource} */
+    topicSource: {
       type: Number,
-      value() {
-        return AmbientModeTopicSource.UNKNOWN;
-      },
+      value: AmbientModeTopicSource.UNKNOWN,
     },
 
-    /** @private {Array<!AmbientModeAlbum>} */
-    albums_: {
+    /** @type {?Array<!AmbientModeAlbum>} */
+    albums: {
       type: Array,
       notify: true,
       // Set to null to differentiate from an empty album.
@@ -77,11 +74,11 @@ Polymer({
       return;
     }
 
-    this.topicSource_ = /** @type {!AmbientModeTopicSource} */ (topicSourceInt);
-    if (this.topicSource_ === AmbientModeTopicSource.GOOGLE_PHOTOS) {
+    this.topicSource = /** @type {!AmbientModeTopicSource} */ (topicSourceInt);
+    if (this.topicSource === AmbientModeTopicSource.GOOGLE_PHOTOS) {
       this.parentNode.pageTitle =
           this.i18n('ambientModeTopicSourceGooglePhotos');
-    } else if (this.topicSource_ === AmbientModeTopicSource.ART_GALLERY) {
+    } else if (this.topicSource === AmbientModeTopicSource.ART_GALLERY) {
       this.parentNode.pageTitle = this.i18n('ambientModeTopicSourceArtGallery');
     } else {
       assertNotReached();
@@ -90,8 +87,8 @@ Polymer({
 
     // TODO(b/162793904): Have a better plan to cache the UI data.
     // Reset to null to distinguish empty albums fetched from server.
-    this.albums_ = null;
-    this.browserProxy_.requestAlbums(this.topicSource_);
+    this.albums = null;
+    this.browserProxy_.requestAlbums(this.topicSource);
   },
 
   /**
@@ -101,10 +98,10 @@ Polymer({
   onAlbumsChanged_(settings) {
     // This page has been reused by other topic source since the last time
     // requesting the albums. Do not update on this stale event.
-    if (settings.topicSource !== this.topicSource_) {
+    if (settings.topicSource !== this.topicSource) {
       return;
     }
-    this.albums_ = settings.albums;
+    this.albums = settings.albums;
   },
 
   /**
@@ -112,13 +109,13 @@ Polymer({
    * @private
    */
   onAlbumPreviewChanged_(album) {
-    if (album.topicSource !== this.topicSource_) {
+    if (album.topicSource !== this.topicSource) {
       return;
     }
 
-    for (let i = 0; i < this.albums_.length; ++i) {
-      if (this.albums_[i].albumId === album.albumId) {
-        this.set('albums_.' + i + '.url', album.url);
+    for (let i = 0; i < this.albums.length; ++i) {
+      if (this.albums[i].albumId === album.albumId) {
+        this.set('albums.' + i + '.url', album.url);
       }
     }
   },
@@ -142,13 +139,14 @@ Polymer({
    */
   onSelectedAlbumsChanged_(event) {
     const albums = [];
-    this.albums_.forEach((/** @param {AmbientModeAlbum} album */ (album) => {
+    this.albums.forEach(/** @param {AmbientModeAlbum} album */ (album) => {
       if (album.checked) {
         albums.push({albumId: album.albumId});
       }
-    }));
+    });
+
     this.browserProxy_.setSelectedAlbums(
-        {topicSource: this.topicSource_, albums: albums});
+        {topicSource: this.topicSource, albums: albums});
   },
 
   /** @private */
@@ -161,7 +159,14 @@ Polymer({
       }
     });
     this.browserProxy_.setSelectedAlbums(
-        {topicSource: this.topicSource_, albums: albums});
-  }
+        {topicSource: this.topicSource, albums: albums});
+  },
 
+  /**
+   * @return {boolean}
+   * @private
+   */
+  hasNoAlbums_() {
+    return !!this.albums && !this.albums.length;
+  },
 });

chrome/browser/resources/settings/chromeos/ambient_mode_page/constants.js

@@ -20,6 +20,16 @@
   CELSIUS: 'celsius',
 };
 
+/**
+ * Item of AmbientModeTopicSource.
+ *
+ * @typedef {{
+ *   topicSource: AmbientModeTopicSource,
+ *   hasGooglePhotosAlbums: boolean,
+ * }}
+ */
+/* #export */ let TopicSourceItem;
+
 /**
  * Album metadata for UI.
  *

chrome/browser/resources/settings/chromeos/ambient_mode_page/topic_source_item.html

@@ -37,7 +37,7 @@
       <div id="labelWrapper" aria-hidden="true">
         <div>[[getItemName_(item)]]</div>
         <div class="cr-secondary-text">
-          [[getItemDescription_(item)]]
+          [[getItemDescription_(item, hasGooglePhotosAlbums)]]
         </div>
       </div>
 

chrome/browser/resources/settings/chromeos/ambient_mode_page/topic_source_item.js

@@ -23,6 +23,11 @@ Polymer({
       reflectToAttribute: true,
     },
 
+    hasGooglePhotosAlbums: {
+      type: Boolean,
+      value: false,
+    },
+
     /** @type {!AmbientModeTopicSource} */
     item: Object,
 
@@ -69,7 +74,12 @@ Polymer({
    */
   getItemDescription_() {
     if (this.item === AmbientModeTopicSource.GOOGLE_PHOTOS) {
-      return this.i18n('ambientModeTopicSourceGooglePhotosDescription');
+      if (this.hasGooglePhotosAlbums) {
+        return this.i18n('ambientModeTopicSourceGooglePhotosDescription');
+      } else {
+        return this.i18n(
+            'ambientModeTopicSourceGooglePhotosDescriptionNoAlbum');
+      }
     } else if (this.item === AmbientModeTopicSource.ART_GALLERY) {
       return this.i18n('ambientModeTopicSourceArtGalleryDescription');
     } else {
@@ -107,19 +117,11 @@ Polymer({
    */
   onKeydown_(event) {
     // The only key event handled by this element is pressing Enter.
-    // (1) Pressing the subpage arrow leads to the subpage.
-    // (2) Pressing anywhere on an already-selected item leads to the subpage.
+    // Pressing anywhere leads to the subpage.
     if (event.key !== 'Enter') {
       return;
     }
 
-    // If the item is not checked, it will be handled by
-    // onSelectedItemChanged_() in topic_source_list.js.
-    if (!this.checked &&
-        this.$$('#subpage-button') !== this.shadowRoot.activeElement) {
-      return;
-    }
-
     this.fireShowAlbums_();
     event.preventDefault();
     event.stopPropagation();
@@ -130,11 +132,9 @@ Polymer({
    * @private
    */
   onItemClick_(event) {
-    // Clicking anywhere on an already-selected item leads to the subpage.
-    if (this.checked) {
-      this.fireShowAlbums_();
-      event.stopPropagation();
-    }
+    // Clicking anywhere leads to the subpage.
+    this.fireShowAlbums_();
+    event.stopPropagation();
   },
 
   /**

chrome/browser/resources/settings/chromeos/ambient_mode_page/topic_source_list.html

@@ -29,18 +29,13 @@
         background-color: var(--cr-focused-item-color);
       }
     </style>
-    <h2 id="topicSourceTitle">
-      $i18n{ambientModeTopicSourceTitle}
-    </h2>
+    <h2 id="topicSourceTitle">$i18n{ambientModeTopicSourceTitle}</h2>
 
-    <iron-list id="topicSourceList" selection-enabled
-        items="[[topicSources]]"
-        selected-item="{{selectedItem}}"
-        on-selected-item-changed="onSelectedItemChanged_">
+    <iron-list id="topicSourceList" items="[[topicSources]]">
       <template>
-        <topic-source-item item="[[item]]"
-            checked="[[isItemSelected_(item, selectedItem)]]"
-            tabindex$="[[tabIndex]]"
+        <topic-source-item item="[[item]]" tabindex$="[[tabIndex]]"
+            has-google-photos-albums="[[hasGooglePhotosAlbums]]"
+            checked="[[isSelected_(item, selectedTopicSource)]]"
             disabled$="[[disabled]]">
         </topic-source-item>
       </template>

chrome/browser/resources/settings/chromeos/ambient_mode_page/topic_source_list.js

@@ -16,48 +16,27 @@ Polymer({
   properties: {
     /**
      * Contains topic sources.
-     * @private {!Array<!AmbientModeTopicSource>}
+     * @type {!Array<!AmbientModeTopicSource>}
      */
     topicSources: {
       type: Array,
       value: [],
     },
 
-    /**
-     * Reflects the iron-list selectedItem property.
-     * @type {AmbientModeTopicSource}
-     */
-    selectedItem: Object,
-
-    /**
-     * The last iron-list selectedItem.
-     * @type {AmbientModeTopicSource}
-     */
-    lastSelectedItem_: Object,
-  },
-
-  /** @private */
-  onSelectedItemChanged_() {
-    // <iron-list> causes |this.$.topicSourceList.selectedItem| to be null if
-    // tapped a second time.
-    if (this.$.topicSourceList.selectedItem === null) {
-      // In the case that the user deselects an item, reselect the item manually
-      // by altering the list.
-      this.selectedItem_ = this.lastSelectedItem_;
-      return;
-    }
+    /** @type {!AmbientModeTopicSource} */
+    selectedTopicSource: {
+      type: AmbientModeTopicSource,
+      value: AmbientModeTopicSource.UNKNOWN,
+    },
 
-    if (this.lastSelectedItem_ !== this.$.topicSourceList.selectedItem) {
-      this.lastSelectedItem_ = this.$.topicSourceList.selectedItem;
-      this.fire('selected-topic-source-changed', this.lastSelectedItem_);
-    }
+    hasGooglePhotosAlbums: Boolean,
   },
 
   /**
-   * @param {!AmbientModeTopicSource} item
+   * @param {!AmbientModeTopicSource} topic_source
    * @private
    */
-  isItemSelected_(item) {
-    return this.selectedItem === item;
+  isSelected_(topic_source) {
+    return this.selectedTopicSource === topic_source;
   },
 });

chrome/browser/resources/settings/chromeos/os_settings.gni

@@ -87,7 +87,7 @@ os_settings_auto_imports = settings_auto_imports +
                            cr_components_chromeos_auto_imports +
                            cr_elements_chromeos_auto_imports + [
                              "chrome/browser/resources/settings/chromeos/ambient_mode_page/ambient_mode_browser_proxy.html|AmbientModeBrowserProxy,AmbientModeBrowserProxyImpl",
-                             "chrome/browser/resources/settings/chromeos/ambient_mode_page/constants.html|AmbientModeTopicSource,AmbientModeTemperatureUnit,AmbientModeAlbum,AmbientModeSettings",
+                             "chrome/browser/resources/settings/chromeos/ambient_mode_page/constants.html|AmbientModeTopicSource,AmbientModeTemperatureUnit,AmbientModeAlbum,AmbientModeSettings,TopicSourceItem",
                              "chrome/browser/resources/settings/chromeos/deep_linking_behavior.html|DeepLinkingBehavior",
                              "chrome/browser/resources/settings/chromeos/metrics_recorder.html|recordSettingChange",
                              "chrome/browser/resources/settings/chromeos/multidevice_page/multidevice_browser_proxy.html|MultiDeviceBrowserProxy,MultiDeviceBrowserProxyImpl",

chrome/browser/ui/webui/settings/chromeos/ambient_mode_handler.cc

@@ -123,11 +123,6 @@ void AmbientModeHandler::RegisterMessages() {
       base::BindRepeating(&AmbientModeHandler::HandleRequestAlbums,
                           base::Unretained(this)));
 
-  web_ui()->RegisterMessageCallback(
-      "setSelectedTopicSource",
-      base::BindRepeating(&AmbientModeHandler::HandleSetSelectedTopicSource,
-                          base::Unretained(this)));
-
   web_ui()->RegisterMessageCallback(
       "setSelectedTemperatureUnit",
       base::BindRepeating(&AmbientModeHandler::HandleSetSelectedTemperatureUnit,
@@ -184,15 +179,6 @@ void AmbientModeHandler::HandleSetSelectedTemperatureUnit(
   UpdateSettings();
 }
 
-void AmbientModeHandler::HandleSetSelectedTopicSource(
-    const base::ListValue* args) {
-  DCHECK(settings_);
-  CHECK_EQ(1U, args->GetSize());
-
-  settings_->topic_source = ExtractTopicSource(args);
-  UpdateSettings();
-}
-
 void AmbientModeHandler::HandleSetSelectedAlbums(const base::ListValue* args) {
   const base::DictionaryValue* dictionary = nullptr;
   CHECK(!args->GetList().empty());
@@ -217,6 +203,12 @@ void AmbientModeHandler::HandleSetSelectedAlbums(const base::ListValue* args) {
         DCHECK(personal_album);
         settings_->selected_album_ids.emplace_back(personal_album->album_id);
       }
+
+      // Update topic source based on selections.
+      if (settings_->selected_album_ids.empty())
+        settings_->topic_source = ash::AmbientModeTopicSource::kArtGallery;
+      else
+        settings_->topic_source = ash::AmbientModeTopicSource::kGooglePhotos;
       break;
     case ash::AmbientModeTopicSource::kArtGallery:
       // For Art gallery, we set the corresponding setting to be enabled or not
@@ -235,6 +227,8 @@ void AmbientModeHandler::HandleSetSelectedAlbums(const base::ListValue* args) {
   }
 
   UpdateSettings();
+  // TODO(wutao): Undate the UI when success in OnUpdateSettings.
+  SendTopicSource();
 }
 
 void AmbientModeHandler::SendTemperatureUnit() {
@@ -246,8 +240,13 @@ void AmbientModeHandler::SendTemperatureUnit() {
 
 void AmbientModeHandler::SendTopicSource() {
   DCHECK(settings_);
+  base::Value topic_source(base::Value::Type::DICTIONARY);
+  topic_source.SetKey("hasGooglePhotosAlbums",
+                      base::Value(!personal_albums_.albums.empty()));
+  topic_source.SetKey("topicSource",
+                      base::Value(static_cast<int>(settings_->topic_source)));
   FireWebUIListener("topic-source-changed",
-                    base::Value(static_cast<int>(settings_->topic_source)));
+                    base::Value(std::move(topic_source)));
 }
 
 void AmbientModeHandler::SendAlbums(ash::AmbientModeTopicSource topic_source) {
@@ -281,6 +280,8 @@ void AmbientModeHandler::SendAlbums(ash::AmbientModeTopicSource topic_source) {
   }
 
   dictionary.SetKey("topicSource", base::Value(static_cast<int>(topic_source)));
+  dictionary.SetKey("selectedTopicSource",
+                    base::Value(static_cast<int>(settings_->topic_source)));
   dictionary.SetKey("albums", std::move(albums));
   FireWebUIListener("albums-changed", std::move(dictionary));
 }
@@ -342,6 +343,7 @@ void AmbientModeHandler::OnSettingsAndAlbumsFetched(
   if (chromeos::features::IsAmbientModePhotoPreviewEnabled())
     DownloadAlbumPreviewImage(*topic_source);
 
+  UpdateTopicSource(*topic_source);
   SendAlbums(*topic_source);
 }
 
@@ -358,6 +360,40 @@ void AmbientModeHandler::SyncSettingsAndAlbums() {
       it = settings_->selected_album_ids.erase(it);
     }
   }
+
+  if (settings_->selected_album_ids.empty())
+    MaybeUpdateTopicSource(ash::AmbientModeTopicSource::kArtGallery);
+}
+
+void AmbientModeHandler::UpdateTopicSource(
+    ash::AmbientModeTopicSource topic_source) {
+  // If this is an Art gallery album page, will select art gallery topic source.
+  if (topic_source == ash::AmbientModeTopicSource::kArtGallery) {
+    MaybeUpdateTopicSource(topic_source);
+    return;
+  }
+
+  // If this is a Google Photos album page, will
+  // 1. Select art gallery topic source if no albums or no album is selected.
+  if (settings_->selected_album_ids.empty()) {
+    MaybeUpdateTopicSource(ash::AmbientModeTopicSource::kArtGallery);
+    return;
+  }
+
+  // 2. Select Google Photos topic source if at least one album is selected.
+  MaybeUpdateTopicSource(ash::AmbientModeTopicSource::kGooglePhotos);
+}
+
+void AmbientModeHandler::MaybeUpdateTopicSource(
+    ash::AmbientModeTopicSource topic_source) {
+  // If the setting is the same, no need to update.
+  if (settings_->topic_source == topic_source)
+    return;
+
+  settings_->topic_source = topic_source;
+  UpdateSettings();
+  // TODO(wutao): Undate the UI when success in OnUpdateSettings.
+  SendTopicSource();
 }
 
 void AmbientModeHandler::DownloadAlbumPreviewImage(

chrome/browser/ui/webui/settings/chromeos/ambient_mode_handler.h

@@ -54,9 +54,6 @@ class AmbientModeHandler : public ::settings::SettingsPageUIHandler {
   // WebUI call to sync temperature unit with server.
   void HandleSetSelectedTemperatureUnit(const base::ListValue* args);
 
-  // WebUI call to sync topic source with server.
-  void HandleSetSelectedTopicSource(const base::ListValue* args);
-
   // WebUI call to sync albums with server.
   void HandleSetSelectedAlbums(const base::ListValue* args);
 
@@ -100,6 +97,10 @@ class AmbientModeHandler : public ::settings::SettingsPageUIHandler {
   // Populate albums with selected info which will be shown on Settings UI.
   void SyncSettingsAndAlbums();
 
+  // Update topic source if needed.
+  void UpdateTopicSource(ash::AmbientModeTopicSource topic_source);
+  void MaybeUpdateTopicSource(ash::AmbientModeTopicSource topic_source);
+
   void DownloadAlbumPreviewImage(ash::AmbientModeTopicSource topic_source);
 
   void OnAlbumPreviewImageDownloaded(ash::AmbientModeTopicSource topic_source,

chrome/browser/ui/webui/settings/chromeos/ambient_mode_handler_unittest.cc

@@ -6,6 +6,7 @@
 
 #include <memory>
 
+#include "ash/public/cpp/ambient/common/ambient_settings.h"
 #include "ash/public/cpp/ambient/fake_ambient_backend_controller_impl.h"
 #include "chrome/test/base/browser_with_test_window_test.h"
 #include "content/public/test/test_web_ui.h"
@@ -52,9 +53,9 @@ class AmbientModeHandlerTest : public testing::Test {
     handler_->HandleRequestSettings(&args);
   }
 
-  void RequestAlbums(int topic_source) {
+  void RequestAlbums(ash::AmbientModeTopicSource topic_source) {
     base::ListValue args;
-    args.Append(topic_source);
+    args.Append(static_cast<int>(topic_source));
     handler_->HandleRequestAlbums(&args);
   }
 
@@ -74,7 +75,10 @@ class AmbientModeHandlerTest : public testing::Test {
               topic_source_call_data.arg1()->GetString());
     // In FakeAmbientBackendControllerImpl, the |topic_source| is
     // kGooglePhotos.
-    EXPECT_EQ(0, topic_source_call_data.arg2()->GetInt());
+    const base::DictionaryValue* dictionary = nullptr;
+    topic_source_call_data.arg2()->GetAsDictionary(&dictionary);
+    const base::Value* topic_source_value = dictionary->FindKey("topicSource");
+    EXPECT_EQ(0, topic_source_value->GetInt());
 
     // Temperature Unit
     EXPECT_EQ(kWebCallbackFunctionName,
@@ -87,8 +91,21 @@ class AmbientModeHandlerTest : public testing::Test {
     run_loop->Quit();
   }
 
-  void VerifyAlbumsSent(int expected_topic_source, base::RunLoop* run_loop) {
-    EXPECT_EQ(1U, web_ui_->call_data().size());
+  void VerifyAlbumsSent(ash::AmbientModeTopicSource topic_source,
+                        base::RunLoop* run_loop) {
+    // Art gallery has an extra call to update the topic source to Art gallery.
+    std::vector<std::unique_ptr<content::TestWebUI::CallData>>::size_type call_size =
+        topic_source == ash::AmbientModeTopicSource::kGooglePhotos ? 1U : 2U;
+    EXPECT_EQ(call_size, web_ui_->call_data().size());
+
+    if (topic_source == ash::AmbientModeTopicSource::kArtGallery) {
+      const auto& topic_source_call_data = *web_ui_->call_data().front();
+      const base::DictionaryValue* dictionary = nullptr;
+      topic_source_call_data.arg2()->GetAsDictionary(&dictionary);
+      const base::Value* topic_source_value =
+          dictionary->FindKey("topicSource");
+      EXPECT_EQ(static_cast<int>(topic_source), topic_source_value->GetInt());
+    }
 
     const content::TestWebUI::CallData& call_data =
         *web_ui_->call_data().back();
@@ -103,7 +120,7 @@ class AmbientModeHandlerTest : public testing::Test {
     call_data.arg2()->GetAsDictionary(&dictionary);
 
     const base::Value* topic_source_value = dictionary->FindKey("topicSource");
-    EXPECT_EQ(expected_topic_source, topic_source_value->GetInt());
+    EXPECT_EQ(static_cast<int>(topic_source), topic_source_value->GetInt());
 
     const base::Value* albums = dictionary->FindKey("albums");
     EXPECT_EQ(2U, albums->GetList().size());
@@ -112,22 +129,20 @@ class AmbientModeHandlerTest : public testing::Test {
     albums->GetList()[0].GetAsDictionary(&album0);
     EXPECT_EQ("0", album0->FindKey("albumId")->GetString());
 
-    if (expected_topic_source == 0) {
-      EXPECT_EQ(false, album0->FindKey("checked")->GetBool());
-      EXPECT_EQ("album0", album0->FindKey("title")->GetString());
-
-    } else {
-      EXPECT_EQ(true, album0->FindKey("checked")->GetBool());
-      EXPECT_EQ("art0", album0->FindKey("title")->GetString());
-    }
     const base::DictionaryValue* album1;
     albums->GetList()[1].GetAsDictionary(&album1);
     EXPECT_EQ("1", album1->FindKey("albumId")->GetString());
 
-    if (expected_topic_source == 0) {
+    if (topic_source == ash::AmbientModeTopicSource::kGooglePhotos) {
+      EXPECT_EQ(false, album0->FindKey("checked")->GetBool());
+      EXPECT_EQ("album0", album0->FindKey("title")->GetString());
+
       EXPECT_EQ(true, album1->FindKey("checked")->GetBool());
       EXPECT_EQ("album1", album1->FindKey("title")->GetString());
     } else {
+      EXPECT_EQ(true, album0->FindKey("checked")->GetBool());
+      EXPECT_EQ("art0", album0->FindKey("title")->GetString());
+
       EXPECT_EQ(false, album1->FindKey("checked")->GetBool());
       EXPECT_EQ("art1", album1->FindKey("title")->GetString());
     }
@@ -145,16 +160,15 @@ TEST_F(AmbientModeHandlerTest, TestSendTemperatureUnitAndTopicSource) {
   RequestSettings();
 
   base::RunLoop run_loop;
-  base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE,
-      base::BindOnce(&AmbientModeHandlerTest::VerifySettingsSent,
-                     base::Unretained(this), &run_loop),
-      base::TimeDelta::FromSeconds(1));
+  base::SequencedTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::BindOnce(&AmbientModeHandlerTest::VerifySettingsSent,
+                                base::Unretained(this), &run_loop));
   run_loop.Run();
 }
 
 TEST_F(AmbientModeHandlerTest, TestSendAlbumsForGooglePhotos) {
-  int topic_source = 0;
+  ash::AmbientModeTopicSource topic_source =
+      ash::AmbientModeTopicSource::kGooglePhotos;
   RequestAlbums(topic_source);
 
   base::RunLoop run_loop;
@@ -166,7 +180,8 @@ TEST_F(AmbientModeHandlerTest, TestSendAlbumsForGooglePhotos) {
 }
 
 TEST_F(AmbientModeHandlerTest, TestSendAlbumsForArtGallery) {
-  int topic_source = 1;
+  ash::AmbientModeTopicSource topic_source =
+      ash::AmbientModeTopicSource::kArtGallery;
   RequestAlbums(topic_source);
 
   base::RunLoop run_loop;

chrome/browser/ui/webui/settings/chromeos/personalization_section.cc

@@ -193,8 +193,6 @@ void PersonalizationSection::AddLoadTimeData(
        IDS_OS_SETTINGS_AMBIENT_MODE_TOPIC_SOURCE_UNSELECTED_ROW},
       {"ambientModeTopicSourceSubpage",
        IDS_OS_SETTINGS_AMBIENT_MODE_TOPIC_SOURCE_SUBPAGE},
-      {"ambientModeAlbumsSubpageGooglePhotosNoAlbum",
-       IDS_OS_SETTINGS_AMBIENT_MODE_ALBUMS_SUBPAGE_GOOGLE_PHOTOS_NO_ALBUM},
       {"ambientModeWeatherTitle", IDS_OS_SETTINGS_AMBIENT_MODE_WEATHER_TITLE},
       {"ambientModeTemperatureUnitFahrenheit",
        IDS_OS_SETTINGS_AMBIENT_MODE_TEMPERATURE_UNIT_FAHRENHEIT},
@@ -240,6 +238,11 @@ void PersonalizationSection::AddLoadTimeData(
       l10n_util::GetStringFUTF16(
           IDS_OS_SETTINGS_AMBIENT_MODE_ALBUMS_SUBPAGE_GOOGLE_PHOTOS_TITLE,
           base::UTF8ToUTF16(GetGooglePhotosURL().spec())));
+  html_source->AddString(
+      "ambientModeAlbumsSubpageGooglePhotosNoAlbum",
+      l10n_util::GetStringFUTF16(
+          IDS_OS_SETTINGS_AMBIENT_MODE_ALBUMS_SUBPAGE_GOOGLE_PHOTOS_NO_ALBUM,
+          base::UTF8ToUTF16(GetGooglePhotosURL().spec())));
 }
 
 void PersonalizationSection::AddHandlers(content::WebUI* web_ui) {

chrome/test/data/webui/settings/chromeos/ambient_mode_page_test.js

@@ -22,7 +22,6 @@ class TestAmbientModeBrowserProxy extends TestBrowserProxy {
       'requestSettings',
       'requestAlbums',
       'setSelectedTemperatureUnit',
-      'setSelectedTopicSource',
       'setSelectedAlbums',
     ]);
   }
@@ -42,11 +41,6 @@ class TestAmbientModeBrowserProxy extends TestBrowserProxy {
     this.methodCalled('setSelectedTemperatureUnit', [temperatureUnit]);
   }
 
-  /** @override */
-  setSelectedTopicSource(topicSource) {
-    this.methodCalled('setSelectedTopicSource', [topicSource]);
-  }
-
   /** @override */
   setSelectedAlbums(settings) {
     this.methodCalled('setSelectedAlbums', [settings]);
@@ -126,8 +120,10 @@ suite('AmbientModeHandler', function() {
 
   test('doubleClickTopicSource', () => {
     // Select the google photos topic source.
-    cr.webUIListenerCallback(
-        'topic-source-changed', AmbientModeTopicSource.GOOGLE_PHOTOS);
+    cr.webUIListenerCallback('topic-source-changed', {
+      'topicSource': AmbientModeTopicSource.GOOGLE_PHOTOS,
+      'hasAlbums': true
+    });
 
     const topicSourceList = ambientModePage.$$('topic-source-list');
     const ironList = topicSourceList.$$('iron-list');
@@ -177,7 +173,7 @@ suite('AmbientModeHandler', function() {
   });
 
   test('hasAlbums', function() {
-    ambientModePhotosPage.albums_ = [
+    ambientModePhotosPage.albums = [
       {albumId: 'id0', checked: true, title: 'album0'},
       {albumId: 'id1', checked: false, title: 'album1'}
     ];

chrome/test/data/webui/settings/chromeos/ambient_mode_photos_page_test.js

@@ -19,7 +19,6 @@ class TestAmbientModeBrowserProxy extends TestBrowserProxy {
     super([
       'requestTopicSource',
       'requestAlbums',
-      'setSelectedTopicSource',
       'setSelectedAlbums',
     ]);
   }
@@ -34,12 +33,6 @@ class TestAmbientModeBrowserProxy extends TestBrowserProxy {
     this.methodCalled('requestAlbums', [topicSource]);
   }
 
-  /** @override */
-  setSelectedTopicSource(topicSource) {
-    this.methodCalled('setSelectedTopicSource', [topicSource]);
-  }
-
-  /** @override */
   setSelectedAlbums(settings) {
     this.methodCalled('setSelectedAlbums', [settings]);
   }
@@ -70,7 +63,7 @@ suite('AmbientModeHandler', function() {
   });
 
   test('hasAlbums', function() {
-    ambientModePhotosPage.albums_ = [
+    ambientModePhotosPage.albums = [
       {albumId: 'id0', checked: true, title: 'album0'},
       {albumId: 'id1', checked: false, title: 'album1'}
     ];
@@ -92,7 +85,7 @@ suite('AmbientModeHandler', function() {
   });
 
   test('toggleAlbumSelectionByClick', function() {
-    ambientModePhotosPage.albums_ = [
+    ambientModePhotosPage.albums = [
       {albumId: 'id0', checked: true, title: 'album0'},
       {albumId: 'id1', checked: false, title: 'album1'}
     ];
@@ -137,7 +130,7 @@ suite('AmbientModeHandler', function() {
   });
 
   test('setSelectedAlbums', async () => {
-    ambientModePhotosPage.albums_ = [
+    ambientModePhotosPage.albums = [
       {albumId: 'id0', checked: true, title: 'album0'},
       {albumId: 'id1', checked: false, title: 'album1'}
     ];
@@ -176,7 +169,7 @@ suite('AmbientModeHandler', function() {
   });
 
   test('notToggleAlbumSelection', function() {
-    ambientModePhotosPage.albums_ = [
+    ambientModePhotosPage.albums = [
       {albumId: 'id0', checked: true, title: 'album0'},
     ];
     Polymer.dom.flush();
@@ -203,7 +196,7 @@ suite('AmbientModeHandler', function() {
   });
 
   test('toggleAlbumSelectionByKeypress', function() {
-    ambientModePhotosPage.albums_ = [
+    ambientModePhotosPage.albums = [
       {albumId: 'id0', checked: true, title: 'album0'},
     ];
     Polymer.dom.flush();
@@ -237,10 +230,10 @@ suite('AmbientModeHandler', function() {
   });
 
   test('updateAlbumURL', function() {
-    ambientModePhotosPage.albums_ = [
+    ambientModePhotosPage.albums = [
       {albumId: 'id0', checked: true, title: 'album0', url: ''},
     ];
-    ambientModePhotosPage.topicSource_ = AmbientModeTopicSource.ART_GALLERY;
+    ambientModePhotosPage.topicSource = AmbientModeTopicSource.ART_GALLERY;
     Polymer.dom.flush();
 
     const albumList = ambientModePhotosPage.$$('album-list');
@@ -262,10 +255,10 @@ suite('AmbientModeHandler', function() {
   });
 
   test('notUpdateAlbumURL', function() {
-    ambientModePhotosPage.albums_ = [
+    ambientModePhotosPage.albums = [
       {albumId: 'id0', checked: true, title: 'album0', url: ''},
     ];
-    ambientModePhotosPage.topicSource_ = AmbientModeTopicSource.ART_GALLERY;
+    ambientModePhotosPage.topicSource = AmbientModeTopicSource.ART_GALLERY;
     Polymer.dom.flush();
 
     const albumList = ambientModePhotosPage.$$('album-list');