Fix minimize -> fullscreen transition.

remoting/webapp/crd/js/fullscreen_v2.js

 // 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.
 
 /**
  * @fileoverview
  * Full-screen implementation for apps v2, using chrome.AppWindow.
  */
 
 'use strict';
 
 /** @suppress {duplicate} */
 var remoting = remoting || {};
 
 /**
  * @constructor
  * @implements {remoting.Fullscreen}
  */
 remoting.FullscreenAppsV2 = function() {
   /**
-   * @type {boolean} True if the next onRestored event should cause callbacks
-   *     to be notified of a full-screen changed event. onRestored fires when
-   *     full-screen mode is exited and also when the window is restored from
-   *     being minimized; callbacks should not be notified of the latter.
+   * @type {boolean} True if the window is minimized. onRestored fires when the
+   *     the window transitions from minimized to any other state, but since we
+   *     only want transitions from full-screen to windowed to cause a callback,

[kelvinp, 2015/01/22 17:42:08]

s/cause/call

[Jamie, 2015/01/22 23:27:31]

Cause is acceptable here. Apart from anything else, "call a callback" is a
little too alliterative for my tastes, and my comment wouldn't right-align as
nicely :D

[kelvinp, 2015/01/23 00:14:59]

Acknowledged.

+   *     we must keep track of the minimized state of the window.
    * @private
    */
-  this.notifyCallbacksOnRestore_ = this.isActive();
+  this.isMinimized_ = chrome.app.window.current().isMinimized();
 
   /**
    * @type {string} Internal 'full-screen changed' event name
    * @private
    */
   this.kEventName_ = '_fullscreenchanged';
 
   /**
    * @type {base.EventSource}
    * @private
    */
   this.eventSource_ = new base.EventSource();
   this.eventSource_.defineEvents([this.kEventName_]);
 
   chrome.app.window.current().onFullscreened.addListener(
       this.onFullscreened_.bind(this));
   chrome.app.window.current().onRestored.addListener(
       this.onRestored_.bind(this));
+  chrome.app.window.current().onMinimized.addListener(
+      this.onMinimized_.bind(this));
 };
 
 remoting.FullscreenAppsV2.prototype.activate = function(
     fullscreen, opt_onDone) {
   if (opt_onDone) {
     if (this.isActive() == fullscreen) {
       opt_onDone();
     } else {
       /** @type {remoting.Fullscreen} */
       var that = this;
@@ skipping 28 matching lines @@
 };
 
 /**
  * @param {function(boolean=):void} callback
  */
 remoting.FullscreenAppsV2.prototype.removeListener = function(callback) {
   this.eventSource_.removeEventListener(this.kEventName_, callback);
 };
 
 remoting.FullscreenAppsV2.prototype.onFullscreened_ = function() {
-  this.notifyCallbacksOnRestore_ = true;
+  this.isMinimized_ = false;
   this.eventSource_.raiseEvent(this.kEventName_, true);
   document.body.classList.add('fullscreen');
 };
 
 remoting.FullscreenAppsV2.prototype.onRestored_ = function() {
-  document.body.classList.remove('fullscreen');
-  if (this.notifyCallbacksOnRestore_) {
+  if (!this.isMinimized_) {

[kelvinp, 2015/01/22 17:42:08]

According to 
OnRestored is called whenever the window is restored from being minimized or
maximized.
When the window transits from maximized to restored, this.isMinimized will be
false.  This may cause the callback to be notified unintentionally.  You may
also want to listen to maximized as well as minimized.

[Jamie, 2015/01/22 23:27:31]

That's a valid observation. I had a go at adding an isMaximized member as well,
but it's a lot harder to keep track of than isMinimized because it's not the
case that onRestored implies !maximized in the same way that it does for
minimized. It's also safe to invoke the callback without including that
logic--the worst-case scenario is that we tell the callback that full-screen is
not active when it already knew that. Contrast that with the bug this CL is
fixing, which caused the callback to be passed the wrong information.

I've added a comment to the code explaining why we don't track the maximized
state of the window. LMK if you think it needs more.

[kelvinp, 2015/01/23 00:14:59]

Maybe we can add a member variable isFullScreened to remember the state that we
last notified our observers.  In this way, our observers won't be notified twice
when isFullScreened is not changed.

[Jamie, 2015/01/23 00:44:16]

Done.

+    document.body.classList.remove('fullscreen');
     this.notifyCallbacksOnRestore_ = false;
     this.eventSource_.raiseEvent(this.kEventName_, false);
   }
+  this.isMinimized_ = false;
 };
+
+remoting.FullscreenAppsV2.prototype.onMinimized_ = function() {
+  this.isMinimized_ = true;
+};