Cleanup mojo Wallpaper interfaces for mash.

chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc

Index: chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc
diff --git a/chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc b/chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc
index 5523f1b937643c2f25ce16dde96246ef8b69279d..c5c57939f3de3dead97a4a12f3e21d1d0d370d8a 100644
--- a/chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc
+++ b/chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc
@@ -4,29 +4,19 @@
 
 #include "chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.h"
 
-#include <stdint.h>
-#include <numeric>
 #include <utility>
-#include <vector>
 
 #include "ash/common/ash_constants.h"
 #include "ash/common/wallpaper/wallpaper_controller.h"
 #include "ash/common/wm_shell.h"
-#include "ash/public/interfaces/wallpaper.mojom.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
-#include "base/files/file_enumerator.h"
 #include "base/files/file_path.h"
-#include "base/files/file_util.h"
 #include "base/logging.h"
-#include "base/macros.h"
 #include "base/metrics/histogram_macros.h"
-#include "base/path_service.h"
 #include "base/sha1.h"
 #include "base/strings/string_number_conversions.h"
-#include "base/strings/string_util.h"
-#include "base/strings/stringprintf.h"
 #include "base/sys_info.h"
 #include "base/threading/worker_pool.h"
 #include "base/time/time.h"
@@ -35,25 +25,23 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/chromeos/customization/customization_document.h"
+#include "chrome/browser/chromeos/extensions/wallpaper_manager_util.h"
 #include "chrome/browser/chromeos/login/startup_utils.h"
+#include "chrome/browser/chromeos/login/users/avatar/user_image_loader.h"
 #include "chrome/browser/chromeos/login/wizard_controller.h"
-#include "chrome/browser/chromeos/settings/cros_settings.h"
+#include "chrome/browser/image_decoder.h"
 #include "chrome/browser/ui/ash/ash_util.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chromeos/chromeos_switches.h"
 #include "chromeos/cryptohome/system_salt_getter.h"
-#include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/login/user_names.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 #include "components/prefs/scoped_user_pref_update.h"
 #include "components/signin/core/account_id/account_id.h"
 #include "components/user_manager/known_user.h"
-#include "components/user_manager/user.h"
-#include "components/user_manager/user_image/user_image.h"
-#include "components/user_manager/user_manager.h"
 #include "components/user_manager/user_type.h"
 #include "components/wallpaper/wallpaper_files_id.h"
 #include "components/wallpaper/wallpaper_layout.h"
@@ -62,10 +50,6 @@
 #include "content/public/common/content_switches.h"
 #include "content/public/common/service_manager_connection.h"
 #include "services/shell/public/cpp/connector.h"
-#include "third_party/skia/include/core/SkColor.h"
-#include "ui/gfx/codec/jpeg_codec.h"
-#include "ui/gfx/image/image_skia_operations.h"
-#include "ui/gfx/skia_util.h"
 
 using content::BrowserThread;
 using wallpaper::WallpaperManagerBase;
@@ -187,39 +171,23 @@ void SetKnownUserWallpaperFilesId(
                                           wallpaper_files_id.id());
 }
 
-ash::mojom::WallpaperLayout WallpaperLayoutToMojo(
-    wallpaper::WallpaperLayout layout) {
-  switch (layout) {
-    case wallpaper::WALLPAPER_LAYOUT_CENTER:
-      return ash::mojom::WallpaperLayout::CENTER;
-    case wallpaper::WALLPAPER_LAYOUT_CENTER_CROPPED:
-      return ash::mojom::WallpaperLayout::CENTER_CROPPED;
-    case wallpaper::WALLPAPER_LAYOUT_STRETCH:
-      return ash::mojom::WallpaperLayout::STRETCH;
-    case wallpaper::WALLPAPER_LAYOUT_TILE:
-      return ash::mojom::WallpaperLayout::TILE;
-    case wallpaper::NUM_WALLPAPER_LAYOUT:
-      NOTREACHED();
-      return ash::mojom::WallpaperLayout::CENTER;
-  }
-  NOTREACHED();
-  return ash::mojom::WallpaperLayout::CENTER;
-}
-
-// A helper to set the wallpaper image for Ash and Mash.
+// A helper to set the wallpaper image for Classic Ash and Mash.
 void SetWallpaper(const gfx::ImageSkia& image,
                   wallpaper::WallpaperLayout layout) {
+  auto connection = content::ServiceManagerConnection::GetForProcess();
+  auto connector = connection ? connection->GetConnector() : nullptr;

[James Cook, 2016/10/14 21:58:02]

ditto

[msw, 2016/10/17 22:22:48]

Done.

+  if (!connector)
+    return;
+
+  ash::mojom::WallpaperControllerPtr wallpaper_controller;
+  // Under mash the WallpaperController interface is in the ash process. In
+  // classic ash we provide it to ourself.
   if (chrome::IsRunningInMash()) {
-    shell::Connector* connector =
-        content::ServiceManagerConnection::GetForProcess()->GetConnector();
-    ash::mojom::WallpaperControllerPtr wallpaper_controller;
     connector->ConnectToInterface("service:ash", &wallpaper_controller);
-    wallpaper_controller->SetWallpaper(*image.bitmap(),
-                                       WallpaperLayoutToMojo(layout));
-    return;
-  }
-  // Avoid loading unnecessary wallpapers in tests without a shell instance.
-  if (ash::WmShell::HasInstance()) {
+    // TODO(crbug.com/655875): Optimize ash wallpaper transport; avoid sending

[msw, 2016/10/14 21:07:43]

Because sending large wallpaper bitmaps over Mojo might cause UI jank (Robert
and I estimated ~500ms just to send ~100MB over a pipe in the login UI for some
lower spec devices with 4k support, etc.), I'm going to leave classic ash's
pattern of communicating directly with the WallpaperController here for now. We
can investigate the performance impact and/or optimize this separately before
enabling. (also note that enabling mojo here would require additional changes to
tests and SetDefaultWallpaperPath's use of WallpaperIsAlreadyLoaded that I
haven't quite resolved) This CL is still a worthwhile consolidation of logic and
addition of wallpaper enum traits.

+    // large bitmaps over Mojo; use shared memory like BitmapUploader, etc.
+    wallpaper_controller->SetWallpaper(*image.bitmap(), layout);
+  } else if (ash::WmShell::HasInstance()) {

[James Cook, 2016/10/14 21:58:02]

I would keep the previous comment about tests. (In general I would expect
!IsRunningInMash to imply we have a shell instance.)

Frankly I wonder if that comment is still true. It seems odd that we would run
tests without a shell.

[msw, 2016/10/17 22:22:48]

Running *Wallpaper* ash_unittests, browser_tests and interactive_ui_tests seems
to show that you're right. I've removed the two shell HasInstance checks and
test comments in this file; we'll see how the trybot runs go.

     ash::WmShell::Get()->wallpaper_controller()->SetWallpaperImage(image,
                                                                    layout);
   }
@@ -400,6 +368,11 @@ void WallpaperManager::Shutdown() {
   wallpaper_manager = nullptr;
 }
 
+void WallpaperManager::BindRequest(
+    ash::mojom::WallpaperManagerRequest request) {
+  bindings_.AddBinding(this, std::move(request));
+}
+
 WallpaperManager::WallpaperResolution
 WallpaperManager::GetAppropriateResolution() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ -478,6 +451,10 @@ void WallpaperManager::InitializeWallpaper() {
   SetUserWallpaperDelayed(user_manager->GetLoggedInUser()->GetAccountId());
 }
 
+void WallpaperManager::Open() {
+  wallpaper_manager_util::OpenWallpaperManager();
+}
+
 void WallpaperManager::Observe(int type,
                                const content::NotificationSource& source,
                                const content::NotificationDetails& details) {