Move logic in UserImageScreen from UserImageScreenHandler

chrome/browser/ui/webui/chromeos/login/user_image_screen_handler.cc

Index: chrome/browser/ui/webui/chromeos/login/user_image_screen_handler.cc
diff --git a/chrome/browser/ui/webui/chromeos/login/user_image_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/user_image_screen_handler.cc
index 6ce65e6e5a4fc528b02fb3f1880dbc1613dc3689..d02155c125110ae19b2f03fc725531a9d19bc54a 100644
--- a/chrome/browser/ui/webui/chromeos/login/user_image_screen_handler.cc
+++ b/chrome/browser/ui/webui/chromeos/login/user_image_screen_handler.cc
@@ -8,63 +8,28 @@
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/values.h"
-#include "chrome/browser/chromeos/camera_detector.h"
 #include "chrome/browser/chromeos/login/default_user_images.h"
 #include "chrome/browser/chromeos/login/user.h"
 #include "chrome/browser/chromeos/login/webui_login_display.h"
 #include "chrome/browser/ui/webui/chromeos/login/oobe_ui.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/url_constants.h"
-#include "content/public/browser/browser_thread.h"
 #include "googleurl/src/gurl.h"
 #include "grit/generated_resources.h"
 #include "net/base/data_url.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/views/widget/widget.h"

[Nikita (slow), 2013/04/17 20:28:04]

nit: not needed

[Denis Kuznetsov (DE-MUC), 2013/04/18 08:08:29]

Done.

-#include "ui/webui/web_ui_util.h"
-
-using content::BrowserThread;
 
 namespace chromeos {
 
 UserImageScreenHandler::UserImageScreenHandler()
     : screen_(NULL),
-      show_on_init_(false),
-      selected_image_(User::kInvalidImageIndex),
-      accept_photo_after_decoding_(false),
-      user_photo_data_url_(chrome::kAboutBlankURL),
-      profile_picture_data_url_(chrome::kAboutBlankURL),
-      profile_picture_absent_(false),
-      ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
+      show_on_init_(false) {
 }
 
 UserImageScreenHandler::~UserImageScreenHandler() {
   if (screen_)
     screen_->OnActorDestroyed(this);
-  if (image_decoder_.get())
-    image_decoder_->set_delegate(NULL);
-}
-
-void UserImageScreenHandler::DeclareLocalizedValues(
-    LocalizedValuesBuilder* builder) {
-  // TODO(ivankr): string should be renamed to something like
-  // IDS_USER_IMAGE_SCREEN_TITLE (currently used for Take Photo dialog).
-  builder->Add("userImageScreenTitle", IDS_OOBE_PICTURE);
-  builder->Add("userImageScreenDescription",
-               IDS_OPTIONS_CHANGE_PICTURE_DIALOG_TEXT);
-  builder->Add("takePhoto", IDS_OPTIONS_CHANGE_PICTURE_TAKE_PHOTO);
-  builder->Add("discardPhoto", IDS_OPTIONS_CHANGE_PICTURE_DISCARD_PHOTO);
-  builder->Add("flipPhoto", IDS_OPTIONS_CHANGE_PICTURE_FLIP_PHOTO);
-  builder->Add("profilePhoto", IDS_IMAGE_SCREEN_PROFILE_PHOTO);
-  builder->Add("profilePhotoLoading",
-               IDS_IMAGE_SCREEN_PROFILE_LOADING_PHOTO);
-  builder->Add("okButtonText", IDS_OK);
-  builder->Add("authorCredit", IDS_OPTIONS_SET_WALLPAPER_AUTHOR_TEXT);
-  builder->Add("photoFromCamera", IDS_OPTIONS_CHANGE_PICTURE_PHOTO_FROM_CAMERA);
-  builder->Add("photoCaptureAccessibleText",
-               IDS_OPTIONS_PHOTO_CAPTURE_ACCESSIBLE_TEXT);
-  builder->Add("photoDiscardAccessibleText",
-               IDS_OPTIONS_PHOTO_DISCARD_ACCESSIBLE_TEXT);
 }
 
 void UserImageScreenHandler::Initialize() {
@@ -88,7 +53,9 @@ void UserImageScreenHandler::Show() {
   ShowScreen(OobeUI::kScreenUserImagePicker, NULL);
 
   // When shown, query camera presence.
-  CheckCameraPresence();
+  if (!screen_)
+    return;
+  screen_->CheckCameraPresence();
 }
 
 void UserImageScreenHandler::Hide() {
@@ -97,19 +64,26 @@ void UserImageScreenHandler::Hide() {
 void UserImageScreenHandler::PrepareToShow() {
 }
 
-void UserImageScreenHandler::SelectImage(int index) {
-  selected_image_ = index;
-  if (page_is_ready()) {
-    base::StringValue image_url(GetDefaultImageUrl(index));
-    CallJS("oobe.UserImageScreen.setSelectedImage",
-           image_url);
-  }
-}
-
-void UserImageScreenHandler::CheckCameraPresence() {
-  CameraDetector::StartPresenceCheck(
-      base::Bind(&UserImageScreenHandler::OnCameraPresenceCheckDone,
-                 weak_factory_.GetWeakPtr()));
+void UserImageScreenHandler::DeclareLocalizedValues(
+    LocalizedValuesBuilder* builder) {
+  // TODO(ivankr): string should be renamed to something like
+  // IDS_USER_IMAGE_SCREEN_TITLE (currently used for Take Photo dialog).
+  builder->Add("userImageScreenTitle", IDS_OOBE_PICTURE);
+  builder->Add("userImageScreenDescription",
+               IDS_OPTIONS_CHANGE_PICTURE_DIALOG_TEXT);
+  builder->Add("takePhoto", IDS_OPTIONS_CHANGE_PICTURE_TAKE_PHOTO);
+  builder->Add("discardPhoto", IDS_OPTIONS_CHANGE_PICTURE_DISCARD_PHOTO);
+  builder->Add("flipPhoto", IDS_OPTIONS_CHANGE_PICTURE_FLIP_PHOTO);
+  builder->Add("profilePhoto", IDS_IMAGE_SCREEN_PROFILE_PHOTO);
+  builder->Add("profilePhotoLoading",
+               IDS_IMAGE_SCREEN_PROFILE_LOADING_PHOTO);
+  builder->Add("okButtonText", IDS_OK);
+  builder->Add("authorCredit", IDS_OPTIONS_SET_WALLPAPER_AUTHOR_TEXT);
+  builder->Add("photoFromCamera", IDS_OPTIONS_CHANGE_PICTURE_PHOTO_FROM_CAMERA);
+  builder->Add("photoCaptureAccessibleText",
+               IDS_OPTIONS_PHOTO_CAPTURE_ACCESSIBLE_TEXT);
+  builder->Add("photoDiscardAccessibleText",
+               IDS_OPTIONS_PHOTO_DISCARD_ACCESSIBLE_TEXT);
 }
 
 void UserImageScreenHandler::RegisterMessages() {
@@ -124,26 +98,26 @@ void UserImageScreenHandler::RegisterMessages() {
               &UserImageScreenHandler::HandleScreenShown);
 }
 
-void UserImageScreenHandler::AddProfileImage(const gfx::ImageSkia& image) {
-  profile_picture_data_url_ = webui::GetBitmapDataUrl(*image.bitmap());
-  SendProfileImage(profile_picture_data_url_);
+void UserImageScreenHandler::SelectImage(int index) {
+  if (page_is_ready()) {

[Nikita (slow), 2013/04/17 20:28:04]

nit: drop {}

[Denis Kuznetsov (DE-MUC), 2013/04/18 08:08:29]

Done.

+    CallJS("oobe.UserImageScreen.setSelectedImage", GetDefaultImageUrl(index));
+  }
 }
 
 void UserImageScreenHandler::SendProfileImage(const std::string& data_url) {
   if (page_is_ready()) {

[Nikita (slow), 2013/04/17 20:28:04]

nit: drop {}

[Denis Kuznetsov (DE-MUC), 2013/04/18 08:08:29]

Done.

-    base::StringValue data_url_value(data_url);
-    CallJS("oobe.UserImageScreen.setProfileImage", data_url_value);
+    CallJS("oobe.UserImageScreen.setProfileImage", data_url);
   }
 }
 
 void UserImageScreenHandler::OnProfileImageAbsent() {
-  profile_picture_absent_ = true;
   if (page_is_ready()) {
     scoped_ptr<base::Value> null_value(base::Value::CreateNullValue());
     CallJS("oobe.UserImageScreen.setProfileImage", *null_value);
   }
 }
 
+// TODO(antrim) : It looks more like parameters for "Init" rather than callback.
 void UserImageScreenHandler::HandleGetImages() {
   base::ListValue image_urls;
   for (int i = kFirstDefaultImageIndex; i < kDefaultImagesCount; ++i) {
@@ -157,77 +131,42 @@ void UserImageScreenHandler::HandleGetImages() {
     image_urls.Append(image_data.release());
   }
   CallJS("oobe.UserImageScreen.setDefaultImages", image_urls);
+  if (!screen_)
+    return;
+  if (screen_->selected_image() != User::kInvalidImageIndex)
+    SelectImage(screen_->selected_image());
 
-  if (selected_image_ != User::kInvalidImageIndex)
-    SelectImage(selected_image_);
-
-  if (profile_picture_data_url_ != chrome::kAboutBlankURL)
-    SendProfileImage(profile_picture_data_url_);
-  else if (profile_picture_absent_)
+  if (screen_->profile_picture_data_url() != chrome::kAboutBlankURL)
+    SendProfileImage(screen_->profile_picture_data_url());
+  else if (screen_->profile_picture_absent())
     OnProfileImageAbsent();
 }
 
 void UserImageScreenHandler::HandlePhotoTaken(const std::string& image_url) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   std::string mime_type, charset, raw_data;
   if (!net::DataURL::Parse(GURL(image_url), &mime_type, &charset, &raw_data))

[Nikita (slow), 2013/04/17 20:28:04]

Should this code be also moved to screen?

[Denis Kuznetsov (DE-MUC), 2013/04/18 02:57:50]

It depends on how we treat it - is passing image in URL a part of JS<->handler
interaction just like wrapping things in base::Value (in this case this code
belongs to handler), or it is a part of higher logic, in this case it belongs to
screen code.

     NOTREACHED();
   DCHECK_EQ("image/png", mime_type);
 
-  user_photo_ = gfx::ImageSkia();
-  user_photo_data_url_ = image_url;
-
-  if (image_decoder_.get())
-    image_decoder_->set_delegate(NULL);
-  image_decoder_ = new ImageDecoder(this, raw_data,
-                                    ImageDecoder::DEFAULT_CODEC);
-  scoped_refptr<base::MessageLoopProxy> task_runner =
-      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI);
-  image_decoder_->Start(task_runner);
+  if (screen_)
+    screen_->OnPhotoTaken(raw_data);
 }
 
 void UserImageScreenHandler::HandleCheckCameraPresence() {
-  CheckCameraPresence();
+  if (!screen_)
+    return;
+  screen_->CheckCameraPresence();
 }
 
 void UserImageScreenHandler::HandleSelectImage(const std::string& image_url,
                                                const std::string& image_type) {
-  if (image_url.empty())
-    return;
-
-  int user_image_index = User::kInvalidImageIndex;
-  if (image_type == "default" &&
-      IsDefaultImageUrl(image_url, &user_image_index)) {
-    selected_image_ = user_image_index;
-  } else if (image_type == "camera") {
-    selected_image_ = User::kExternalImageIndex;
-  } else if (image_type == "profile") {
-    selected_image_ = User::kProfileImageIndex;
-  } else {
-    NOTREACHED() << "Unexpected image type: " << image_type;
-  }
+  if (screen_)
+    screen_->OnImageSelected(image_type, image_url);
 }
 
 void UserImageScreenHandler::HandleImageAccepted() {
-  if (!screen_)
-    return;
-  switch (selected_image_) {
-    case User::kExternalImageIndex:
-      // Photo decoding may not have been finished yet.
-      if (user_photo_.isNull())
-        accept_photo_after_decoding_ = true;
-      else
-        screen_->OnPhotoTaken(user_photo_);
-      break;
-
-    case User::kProfileImageIndex:
-      screen_->OnProfileImageSelected();
-      break;
-
-    default:
-      DCHECK(selected_image_ >= 0 && selected_image_ < kDefaultImagesCount);
-      screen_->OnDefaultImageSelected(selected_image_);
-  }
+  if (screen_)
+    screen_->OnImageAccepted();
 }
 
 void UserImageScreenHandler::HandleScreenShown() {
@@ -238,27 +177,12 @@ void UserImageScreenHandler::HandleScreenShown() {
   UMA_HISTOGRAM_TIMES("UserImage.ScreenIsShownTime", delta);
 }
 
-void UserImageScreenHandler::OnCameraPresenceCheckDone() {
-  base::FundamentalValue present_value(
-      CameraDetector::camera_presence() == CameraDetector::kCameraPresent);
-  CallJS("oobe.UserImageScreen.setCameraPresent", present_value);
-}
-
-void UserImageScreenHandler::OnImageDecoded(const ImageDecoder* decoder,
-                                            const SkBitmap& decoded_image) {
-  DCHECK_EQ(image_decoder_.get(), decoder);
-  user_photo_ = gfx::ImageSkia::CreateFrom1xBitmap(decoded_image);
-  if (screen_ && accept_photo_after_decoding_)
-    screen_->OnPhotoTaken(user_photo_);
-}
-
-void UserImageScreenHandler::OnDecodeImageFailed(const ImageDecoder* decoder) {
-  NOTREACHED() << "Failed to decode PNG image from WebUI";
+void UserImageScreenHandler::SetCameraPresent(bool present) {
+  CallJS("oobe.UserImageScreen.setCameraPresent", present);
 }
 
 void UserImageScreenHandler::SetProfilePictureEnabled(bool enabled) {
-  base::FundamentalValue present_value(enabled);
-  CallJS("oobe.UserImageScreen.setProfilePictureEnabled", present_value);
+  CallJS("oobe.UserImageScreen.setProfilePictureEnabled", enabled);
 }
 
 }  // namespace chromeos