DialogClientView: Correct the order in which subviews are focused.

ui/views/window/dialog_client_view.cc

Index: ui/views/window/dialog_client_view.cc
diff --git a/ui/views/window/dialog_client_view.cc b/ui/views/window/dialog_client_view.cc
index 569944542f94a7623b6c4c9f96f84cc393728a72..e700dd13606d959097c1bb30055b4edb7c80ef3c 100644
--- a/ui/views/window/dialog_client_view.cc
+++ b/ui/views/window/dialog_client_view.cc
@@ -81,39 +81,19 @@ void DialogClientView::CancelWindow() {
 }
 
 void DialogClientView::UpdateDialogButtons() {
-  const int buttons = GetDialogDelegate()->GetDialogButtons();
-  ui::Accelerator escape(ui::VKEY_ESCAPE, ui::EF_NONE);
-
-  if (buttons & ui::DIALOG_BUTTON_OK) {
-    if (!ok_button_) {
-      ok_button_ = CreateDialogButton(ui::DIALOG_BUTTON_OK);
-      if (!(buttons & ui::DIALOG_BUTTON_CANCEL))
-        ok_button_->AddAccelerator(escape);
-      AddChildView(ok_button_);
-    }
-
-    UpdateButton(ok_button_, ui::DIALOG_BUTTON_OK);
-  } else if (ok_button_) {
-    delete ok_button_;
-    ok_button_ = NULL;
-  }
-
-  if (buttons & ui::DIALOG_BUTTON_CANCEL) {
-    if (!cancel_button_) {
-      cancel_button_ = CreateDialogButton(ui::DIALOG_BUTTON_CANCEL);
-      cancel_button_->AddAccelerator(escape);
-      AddChildView(cancel_button_);
-    }
-
-    UpdateButton(cancel_button_, ui::DIALOG_BUTTON_CANCEL);
-  } else if (cancel_button_) {
-    delete cancel_button_;
-    cancel_button_ = NULL;
+  // Add buttons in left to right order, so that the focus order is left to
+  // right.
+  if (kIsOkButtonOnLeftSide) {
+    UpdateOKButton();
+    UpdateCancelButton();
+  } else {
+    UpdateCancelButton();
+    UpdateOKButton();
   }
 
   // Use the escape key to close the window if there are no dialog buttons.
   if (!has_dialog_buttons())
-    AddAccelerator(escape);
+    AddAccelerator(ui::Accelerator(ui::VKEY_ESCAPE, ui::EF_NONE));
   else
     ResetAccelerators();
 }
@@ -247,8 +227,8 @@ void DialogClientView::ViewHierarchyChanged(
     const ViewHierarchyChangedDetails& details) {
   ClientView::ViewHierarchyChanged(details);
   if (details.is_add && details.child == this) {
-    UpdateDialogButtons();
     CreateExtraView();
+    UpdateDialogButtons();
     CreateFootnoteView();
   } else if (!details.is_add && details.child != this) {
     if (details.child == ok_button_)
@@ -359,6 +339,40 @@ void DialogClientView::UpdateButton(LabelButton* button,
   button->SetIsDefault(type == dialog->GetDefaultDialogButton());
 }
 
+void DialogClientView::UpdateOKButton() {
+  const int buttons = GetDialogDelegate()->GetDialogButtons();
+  if (buttons & ui::DIALOG_BUTTON_OK) {
+    if (!ok_button_) {
+      ok_button_ = CreateDialogButton(ui::DIALOG_BUTTON_OK);
+      if (!(buttons & ui::DIALOG_BUTTON_CANCEL))
+        ok_button_->AddAccelerator(
+            ui::Accelerator(ui::VKEY_ESCAPE, ui::EF_NONE));
+      AddChildView(ok_button_);
+    }
+
+    UpdateButton(ok_button_, ui::DIALOG_BUTTON_OK);
+  } else if (ok_button_) {
+    delete ok_button_;
+    ok_button_ = NULL;

[tapted, 2016/02/15 10:22:20]

nit: now this is a move in the diff it should probably be updated to nullptr,
same below

[karandeepb, 2016/02/15 23:29:33]

Done.

+  }
+}
+
+void DialogClientView::UpdateCancelButton() {
+  if (GetDialogDelegate()->GetDialogButtons() & ui::DIALOG_BUTTON_CANCEL) {
+    if (!cancel_button_) {
+      cancel_button_ = CreateDialogButton(ui::DIALOG_BUTTON_CANCEL);
+      cancel_button_->AddAccelerator(
+          ui::Accelerator(ui::VKEY_ESCAPE, ui::EF_NONE));
+      AddChildView(cancel_button_);
+    }
+
+    UpdateButton(cancel_button_, ui::DIALOG_BUTTON_CANCEL);
+  } else if (cancel_button_) {
+    delete cancel_button_;
+    cancel_button_ = NULL;
+  }
+}
+
 int DialogClientView::GetButtonsAndExtraViewRowHeight() const {
   int extra_view_height = ShouldShow(extra_view_) ?
       extra_view_->GetPreferredSize().height() : 0;