Make app windows restore position even if bounds are explicitly specified.

Make app windows restore position even if bounds are explicitly specified.

Previously, if bounds were specified, the window's bounds would not be
restored. This change essentially turns left/top/width/height into
defaultLeft/defaultTop/defaultWidth/defaultHeight.

In order to create a window with an ID at a particular location, create it
hidden and move it into position before showing it.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=168159

[Marijn Kruisselbrink, 2012-11-12 17:56:50]

LGTM

http://codereview.chromium.org/11362003/diff/1/chrome/browser/extensions/api/...
File chrome/browser/extensions/api/app_window/app_window_api.cc (right):

http://codereview.chromium.org/11362003/diff/1/chrome/browser/extensions/api/...
chrome/browser/extensions/api/app_window/app_window_api.cc:126: if
(options->width.get() || options->height.get()) {
nit: With the restore_size bit gone, there is no reason to keep this if wrapping
the width/height getting.

http://codereview.chromium.org/11362003/diff/1/chrome/browser/extensions/api/...
chrome/browser/extensions/api/app_window/app_window_api.cc:133: if
(options->left.get() || options->top.get()) {
same here, this outer if no longer has a purpose

[koz (OOO until 15th September), 2012-11-12 22:29:32]

lgtm

http://codereview.chromium.org/11362003/diff/1/chrome/common/extensions/api/a...
File chrome/common/extensions/api/app_window.idl (right):

http://codereview.chromium.org/11362003/diff/1/chrome/common/extensions/api/a...
chrome/common/extensions/api/app_window.idl:101: // opening of a window with the
same id. The defaultLeft/defaultTop and
This comment looks like it needs updating. Maybe mention how
left/top/width/height will be overridden by retained window positions and the
workaround you described in your comment.

[jeremya, 2012-11-12 23:11:16]

All done :)

[commit-bot: I haz the power, 2012-11-12 23:13:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jeremya@chromium.org/11362003/7

[commit-bot: I haz the power, 2012-11-12 23:13:05]

Failed to apply patch for chrome/browser/ui/extensions/shell_window.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/ui/extensions/shell_window.cc
  Hunk #1 FAILED at 76.
  1 out of 2 hunks FAILED -- saving rejects to file
chrome/browser/ui/extensions/shell_window.cc.rej

Patch:       chrome/browser/ui/extensions/shell_window.cc
Index: chrome/browser/ui/extensions/shell_window.cc
diff --git a/chrome/browser/ui/extensions/shell_window.cc
b/chrome/browser/ui/extensions/shell_window.cc
index
2c4e5a7a2777346692da0def1c592c7362d77924..46e709f5dbe6304691a37f31920e8cd4107b8eba
100644
--- a/chrome/browser/ui/extensions/shell_window.cc
+++ b/chrome/browser/ui/extensions/shell_window.cc
@@ -76,7 +76,6 @@ void SuspendRenderViewHost(RenderViewHost* rvh) {
 ShellWindow::CreateParams::CreateParams()
   : frame(ShellWindow::CreateParams::FRAME_CHROME),
     bounds(INT_MIN, INT_MIN, INT_MIN, INT_MIN),
-    restore_position(true), restore_size(true),
     creator_process_id(0), hidden(false) {
 }
 
@@ -133,19 +132,13 @@ void ShellWindow::Init(const GURL& url,
   if (!params.window_key.empty()) {
     window_key_ = params.window_key;
 
-    if (params.restore_position || params.restore_size) {
-      extensions::ShellWindowGeometryCache* cache =
-          extensions::ExtensionSystem::Get(profile())->
-            shell_window_geometry_cache();
-      gfx::Rect cached_bounds;
-      if (cache->GetGeometry(extension()->id(), params.window_key,
-                             &cached_bounds)) {
-        if (params.restore_position)
-          bounds.set_origin(cached_bounds.origin());
-        if (params.restore_size)
-          bounds.set_size(cached_bounds.size());
-      }
-    }
+    extensions::ShellWindowGeometryCache* cache =
+        extensions::ExtensionSystem::Get(profile())->
+          shell_window_geometry_cache();
+    gfx::Rect cached_bounds;
+    if (cache->GetGeometry(extension()->id(), params.window_key,
+                           &cached_bounds))
+      bounds = cached_bounds;
   }
 
   ShellWindow::CreateParams new_params = params;

[commit-bot: I haz the power, 2012-11-12 23:19:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jeremya@chromium.org/11362003/7002

[commit-bot: I haz the power, 2012-11-12 23:19:35]

Failed to apply patch for chrome/browser/ui/extensions/shell_window.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/ui/extensions/shell_window.cc
  Hunk #1 FAILED at 76.
  1 out of 2 hunks FAILED -- saving rejects to file
chrome/browser/ui/extensions/shell_window.cc.rej

Patch:       chrome/browser/ui/extensions/shell_window.cc
Index: chrome/browser/ui/extensions/shell_window.cc
diff --git a/chrome/browser/ui/extensions/shell_window.cc
b/chrome/browser/ui/extensions/shell_window.cc
index
2c4e5a7a2777346692da0def1c592c7362d77924..46e709f5dbe6304691a37f31920e8cd4107b8eba
100644
--- a/chrome/browser/ui/extensions/shell_window.cc
+++ b/chrome/browser/ui/extensions/shell_window.cc
@@ -76,7 +76,6 @@ void SuspendRenderViewHost(RenderViewHost* rvh) {
 ShellWindow::CreateParams::CreateParams()
   : frame(ShellWindow::CreateParams::FRAME_CHROME),
     bounds(INT_MIN, INT_MIN, INT_MIN, INT_MIN),
-    restore_position(true), restore_size(true),
     creator_process_id(0), hidden(false) {
 }
 
@@ -133,19 +132,13 @@ void ShellWindow::Init(const GURL& url,
   if (!params.window_key.empty()) {
     window_key_ = params.window_key;
 
-    if (params.restore_position || params.restore_size) {
-      extensions::ShellWindowGeometryCache* cache =
-          extensions::ExtensionSystem::Get(profile())->
-            shell_window_geometry_cache();
-      gfx::Rect cached_bounds;
-      if (cache->GetGeometry(extension()->id(), params.window_key,
-                             &cached_bounds)) {
-        if (params.restore_position)
-          bounds.set_origin(cached_bounds.origin());
-        if (params.restore_size)
-          bounds.set_size(cached_bounds.size());
-      }
-    }
+    extensions::ShellWindowGeometryCache* cache =
+        extensions::ExtensionSystem::Get(profile())->
+          shell_window_geometry_cache();
+    gfx::Rect cached_bounds;
+    if (cache->GetGeometry(extension()->id(), params.window_key,
+                           &cached_bounds))
+      bounds = cached_bounds;
   }
 
   ShellWindow::CreateParams new_params = params;

[commit-bot: I haz the power, 2012-11-16 00:08:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jeremya@chromium.org/11362003/12001

[commit-bot: I haz the power, 2012-11-16 02:24:44]

Retried try job too often for step(s) browser_tests

[commit-bot: I haz the power, 2012-11-16 02:50:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jeremya@chromium.org/11362003/15003

[commit-bot: I haz the power, 2012-11-16 07:45:37]

Change committed as 168159