Do not send the OnWindowShowTypeChanged() notification if the show type has not changed

ash/wm/window_state.cc

Index: ash/wm/window_state.cc
diff --git a/ash/wm/window_state.cc b/ash/wm/window_state.cc
index 284a2ee095306b04155653d14f7c772a66d7169a..27d751c182dd2a2f795c942e956b29739480e787 100644
--- a/ash/wm/window_state.cc
+++ b/ash/wm/window_state.cc
@@ -13,6 +13,7 @@
 #include "ash/wm/window_state_observer.h"
 #include "ash/wm/window_util.h"
 #include "ash/wm/wm_types.h"
+#include "base/auto_reset.h"
 #include "base/command_line.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/aura/window.h"
@@ -43,6 +44,7 @@ WindowState::WindowState(aura::Window* window)
       hide_shelf_when_fullscreen_(true),
       animate_to_fullscreen_(true),
       minimum_visibility_(false),
+      in_set_window_show_type_(false),
       window_show_type_(ToWindowShowType(GetShowState())) {
   window_->AddObserver(this);
 
@@ -179,9 +181,6 @@ void WindowState::Deactivate() {
 }
 
 void WindowState::Restore() {

[pkotwicz, 2013/12/09 22:29:03]

I removed this code because it is not helpful. In particular, it does not get
trigged via views::Widget::Restore().

-  // Set |window_show_type_| to SHOW_TYPE_NORMAL now so that an observer
-  // observing kShowStateKey gets the correct value when querying IsSnapped().
-  window_show_type_ = SHOW_TYPE_NORMAL;
   window_->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_NORMAL);
 }
 
@@ -262,19 +261,17 @@ void WindowState::OnWindowPropertyChanged(aura::Window* window,
                                           const void* key,
                                           intptr_t old) {
   DCHECK_EQ(window, window_);
-  if (key == aura::client::kShowStateKey) {
-    window_show_type_ = ToWindowShowType(GetShowState());
-    ui::WindowShowState old_state = static_cast<ui::WindowShowState>(old);
-    // TODO(oshima): Notify only when the state has changed.

[pkotwicz, 2013/12/09 22:29:03]

I actually did not find any tests which are broken because of this

-    // Doing so break a few tests now.
-    FOR_EACH_OBSERVER(
-        WindowStateObserver, observer_list_,
-        OnWindowShowTypeChanged(this, ToWindowShowType(old_state)));
-  }
+  if (key == aura::client::kShowStateKey)
+    SetWindowShowType(ToWindowShowType(GetShowState()));
 }
 
 void WindowState::SnapWindow(WindowShowType left_or_right,
                              const gfx::Rect& bounds) {
+  if (window_show_type_ == left_or_right) {
+    window_->SetBounds(bounds);
+    return;
+  }
+
   // Compute the bounds that the window will restore to. If the window does not
   // already have restore bounds, it will be restored (when un-snapped) to the
   // last bounds that it had before getting snapped.
@@ -284,20 +281,9 @@ void WindowState::SnapWindow(WindowShowType left_or_right,
   // which width to use when the snapped window is moved to the edge.
   SetRestoreBoundsInParent(bounds);
 
-  bool was_maximized = IsMaximizedOrFullscreen();
-  // Before we can set the bounds we need to restore the window.
-  // Restoring the window will set the window to its restored bounds set above.
-  // Restore will cause OnWindowPropertyChanged() so it needs to be done
-  // before notifying that the WindowShowType has changed to |left_or_right|.
-  if (was_maximized)
-    Restore();
   DCHECK(left_or_right == SHOW_TYPE_LEFT_SNAPPED ||
          left_or_right == SHOW_TYPE_RIGHT_SNAPPED);
-  WindowShowType old_type = window_show_type_;
-  window_show_type_ = left_or_right;
-  FOR_EACH_OBSERVER(
-      WindowStateObserver, observer_list_,
-      OnWindowShowTypeChanged(this, old_type));
+  SetWindowShowType(left_or_right);
   // TODO(varkha): Ideally the bounds should be changed in a LayoutManager upon
   // observing the WindowShowType change.
   // If the window is a child of kShellWindowId_DockedContainer such as during
@@ -310,6 +296,23 @@ void WindowState::SnapWindow(WindowShowType left_or_right,
   SetRestoreBoundsInScreen(restore_bounds_in_screen);
 }
 
+void WindowState::SetWindowShowType(WindowShowType new_window_show_type) {
+  if (in_set_window_show_type_)
+    return;
+  base::AutoReset<bool>(&in_set_window_show_type_, true);
+
+  ui::WindowShowState new_window_state =
+       ToWindowShowState(new_window_show_type);
+  if (new_window_state != GetShowState())
+    window_->SetProperty(aura::client::kShowStateKey, new_window_state);
+  WindowShowType old_window_show_type = window_show_type_;
+  window_show_type_ = new_window_show_type;
+  if (old_window_show_type != window_show_type_) {
+    FOR_EACH_OBSERVER(WindowStateObserver, observer_list_,
+                      OnWindowShowTypeChanged(this, old_window_show_type));
+  }
+}
+
 WindowState* GetActiveWindowState() {
   aura::Window* active = GetActiveWindow();
   return active ? GetWindowState(active) : NULL;