Enhance chrome.app.window API with better shelf integration

extensions/browser/app_window/app_window.h

 // 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.
 
 #ifndef EXTENSIONS_BROWSER_APP_WINDOW_APP_WINDOW_H_
 #define EXTENSIONS_BROWSER_APP_WINDOW_APP_WINDOW_H_
 
 #include <stdint.h>
 
 #include <memory>
@@ skipping 173 matching lines @@
     // If true, the window will be focused on creation. Defaults to true.
     bool focused;
 
     // If true, the window will stay on top of other windows that are not
     // configured to be always on top. Defaults to false.
     bool always_on_top;
 
     // If true, the window will be visible on all workspaces. Defaults to false.
     bool visible_on_all_workspaces;
 
+    // If true, the window will be tied to its own icon in the shelf instead of
+    // being folded in the menu of the parent app. Defaults to false.

[stevenjb, 2016/04/26 16:30:01]

"folded in the menu of the parent app" isn't clear. If all windows have
show_in_shelf = true, is there a "parent app menu"? I think the logic would be
more clear as:

"If true, the window will have its own shelf icon. Otherwise the window will be
grouped in the shelf with other windows associated with the app."

[Andra Paraschiv, 2016/04/28 09:49:03]

Done.

+    bool show_in_shelf;
+
     // The API enables developers to specify content or window bounds. This
     // function combines them into a single, constrained window size.
     gfx::Rect GetInitialWindowBounds(const gfx::Insets& frame_insets) const;
 
     // The API enables developers to specify content or window size constraints.
     // These functions combine them so that we can work with one set of
     // constraints.
     gfx::Size GetContentMinimumSize(const gfx::Insets& frame_insets) const;
     gfx::Size GetContentMaximumSize(const gfx::Insets& frame_insets) const;
     gfx::Size GetWindowMinimumSize(const gfx::Insets& frame_insets) const;
@@ skipping 157 matching lines @@
 
   // Whether the app window wants to be alpha enabled.
   bool requested_alpha_enabled() const { return requested_alpha_enabled_; }
 
   // Whether the app window is created by IME extensions.
   // TODO(bshe): rename to hide_app_window_in_launcher if it is not used
   // anywhere other than app_window_launcher_controller after M45. Otherwise,
   // remove this TODO.
   bool is_ime_window() const { return is_ime_window_; }
 
+  // Whether the app window will create its own icon in the shelf.

[stevenjb, 2016/04/26 16:30:01]

Trivial getters shouldn't have comments, instead the variable itself should be
commented (which it is). You can also remove he comment for
requested_alpha_enabled() (but leave the TODO for is_ime_window(), that should
be addressed separately).

[Andra Paraschiv, 2016/04/28 09:49:03]

Done.

+  bool show_in_shelf() const { return show_in_shelf_; }
+
   void SetAppWindowContentsForTesting(
       std::unique_ptr<AppWindowContents> contents) {
     app_window_contents_ = std::move(contents);
   }
 
  protected:
   ~AppWindow() override;
 
  private:
   // PlatformAppBrowserTest needs access to web_contents()
@@ skipping 179 matching lines @@
   // reinstated when the window exits fullscreen and moves away from the
   // taskbar.
   bool cached_always_on_top_;
 
   // Whether |alpha_enabled| was set in the CreateParams.
   bool requested_alpha_enabled_;
 
   // Whether |is_ime_window| was set in the CreateParams.
   bool is_ime_window_;
 
+  // Whether |show_in_shelf| was set in the CreateParams.
+  bool show_in_shelf_;
+
   // PlzNavigate: this is called when the first navigation is ready to commit.
   base::Closure on_first_commit_callback_;
 
   base::WeakPtrFactory<AppWindow> image_loader_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(AppWindow);
 };
 
 }  // namespace extensions
 
 #endif  // EXTENSIONS_BROWSER_APP_WINDOW_APP_WINDOW_H_