First part of remembering platform app window geometry.

chrome/browser/ui/extensions/shell_window.cc

 // Copyright (c) 2012 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 "chrome/browser/ui/extensions/shell_window.h"
 
 #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"
 #include "chrome/browser/intents/web_intents_util.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sessions/session_id.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
@@ skipping 37 matching lines @@
   BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
       base::Bind(&ResourceDispatcherHost::BlockRequestsForRoute,
           base::Unretained(ResourceDispatcherHost::Get()),
           rvh->GetProcess()->GetID(), rvh->GetRoutingID()));
 }
 
 }  // namespace
 
 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,
                                  const extensions::Extension* extension,
                                  const GURL& url,
                                  const ShellWindow::CreateParams& params) {
   // This object will delete itself when the window is closed.
   ShellWindow* window = new ShellWindow(profile, extension);
   window->Init(url, params);
   extensions::ShellWindowRegistry::Get(profile)->AddShellWindow(window);
@@ skipping 17 matching lines @@
   contents_.reset(TabContents::Factory::CreateTabContents(web_contents_));
   content::WebContentsObserver::Observe(web_contents_);
   web_contents_->SetDelegate(this);
   chrome::SetViewType(web_contents_, chrome::VIEW_TYPE_APP_SHELL);
   web_contents_->GetMutableRendererPrefs()->
       browser_handles_all_top_level_requests = true;
   web_contents_->GetRenderViewHost()->SyncRendererPrefs();
 
   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());
 
   // TODO(jeremya): there's a bug where navigating a web contents to an
   // extension URL causes it to create a new RVH and discard the old (perfectly
   // usable) one. To work around this, we watch for a RVH_CHANGED message from
   // the web contents (which will be sent during LoadURL) and suspend resource
   // requests on the new RVH to ensure that we block the new RVH from loading
   // anything. It should be okay to remove the NOTIFICATION_RVH_CHANGED
@@ skipping 255 matching lines @@
   extension_function_dispatcher_.Dispatch(params,
                                           web_contents_->GetRenderViewHost());
 }
 
 void ShellWindow::AddMessageToDevToolsConsole(ConsoleMessageLevel level,
                                               const std::string& message) {
   content::RenderViewHost* rvh = web_contents_->GetRenderViewHost();
   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);
+}
+