Realign message center popups when displays are added or removed

ui/message_center/views/desktop_popup_alignment_delegate.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/message_center/views/desktop_popup_alignment_delegate.h"
 
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/message_center/message_center_style.h"
@@ skipping 66 matching lines @@
   // Since on some platforms like Ubuntu Unity there's also a launcher along
   // with a taskbar (panel), we need to check that there is really nothing at
   // the top before concluding that the taskbar is at the left.
   alignment_ |= (work_area_.x() > display.bounds().x() &&
                  work_area_.y() == display.bounds().y())
       ? POPUP_ALIGNMENT_LEFT
       : POPUP_ALIGNMENT_RIGHT;
 }
 
 void DesktopPopupAlignmentDelegate::OnDisplayAdded(
-    const display::Display& new_display) {}
+    const display::Display& new_display) {

[dewittj, 2016/06/20 17:06:00]

rename to added_display

[Jun Mukai, 2016/06/20 20:28:54]

Can you also leave a comment for why these are necessary?

That would be something like:

// Primary display can change upon adding or removing display, and
// this will have to recompute the alignment on the new screen.

[osandov, 2016/06/21 08:01:05]

Done.

[osandov, 2016/06/21 08:01:06]

Done.

+  display::Display display = screen_->GetPrimaryDisplay();

[dewittj, 2016/06/20 17:06:00]

nit: rename to primary_display

[osandov, 2016/06/21 08:01:05]

This moved after refactoring, but also done.

+  if (new_display.id() == display.id() && display.id() != display_id_) {
+    display_id_ = display.id();
+    RecomputeAlignment(display);

[dewittj, 2016/06/20 17:06:00]

Lots of code duplication between these, could you please refactor into a helper
function?

[osandov, 2016/06/21 08:01:06]

Done.

+    DoUpdateIfPossible();
+  }
+}
 
 void DesktopPopupAlignmentDelegate::OnDisplayRemoved(
-    const display::Display& old_display) {}
+    const display::Display& old_display) {

[dewittj, 2016/06/20 17:06:00]

rename to removed_display

[osandov, 2016/06/21 08:01:06]

Done.

+  if (old_display.id() == display_id_) {
+    display::Display display = screen_->GetPrimaryDisplay();

[dewittj, 2016/06/20 17:05:59]

primary_display

[osandov, 2016/06/21 08:01:06]

Done, moved in the refactoring.

+    display_id_ = display.id();
+    RecomputeAlignment(display);
+    DoUpdateIfPossible();
+  }
+}
 
 void DesktopPopupAlignmentDelegate::OnDisplayMetricsChanged(
     const display::Display& display,
     uint32_t metrics) {
   if (display.id() == display_id_) {

[Jun Mukai, 2016/06/20 20:28:54]

Can it be renamed as |primary_display_id_|? That's more descriptive.

[osandov, 2016/06/21 08:02:40]

Done.

     RecomputeAlignment(display);
     DoUpdateIfPossible();
   }
 }
 
 }  // namespace message_center