Add autoresize capability to chromium.

Add autoresize capability to chromium.

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

[darin (slow to review), 2011-11-28 16:53:54]

http://codereview.chromium.org/8704005/diff/1/content/renderer/render_view_im...
File content/renderer/render_view_impl.cc (right):

http://codereview.chromium.org/8704005/diff/1/content/renderer/render_view_im...
content/renderer/render_view_impl.cc:1748: Send(new
ViewHostMsg_DidContentsPreferredSizeChange(routing_id_,
nit: DidChangeContentsPreferredSize or ContentsPreferredSizeChanged

DidContentsPreferredSizeChange sounds like it is asking a question, but that
isn't a question the browser process can answer.

[jennb, 2011-11-28 18:02:24]

Drive-by...

I"ll wait for this patch to land before landing my Panel class refactor. I'm
adding some calls to the RVH to enable auto size when min/max size change. I'd
rather code to this new auto resize API.

http://codereview.chromium.org/8704005/diff/1/chrome/browser/ui/panels/panel.cc
File chrome/browser/ui/panels/panel.cc (left):

http://codereview.chromium.org/8704005/diff/1/chrome/browser/ui/panels/panel....
chrome/browser/ui/panels/panel.cc:626: void Panel::OnWindowSizeAvailable() {
We still need this because on GTK, ContentSizeFromWindowSize() will return the
incorrect value until the window size has stabilized.

http://codereview.chromium.org/8704005/diff/1/content/renderer/render_view_im...
File content/renderer/render_view_impl.cc (right):

http://codereview.chromium.org/8704005/diff/1/content/renderer/render_view_im...
content/renderer/render_view_impl.cc:1748: Send(new
ViewHostMsg_DidContentsPreferredSizeChange(routing_id_,
Maybe move the verb right after Did, i.e. DidChangeContentsPreferredSize?

[levin, 2011-11-28 18:44:56]

http://codereview.chromium.org/8704005/diff/1/content/renderer/render_view_im...
File content/renderer/render_view_impl.cc (right):

http://codereview.chromium.org/8704005/diff/1/content/renderer/render_view_im...
content/renderer/render_view_impl.cc:1748: Send(new
ViewHostMsg_DidContentsPreferredSizeChange(routing_id_,
On 2011/11/28 16:53:55, darin wrote:
> nit: DidChangeContentsPreferredSize or ContentsPreferredSizeChanged
> 
> DidContentsPreferredSizeChange sounds like it is asking a question, but that
> isn't a question the browser process can answer.

I was re-using an existing message (which I think follows a pattern of
"ViewHostMsg_Did*" to indicate an event) but I can see how the name may be
confusing. I will create a new message for my case since the view host may want
to handle it differently.

[darin (slow to review), 2011-11-30 23:30:59]

http://codereview.chromium.org/8704005/diff/13001/content/browser/renderer_ho...
File content/browser/renderer_host/render_view_host.cc (right):

http://codereview.chromium.org/8704005/diff/13001/content/browser/renderer_ho...
content/browser/renderer_host/render_view_host.cc:1347: setShouldAutoSize(true);
nit: setShould -> SetShould

http://codereview.chromium.org/8704005/diff/13001/content/renderer/render_wid...
File content/renderer/render_widget.cc (right):

http://codereview.chromium.org/8704005/diff/13001/content/renderer/render_wid...
content/renderer/render_widget.cc:264: size_ = new_size;
following this call, we should expect to get invalidations, right?  and those
trigger the ViewHostMsg_UpdateRect message.  SGTM.

[levin, 2011-12-01 20:18:06]

All fixed up.

Also, I removed the panel changes from this patch because I think it makes sense
for a number of reasons to actually switch panels to using this as a different
change.

http://codereview.chromium.org/8704005/diff/13001/content/browser/renderer_ho...
File content/browser/renderer_host/render_view_host.cc (right):

http://codereview.chromium.org/8704005/diff/13001/content/browser/renderer_ho...
content/browser/renderer_host/render_view_host.cc:1347: setShouldAutoSize(true);
On 2011/11/30 23:31:00, darin wrote:
> nit: setShould -> SetShould

Done.

http://codereview.chromium.org/8704005/diff/13001/content/renderer/render_wid...
File content/renderer/render_widget.cc (right):

http://codereview.chromium.org/8704005/diff/13001/content/renderer/render_wid...
content/renderer/render_widget.cc:264: size_ = new_size;
On 2011/11/30 23:31:00, darin wrote:
> following this call, we should expect to get invalidations, right?  and those
> trigger the ViewHostMsg_UpdateRect message.  SGTM.

Yes.

[darin (slow to review), 2011-12-02 05:48:24]

LGTM

http://codereview.chromium.org/8704005/diff/21003/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host.cc (right):

http://codereview.chromium.org/8704005/diff/21003/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host.cc:883: should_auto_size_ =
true;
perhaps you should also add some assertions that when we are in auto-size mode
that we don't try to resize the RenderWidgetHost directly?

http://codereview.chromium.org/8704005/diff/21003/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host.h (right):

http://codereview.chromium.org/8704005/diff/21003/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host.h:518: void
SetShouldAutoSize(bool enable);
elsewhere, you say "auto-resize", here you say "auto-size"... be consistent?

http://codereview.chromium.org/8704005/diff/21003/content/common/view_messages.h
File content/common/view_messages.h (right):

http://codereview.chromium.org/8704005/diff/21003/content/common/view_message...
content/common/view_messages.h:1025:
IPC_MESSAGE_ROUTED2(ViewMsg_EnableAutoResize,
nit: auto-resize or auto-size?

http://codereview.chromium.org/8704005/diff/21003/content/renderer/render_wid...
File content/renderer/render_widget.cc (right):

http://codereview.chromium.org/8704005/diff/21003/content/renderer/render_wid...
content/renderer/render_widget.cc:927: void RenderWidget::didAutoResize(const
WebSize& new_size) {
although i was complaining about auto-size vs auto-resize, in this case
auto-resize makes sense.  maybe that is because this is phrased in the past
tense, but saying didAutoSize just doesn't seem to work.  yet, referring to it
as the autoSizeMode does work.  hmm...

[levin, 2011-12-02 17:17:44]

On 2011/12/02 05:48:24, darin wrote:
> LGTM
> 
>
http://codereview.chromium.org/8704005/diff/21003/content/browser/renderer_ho...
> File content/browser/renderer_host/render_widget_host.cc (right):
> 
>
http://codereview.chromium.org/8704005/diff/21003/content/browser/renderer_ho...
> content/browser/renderer_host/render_widget_host.cc:883: should_auto_size_ =
> true;
> perhaps you should also add some assertions that when we are in auto-size mode
> that we don't try to resize the RenderWidgetHost directly?

Didn't do it in this patch but will look at this for a future patch.



I went with resize everywhere. (Here and in the WebKit side.)