Make it impossible to click OK if you've selected 0 datatypes.

BUG=40880
TEST=try to Customize Sync so you're syncing 0 datatypes.  You shouldn't be able to.

Review URL: http://codereview.chromium.org/1641001

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@44373 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 73 insertions, 9 deletions

diff --git a/chrome/browser/sync/sync_setup_flow.cc b/chrome/browser/sync/sync_setup_flow.cc
index 595b3a3..b7f5bae 100644
--- a/chrome/browser/sync/sync_setup_flow.cc
+++ b/chrome/browser/sync/sync_setup_flow.cc
@@ -107,6 +107,19 @@ void FlowHandler::HandleSubmitAuth(const Value* value) {
   if (json.empty())
     return;
 
+  // If ClickOk() returns false (indicating that there's a problem in the
+  // CustomizeSyncWindowView), don't do anything; the CSWV will focus itself,
+  // indicating that there's something to do there.
+  // ClickOk() has no side effects if the singleton dialog is not present.
+  if (!flow_->ClickCustomizeOk()) {
+    // TODO(dantasse): this results in a kinda ugly experience for this edge
+    // case; come back here and add a nice message explaining that you can't
+    // sync zero datatypes.  (OR just make the CSWV modal to the Gaia Login
+    // box, like we want to do anyway.
+    flow_->Advance(SyncSetupWizard::GAIA_LOGIN);
+    return;
+  }
+
   if (!GetAuthData(json, &username, &password, &captcha)) {
     // The page sent us something that we didn't understand.
     // This probably indicates a programming error.
diff --git a/chrome/browser/sync/sync_setup_flow.h b/chrome/browser/sync/sync_setup_flow.h
index 35ac55e..42198fa 100644
--- a/chrome/browser/sync/sync_setup_flow.h
+++ b/chrome/browser/sync/sync_setup_flow.h
@@ -95,11 +95,14 @@ class SyncSetupFlow : public HtmlDialogUIDelegate {
 #endif
   }
 
-  void ClickCustomizeOk() {
+  bool ClickCustomizeOk() {
 #if defined(OS_WIN)
-    CustomizeSyncWindowView::ClickOk();
+    return CustomizeSyncWindowView::ClickOk();
 #elif defined(OS_LINUX)
     CustomizeSyncWindowOk();
+    return true;
+#else
+    return true;
 #endif
   }
 
diff --git a/chrome/browser/views/options/customize_sync_window_view.cc b/chrome/browser/views/options/customize_sync_window_view.cc
index 4dd2fd9..602fec1 100755
--- a/chrome/browser/views/options/customize_sync_window_view.cc
+++ b/chrome/browser/views/options/customize_sync_window_view.cc
@@ -15,8 +15,9 @@
 #include "gfx/font.h"
 #include "grit/generated_resources.h"
 #include "grit/locale_settings.h"
-#include "views/controls/label.h"
+#include "views/controls/button/button.h"
 #include "views/controls/button/checkbox.h"
+#include "views/controls/label.h"
 #include "views/standard_layout.h"
 #include "views/window/window.h"
 
@@ -52,14 +53,21 @@ void CustomizeSyncWindowView::Show(gfx::NativeWindow parent_window,
 }
 
 // static
-void CustomizeSyncWindowView::ClickOk() {
+bool CustomizeSyncWindowView::ClickOk() {
   if (instance_) {
-    instance_->Accept();
-    instance_->window()->Close();
+    if (instance_->IsDialogButtonEnabled(MessageBoxFlags::DIALOGBUTTON_OK)) {
+      instance_->Accept();
+      instance_->window()->Close();
+      return true;
+    } else {
+      instance_->Focus();
+      return false;
+    }
+  } else {
+    return true;
   }
 }
 
-// Ideally this would do the same as when you click "cancel".
 // static
 void CustomizeSyncWindowView::ClickCancel() {
   if (instance_) {
@@ -146,6 +154,8 @@ bool CustomizeSyncWindowView::Accept() {
     desired_types.insert(syncable::THEMES);
   }
 
+  // You shouldn't be able to accept if you've selected 0 datatypes.
+  DCHECK(!desired_types.empty());
   profile_->GetProfileSyncService()->ChangePreferredDataTypes(desired_types);
 
   return true;
@@ -156,6 +166,24 @@ int CustomizeSyncWindowView::GetDialogButtons() const {
          MessageBoxFlags::DIALOGBUTTON_CANCEL;
 }
 
+bool CustomizeSyncWindowView::IsDialogButtonEnabled(
+  MessageBoxFlags::DialogButton button) const {
+    switch (button) {
+      case MessageBoxFlags::DIALOGBUTTON_OK:
+        // The OK button should be enabled if any checkbox is checked.
+        return bookmarks_check_box_->checked() ||
+            (preferences_check_box_ && preferences_check_box_->checked()) ||
+            (autofill_check_box_ && autofill_check_box_->checked()) ||
+            (themes_check_box_ && themes_check_box_->checked());
+      case MessageBoxFlags::DIALOGBUTTON_CANCEL:
+        return true;
+      default:
+        NOTREACHED() << "CustomizeSyncWindowView should only have OK and "
+          << "Cancel buttons.";
+        return false;
+    }
+}
+
 std::wstring CustomizeSyncWindowView::GetWindowTitle() const {
   return l10n_util::GetString(IDS_CUSTOMIZE_SYNC_WINDOW_TITLE);
 }
@@ -171,12 +199,21 @@ void CustomizeSyncWindowView::WindowClosing() {
 }
 
 /////////////////////////////////////////////////////////////////////////////
+// CustomizeSyncWindowView, views::ButtonListener implementations
+
+void CustomizeSyncWindowView::ButtonPressed(views::Button* sender,
+                                            const views::Event& event) {
+  GetWindow()->GetClientView()->AsDialogClientView()->UpdateDialogButtons();
+}
+
+/////////////////////////////////////////////////////////////////////////////
 // CustomizeSyncWindowView, private
 
 views::Checkbox* CustomizeSyncWindowView::AddCheckbox(const std::wstring& text,
                                                       bool checked) {
   views::Checkbox* checkbox = new views::Checkbox(text);
   checkbox->SetChecked(checked);
+  checkbox->set_listener(this);
   AddChildView(checkbox);
   return checkbox;
 }
diff --git a/chrome/browser/views/options/customize_sync_window_view.h b/chrome/browser/views/options/customize_sync_window_view.h
index e9e35a4..ef51c14 100755
--- a/chrome/browser/views/options/customize_sync_window_view.h
+++ b/chrome/browser/views/options/customize_sync_window_view.h
@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_VIEWS_OPTIONS_CUSTOMIZE_SYNC_WINDOW_VIEW_H_
 #define CHROME_BROWSER_VIEWS_OPTIONS_CUSTOMIZE_SYNC_WINDOW_VIEW_H_
 
+#include "views/controls/button/button.h"
 #include "views/view.h"
 #include "views/window/dialog_delegate.h"
 #include "views/window/window.h"
@@ -17,7 +18,8 @@ class Label;
 class Profile;
 
 class CustomizeSyncWindowView : public views::View,
-                                public views::DialogDelegate {
+                                public views::DialogDelegate,
+                                public views::ButtonListener {
  public:
   virtual ~CustomizeSyncWindowView() {}
 
@@ -29,7 +31,9 @@ class CustomizeSyncWindowView : public views::View,
 
   // Simulate clicking the "OK" and "Cancel" buttons on the singleton dialog,
   // if it exists.
-  static void ClickOk();
+  // ClickOk() returns whether it's possible to click OK (i.e. you can't click
+  // OK if you have selected zero data types to sync.)
+  static bool ClickOk();
   static void ClickCancel();
 
   // views::View methods:
@@ -41,6 +45,8 @@ class CustomizeSyncWindowView : public views::View,
   // views::DialogDelegate methods:
   virtual bool Accept();
   virtual int GetDialogButtons() const;
+  virtual bool IsDialogButtonEnabled(
+      MessageBoxFlags::DialogButton button) const;
   virtual bool CanResize() const { return false; }
   virtual bool CanMaximize() const { return false; }
   virtual bool IsAlwaysOnTop() const { return false; }
@@ -51,6 +57,11 @@ class CustomizeSyncWindowView : public views::View,
   virtual void WindowClosing();
   virtual views::View* GetContentsView();
 
+  // views::ButtonListener method:
+  // Update the "OK" button whenever you click a checkbox, so that if you
+  // uncheck all the checkboxes, the "OK" box is grayed out.
+  virtual void ButtonPressed(views::Button* sender, const views::Event& event);
+
  private:
   explicit CustomizeSyncWindowView(Profile* profile);