Fix shelf layout when switching from left-aligned shelf to right-aligned shelf

ash/system/status_area_widget.cc

Index: ash/system/status_area_widget.cc
diff --git a/ash/system/status_area_widget.cc b/ash/system/status_area_widget.cc
index 43b53794eb3c74aa165b0cd021b403328f10aa3a..1176f29e16b1c95c9a12b7c11480a377fa0c08c3 100644
--- a/ash/system/status_area_widget.cc
+++ b/ash/system/status_area_widget.cc
@@ -4,9 +4,13 @@
 
 #include "ash/system/status_area_widget.h"
 
+#include "ash/common/material_design/material_design_controller.h"
+#include "ash/common/shelf/shelf_constants.h"
 #include "ash/common/shelf/wm_shelf.h"
+#include "ash/common/shelf/wm_shelf_util.h"
 #include "ash/common/shell_window_ids.h"
 #include "ash/common/system/tray/system_tray_delegate.h"
+#include "ash/common/system/tray/tray_constants.h"
 #include "ash/common/system/web_notification/web_notification_tray.h"
 #include "ash/common/wm_lookup.h"
 #include "ash/common/wm_root_window_controller.h"
@@ -62,14 +66,38 @@ void StatusAreaWidget::CreateTrayViews() {
 
   SystemTrayDelegate* delegate = WmShell::Get()->system_tray_delegate();
   DCHECK(delegate);
-  // Initialize after all trays have been created.
-  system_tray_->InitializeTrayItems(delegate, web_notification_tray_);
-  web_notification_tray_->Initialize();
+
+  if (MaterialDesignController::IsShelfMaterial()) {
+    gfx::Insets edge_border;
+    gfx::Insets no_edge_border;
+    CalculateBorderMD(&edge_border, system_tray_->shelf_alignment(), true);
+    CalculateBorderMD(&no_edge_border, system_tray_->shelf_alignment(), false);

[tdanderson, 2016/06/29 17:46:19]

The generally preferred pattern in a case like this is to have
CalculateBorderMD() return a gfx::Insets rather than mutate a gfx::Insets*
argument. Ditto for 86-89.

[yiyix, 2016/06/29 18:56:39]

Done.

+    // Initialize after all trays have been created.
+    system_tray_->InitializeTrayItems(
+        delegate, web_notification_tray_,
+        overview_button_tray_->visible() ? no_edge_border : edge_border);
+    web_notification_tray_->Initialize(no_edge_border);
+#if defined(OS_CHROMEOS)
+    logout_button_tray_->Initialize(no_edge_border);
+    virtual_keyboard_tray_->Initialize(no_edge_border);
+#endif
+    overview_button_tray_->Initialize(edge_border);
+  } else {
+    gfx::Insets border;
+    gfx::Insets overview_border;
+    CalculateBorderNonMD(&border, system_tray_->shelf_alignment(), false);
+    CalculateBorderNonMD(&overview_border, system_tray_->shelf_alignment(),
+                         true);
+    // Initialize after all trays have been created.
+    system_tray_->InitializeTrayItems(delegate, web_notification_tray_, border);
+    web_notification_tray_->Initialize(border);
 #if defined(OS_CHROMEOS)
-  logout_button_tray_->Initialize();
-  virtual_keyboard_tray_->Initialize();
+    logout_button_tray_->Initialize(border);
+    virtual_keyboard_tray_->Initialize(border);
 #endif
-  overview_button_tray_->Initialize();
+    overview_button_tray_->Initialize(overview_border);
+  }
+
   SetShelfAlignment(system_tray_->shelf_alignment());
   UpdateAfterLoginStatusChange(delegate->GetUserLoginStatus());
 }
@@ -172,20 +200,49 @@ void StatusAreaWidget::AddOverviewButtonTray() {
   status_area_widget_delegate_->AddTray(overview_button_tray_);
 }
 
+// TODO(yiyix): Once Chrome OS material design is enabled by default,
+// remove all code related to Non Md Border setup code. See crbug.com/614453.

[tdanderson, 2016/06/29 17:46:19]

Thanks for adding this. Though in this specific case I don't think it's strictly
necessary since the dead code will be readily apparent when we do a code search
for uses of MDC::IsShelfMaterial().

nit: "Border setup." instead of "Border setup code."

[yiyix, 2016/06/29 18:56:39]

I will remove this comment

 void StatusAreaWidget::SetShelfAlignment(ShelfAlignment alignment) {
   status_area_widget_delegate_->set_alignment(alignment);
-  if (system_tray_)
-    system_tray_->SetShelfAlignment(alignment);
-  if (web_notification_tray_)
-    web_notification_tray_->SetShelfAlignment(alignment);
+
+  if (MaterialDesignController::IsShelfMaterial()) {
+    gfx::Insets insets_on_edge;
+    gfx::Insets insets_not_on_edge;
+    CalculateBorderMD(&insets_on_edge, alignment, true);
+    CalculateBorderMD(&insets_not_on_edge, alignment, false);
+    if (system_tray_)
+      system_tray_->SetShelfAlignment(
+          alignment, overview_button_tray_->visible() ? insets_not_on_edge
+                                                      : insets_on_edge);
+    if (web_notification_tray_)
+      web_notification_tray_->SetShelfAlignment(alignment, insets_not_on_edge);
 #if defined(OS_CHROMEOS)
   if (logout_button_tray_)
-    logout_button_tray_->SetShelfAlignment(alignment);
+    logout_button_tray_->SetShelfAlignment(alignment, insets_not_on_edge);
   if (virtual_keyboard_tray_)
-    virtual_keyboard_tray_->SetShelfAlignment(alignment);
+    virtual_keyboard_tray_->SetShelfAlignment(alignment, insets_not_on_edge);
 #endif
   if (overview_button_tray_)
-    overview_button_tray_->SetShelfAlignment(alignment);
+    overview_button_tray_->SetShelfAlignment(alignment, insets_on_edge);
+  } else {
+    gfx::Insets insets;
+    gfx::Insets insets_overview;
+    CalculateBorderNonMD(&insets, alignment, false);
+    CalculateBorderNonMD(&insets_overview, alignment, true);

[tdanderson, 2016/06/29 17:46:19]

Consider only performing this calculation inside the if (overview_button_tray_)

[yiyix, 2016/06/29 18:56:39]

Done.

+    if (system_tray_)
+      system_tray_->SetShelfAlignment(alignment, insets);
+    if (web_notification_tray_)
+      web_notification_tray_->SetShelfAlignment(alignment, insets);
+#if defined(OS_CHROMEOS)
+    if (logout_button_tray_)
+      logout_button_tray_->SetShelfAlignment(alignment, insets);
+    if (virtual_keyboard_tray_)
+      virtual_keyboard_tray_->SetShelfAlignment(alignment, insets);
+#endif
+    if (overview_button_tray_)
+      overview_button_tray_->SetShelfAlignment(alignment, insets_overview);
+  }
+
   status_area_widget_delegate_->UpdateLayout();
 }
 
@@ -205,4 +262,54 @@ void StatusAreaWidget::UpdateAfterLoginStatusChange(LoginStatus login_status) {
     overview_button_tray_->UpdateAfterLoginStatusChange(login_status);
 }
 
+void StatusAreaWidget::CalculateBorderMD(gfx::Insets* insets,
+                                         ShelfAlignment alignment,
+                                         bool is_on_edge) {
+  const int shelf_size = GetShelfConstant(SHELF_SIZE);
+  const int item_height = GetTrayConstant(TRAY_ITEM_HEIGHT_LEGACY);
+  int top_edge, left_edge, bottom_edge, right_edge;
+
+  // Tray views are laid out right-to-left or bottom-to-top.
+  const bool horizontal_alignment = IsHorizontalAlignment(alignment);
+  const int padding = (shelf_size - item_height) / 2;
+  const int extended_padding = GetTrayConstant(TRAY_PADDING_FROM_EDGE_OF_SHELF);
+
+  top_edge = horizontal_alignment ? padding : 0;
+  left_edge = horizontal_alignment ? 0 : padding;
+  bottom_edge =
+      horizontal_alignment ? padding : (is_on_edge ? extended_padding : 0);
+  right_edge =
+      horizontal_alignment ? (is_on_edge ? extended_padding : 0) : padding;
+
+  insets->Set(top_edge, left_edge, bottom_edge, right_edge);
+}
+
+void StatusAreaWidget::CalculateBorderNonMD(gfx::Insets* insets,
+                                            ShelfAlignment alignment,
+                                            bool is_overview) {
+  const int shelf_size = GetShelfConstant(SHELF_SIZE);
+  const int item_height = GetTrayConstant(TRAY_ITEM_HEIGHT_LEGACY);
+  int top_edge, left_edge, bottom_edge, right_edge;
+  if (IsHorizontalAlignment(alignment)) {
+    top_edge = kShelfItemInset;
+    left_edge = 0;
+    bottom_edge = shelf_size - kShelfItemInset - item_height;
+    right_edge =
+        is_overview ? GetTrayConstant(TRAY_PADDING_FROM_EDGE_OF_SHELF) : 0;
+  } else if (alignment == SHELF_ALIGNMENT_LEFT) {
+    top_edge = 0;
+    left_edge = shelf_size - kShelfItemInset - item_height;
+    bottom_edge =
+        is_overview ? GetTrayConstant(TRAY_PADDING_FROM_EDGE_OF_SHELF) : 0;
+    right_edge = kShelfItemInset;
+  } else {  // SHELF_ALIGNMENT_RIGHT
+    top_edge = 0;
+    left_edge = kShelfItemInset;
+    bottom_edge =
+        is_overview ? GetTrayConstant(TRAY_PADDING_FROM_EDGE_OF_SHELF) : 0;
+    right_edge = shelf_size - kShelfItemInset - item_height;
+  }
+  insets->Set(top_edge, left_edge, bottom_edge, right_edge);

[tdanderson, 2016/06/29 17:46:19]

Some refactoring may be advisable here, but I will defer to James' opinion to
avoid churn (i.e., having you do it the way I suggest only to have James suggest
something different).

[yiyix, 2016/06/29 18:56:39]

Ok, I agree that some trick we used in space calculation is MD can be used here.
I can wait until James's review

+}
+
 }  // namespace ash