First part of remembering platform app window geometry.

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 da858acec0c294be43baf41133ee3a0245abfac7..5146ea18c056597119d7458e7e310409833d6ccb 100644
--- a/chrome/browser/ui/extensions/shell_window.cc
+++ b/chrome/browser/ui/extensions/shell_window.cc
@@ -6,6 +6,8 @@
 
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/extensions/extension_process_manager.h"
+#include "chrome/browser/extensions/extension_system.h"
+#include "chrome/browser/extensions/shell_window_geometry_cache.h"
 #include "chrome/browser/extensions/shell_window_registry.h"
 #include "chrome/browser/file_select_helper.h"
 #include "chrome/browser/infobars/infobar_tab_helper.h"
@@ -63,7 +65,11 @@ void SuspendRenderViewHost(RenderViewHost* rvh) {
 
 ShellWindow::CreateParams::CreateParams()
   : frame(ShellWindow::CreateParams::FRAME_CHROME),
-    bounds(10, 10, kDefaultWidth, kDefaultHeight) {
+    bounds(-1, -1, kDefaultWidth, kDefaultHeight),
+    restore_position(true), restore_size(true) {
+}
+
+ShellWindow::CreateParams::~CreateParams() {
 }
 
 ShellWindow* ShellWindow::Create(Profile* profile,
@@ -101,6 +107,29 @@ void ShellWindow::Init(const GURL& url,
 
   native_window_.reset(NativeShellWindow::Create(this, params));
 
+  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)) {

[jeremya, 2012/08/30 04:49:33]

nit: indentation

[Marijn Kruisselbrink, 2012/08/30 17:15:05]

Done.

+        gfx::Rect bounds = native_window_->GetBounds();
+
+        if (params.restore_position)
+          bounds.set_origin(cached_bounds.origin());
+        if (params.restore_size)
+          bounds.set_size(cached_bounds.size());
+
+        native_window_->SetBounds(bounds);

[jeremya, 2012/08/30 04:49:33]

Will this set the bounds after the window has already been shown? Perhaps we
should fetch this before creating the window and update the create params with
the restored bounds.

[Marijn Kruisselbrink, 2012/08/30 17:15:05]

In theory I guess this could indeed result in the window moving after it has
been shown (although I haven't noticed such behavior). There is a problem with
just passing updated bounds to NativeShellWindow::Create though: most
NativeShellWindow implementations interpret bounds passed in the create params
differently from those passed to SetBounds (which is why if you look at the OSX
specific bits, I even added a call to SetBounds in the case where there are no
coordinates to be restored
(https://chromiumcodereview.appspot.com/10893020/diff/1/chrome/browser/ui/exte....
I suppose these inconsistency should probably be fixed, but that requires to
first know what the expected behavior is (with regard to size&position including
or not including potential window chrome).

If updating the create params would be the way to go though, would it be better
to pass
params by value to ShellWindow::Init, or make a copy inside the method (I
suspect the second way is the preferred method).

[jeremya, 2012/08/31 04:36:12]

Yes, this should be fixed :) (please add a TODO here so we don't forget).

The desired behaviour for chrome.app.window.create() w.r.t. the dimensions is
that the width/height passed in should refer to the innerWidth/innerHeight of
the created window. And obviously if you create a window and then set its bounds
to exactly the size you told it to be in create(), it shouldn't change size :)

Non-frameless windows will be going away soon (in favour of HTML titlebars, see
http://codereview.chromium.org/10896032/).
Hm, all the options seem a little bit gross here. I think the grossness comes
from the fact that we use CreateParams to get data from the extension API call
into ShellWindow::Init as well as to pass data into the NativeShellWindow
constructor.

Once the chrome-style frame disappears, the only things the native windows will
need access to from the CreateParams are the initial bounds and the min/max
size. Perhaps we could just pass in the initial bounds to the NativeShellWindow
constructor, then set the min/max size with a method?

+      }
+    }
+  }
+
+
   // Block the created RVH from loading anything until the background page
   // has had a chance to do any initialization it wants.
   SuspendRenderViewHost(web_contents_->GetRenderViewHost());
@@ -376,3 +405,17 @@ void ShellWindow::AddMessageToDevToolsConsole(ConsoleMessageLevel level,
   rvh->Send(new ExtensionMsg_AddMessageToConsole(
       rvh->GetRoutingID(), level, message));
 }
+
+void ShellWindow::SaveWindowPosition()
+{
+  if (window_key_.empty())
+    return;
+
+  extensions::ShellWindowGeometryCache* cache =
+      extensions::ExtensionSystem::Get(profile())->
+        shell_window_geometry_cache();

[jeremya, 2012/08/30 04:49:33]

nit: 4 spaces here too, i believe

[Marijn Kruisselbrink, 2012/08/30 17:15:05]

Yeah, I wasn't sure about indentation here. Is that 4 spaces as in 2 more, or 2
less?

[jeremya, 2012/08/31 04:36:12]

I meant 2 more :)

+
+  gfx::Rect bounds = native_window_->GetBounds();
+  cache->SaveGeometry(extension()->id(), window_key_, bounds);
+}
+