[MacViews] Update AppWindow when the native window changes fullscreen state.

extensions/browser/app_window/app_window.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 "extensions/browser/app_window/app_window.h"
 
 #include <algorithm>
 #include <string>
 #include <vector>
 
@@ skipping 436 matching lines @@
     WebContentsModalDialogManager* modal_dialog_manager =
         WebContentsModalDialogManager::FromWebContents(web_contents());
     if (modal_dialog_manager)  // May be null in unit tests.
       modal_dialog_manager->SetDelegate(nullptr);
     app_window_contents_->NativeWindowClosed();
   }
   delete this;
 }
 
 void AppWindow::OnNativeWindowChanged() {
+#if defined(OS_MACOSX)

[tapted, 2015/06/10 05:45:25]

before this, I'd just do

  if (!native_app_window_)
    return;

AppWindow::SaveWindowPosition() does this also, so the call below isn't missing
out. Further, we can just turn the ckech in SaveWindowPosition() into a DCHECK
since the method is private and OnNativeWindowChanged() is the only caller.

In fact... I'm sorely tempted to just DCHECK here as well... the other calls to
OnNativeWindowChanged() in AppWindow are in functions that have already assumed
native_app_window is non-null. The other calls all seem to be from the native
app window itself :/. bleh, maybe we turn it into a CHECK() in a follow-up or do
some git archaeology to smack a comment about why we need to check if it's
non-null.

[jackhou1, 2015/06/10 07:52:32]

Done.

+  // On Mac the user can change the window's fullscreen state. If that has
+  // happened, update AppWindow's internal state.
+  if (native_app_window_) {
+    bool native_is_fullscreen = native_app_window_->IsFullscreen();
+    bool was_fullscreen = IsFullscreen();
+    if (native_is_fullscreen)
+      fullscreen_types_ |= FULLSCREEN_TYPE_OS;
+    else
+      fullscreen_types_ = FULLSCREEN_TYPE_NONE;
+
+    if (native_is_fullscreen != was_fullscreen && cached_always_on_top_)

[tapted, 2015/06/10 05:45:25]

I don't think the `native_is_fullscreen != was_fullscreen` adds anything.
UpdateNativeAlwaysOnTop() has it's own checking to ensure SetAlwaysOnTop(FOO) is
only called if it's not currently FOO. (and the check below for windows doesn't
try detect toggles).

Then this can just be

if (native_app_window_->IsFullscreen())
  fullscreen_types_ |= FULLSCREEN_TYPE_OS;
else
  fullscreen_types_ = FULLSCREEN_TYPE_NONE;

if (cached_always_on_top_)
  UpdateNativeAlwaysOnTop();  // Same as in SetNativeWindowFullscreen.



(there's probably some #ifdef simplifications too... but I'm more inclined to
leave the WIN codepaths alone)

[jackhou1, 2015/06/10 07:52:32]

Done.

+      UpdateNativeAlwaysOnTop();  // Same as in SetNativeWindowFullscreen.
+  }
+#endif
+
   SaveWindowPosition();
 
 #if defined(OS_WIN)
   if (native_app_window_ && cached_always_on_top_ && !IsFullscreen() &&
       !native_app_window_->IsMaximized() &&
       !native_app_window_->IsMinimized()) {
     UpdateNativeAlwaysOnTop();
   }
 #endif
 
@@ skipping 594 matching lines @@
         region.bounds.x(),
         region.bounds.y(),
         region.bounds.right(),
         region.bounds.bottom(),
         region.draggable ? SkRegion::kUnion_Op : SkRegion::kDifference_Op);
   }
   return sk_region;
 }
 
 }  // namespace extensions