Save window placement for App windows.

Save window placement for App windows.
Avoid App window growth on every window close-open cycle.
(Also fixes initial placement of App windows)

BUG=93996, 82690
TEST=Start chrome.exe with "--app=http://www.google.com/".
     Window should be placed properly every time.

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

[Satoshi.Matsuzaki, 2011-09-06 16:02:37]

Hello, could you please review this?
These issues seem to have been regressed at r84303.

[apavlov, 2011-09-06 16:10:27]

LGTM

[Satoshi.Matsuzaki, 2011-09-14 15:24:27]

@jianli:
Is this OK for you too?
If you don't have time, please let me know.
I'll ask Ben (OWNER of this file) for review.

@apavlov:
Thanks for your review (and ideas to fix BUG 82690).

P.S.
I'm not a committer, so I couldn't send this CL to try server.

[Satoshi.Matsuzaki, 2011-09-18 03:27:15]

Hello Ben, colud you please review/approve this?

[tfarina, 2011-09-18 13:22:11]

http://codereview.chromium.org/7739034/diff/1/chrome/browser/ui/views/frame/b...
File chrome/browser/ui/views/frame/browser_view.cc (right):

http://codereview.chromium.org/7739034/diff/1/chrome/browser/ui/views/frame/b...
chrome/browser/ui/views/frame/browser_view.cc:1640: &&
(browser_->ShouldSaveWindowPlacement() || browser_->is_app())) {
the && should be in the end of the previous line.

[Satoshi.Matsuzaki, 2011-09-18 18:44:37]

Thanks for review.

>
http://codereview.chromium.org/7739034/diff/1/chrome/browser/ui/views/frame/b...
> File chrome/browser/ui/views/frame/browser_view.cc (right):
> 
>
http://codereview.chromium.org/7739034/diff/1/chrome/browser/ui/views/frame/b...
> chrome/browser/ui/views/frame/browser_view.cc:1640: &&
> (browser_->ShouldSaveWindowPlacement() || browser_->is_app())) {
> the && should be in the end of the previous line.

Fixed.

[tfarina, 2011-09-19 19:07:07]

On 2011/09/14 15:24:27, Satoshi.Matsuzaki wrote:
> P.S.
> I'm not a committer, so I couldn't send this CL to try server.

I've sent it to the trybots, you should get an email when they finish.

[Ben Goodger (Google), 2011-09-19 20:28:01]

lgtm

[CWD, 2011-09-21 12:23:17]

On 2011/09/19 19:07:07, tfarina wrote:
> On 2011/09/14 15:24:27, Satoshi.Matsuzaki wrote:
> > P.S.
> > I'm not a committer, so I couldn't send this CL to try server.
> 
> I've sent it to the trybots, you should get an email when they finish.

It appears as if the Trybots reported failure?  How could this be for such a
simple patch?

It would be amazing to get this bug (87876, 93996 etc) fixed so I can use
Application Shortcuts again!

[Satoshi.Matsuzaki, 2011-09-21 13:33:48]

On 2011/09/19 19:07:07, tfarina wrote:
> I've sent it to the trybots, you should get an email when they finish.

Thanks for your help, Thiago.

I think tryserver may dislike an aged (r99708) diff patch.
I've tried to rebase to r102092, but lost my local changeset by mistake.
What should I do? Please advise me. (I can upload the same patch as new CL.)

[tfarina, 2011-09-21 13:37:34]

On 2011/09/21 13:33:48, Satoshi.Matsuzaki wrote:
> On 2011/09/19 19:07:07, tfarina wrote:
> > I've sent it to the trybots, you should get an email when they finish.
> 
> Thanks for your help, Thiago.
> 
> I think tryserver may dislike an aged (r99708) diff patch.
> I've tried to rebase to r102092, but lost my local changeset by mistake.
> What should I do? Please advise me. (I can upload the same patch as new CL.)

Did you try checking the commit checkbox, to land this through the commit queue?
If that doesn't work, I can land this manually for you.

[Satoshi.Matsuzaki, 2011-09-21 14:01:23]

On 2011/09/21 13:37:34, tfarina wrote:
> Did you try checking the commit checkbox, to land this through the commit
queue?
> If that doesn't work, I can land this manually for you.

Thank you for advise.
I've done it, and compiling is in progress in commit queue.

[commit-bot: I haz the power, 2011-09-21 15:50:20]

Change committed as 102108

[Satoshi.Matsuzaki, 2011-09-21 20:36:04]

Thanks everyone (especially to tfarina) for your help.

[frame, 2011-11-04 09:25:27]

The patch doesn't seem to restore the original functionality. When you open a
popup window inside a pinned application and close the popup window AFTER the
main window, the size of the popup window gets stored. This was not the case in
earlier Chrome versions.

[Ben Goodger (Google), 2011-11-07 20:47:21]

http://codereview.chromium.org/7739034/diff/8001/chrome/browser/ui/views/fram...
File chrome/browser/ui/views/frame/browser_view.cc (right):

http://codereview.chromium.org/7739034/diff/8001/chrome/browser/ui/views/fram...
chrome/browser/ui/views/frame/browser_view.cc:1640:
(browser_->ShouldSaveWindowPlacement() || browser_->is_app())) {
BTW, upon further consideration I realize this is not the right place to have
this check. It's not really view-state, and as such should not live in
BrowserView, rather it should live in Browser::ShouldSaveWindowPlacement. Can
you fix this? Thanks :-)

[Satoshi.Matsuzaki, 2011-11-08 19:15:25]

On 2011/11/07 20:47:21, Ben Goodger (Google) wrote:
> BTW, upon further consideration I realize this is not the right place to have
> this check. It's not really view-state, and as such should not live in
> BrowserView, rather it should live in Browser::ShouldSaveWindowPlacement. Can
> you fix this? Thanks :-)

I agree with your idea. Thanks for pointing out.
Filed http://crbug.com/103403 for tracking purpose.
I just started investigation, so anyone please feel free to take it.