Make WebWidget::resize early out if the size doesn't change.

Make WebWidget::resize early out if the size doesn't change.

This was already done in most of WebWidget's descendants except for
render_widget_fullscreen_pepper and WebWidgetPopupImpl. It makes more sense to
check this inside the call rather than relying on the caller
(RenderWidget::Resize in this case) and this simplifies some changes I'm making
here in another CL: https://codereview.chromium.org/1844013002/

WebWidgetPopupImpl turned out to be caching the window rect itself. I removed
this cached variable and let it just query the RenderWidget's rect instead. In
doing this, I believe RenderWidget has a bug in how it handles setWindowRect.
It caches the given rect and uses that when queried until a Move ACK comes back
from the browser host at which point it starts returning the view_screen_rect_
again, but it never sets the view_screen_rect_. I now set the view_screen_rect_
to the pending_window_rect_ once we get the last Move ACK.

BUG=600507
Committed: https://crrev.com/71cb5b185f274ee0f96b346ce8a515073e9e2975
Cr-Commit-Position: refs/heads/master@{#389984}

[bokan, 2016-04-18 23:33:04]

The CQ bit was checked by bokan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-18 23:33:33]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1894333002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1894333002/1

[commit-bot: I haz the power, 2016-04-19 01:25:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-19 01:25:31]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[bokan, 2016-04-21 00:47:31]

The CQ bit was checked by bokan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-21 00:47:47]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1894333002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1894333002/60001

[commit-bot: I haz the power, 2016-04-21 01:32:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-21 01:32:10]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[bokan, 2016-04-21 22:02:44]

Description was changed from

==========
Make WebWidget::resize early out if the size doesn't change.

This was already done in most of WebWidget's descendants except for
render_widget_fullscreen_pepper and WebWidgetPopupImpl. It makes more sense to
check this inside the call rather than relying on the caller and this simplifies
some changes I'm making here in another CL.

BUG=600507
==========

to

==========
Make WebWidget::resize early out if the size doesn't change.

This was already done in most of WebWidget's descendants except for
render_widget_fullscreen_pepper and WebWidgetPopupImpl. It makes more sense to
check this inside the call rather than relying on the caller
(RenderWidget::Resize in this case) and this simplifies some changes I'm making
here in another CL: https://codereview.chromium.org/1844013002/

WebWidgetPopupImpl turned out to be caching the window rect itself. I removed
this cached variable and let it just query the RenderWidget's rect instead. In
doing this, I believe RenderWidget has a bug in how it handles setWindowRect.
It caches the given rect and uses that when queried until a Move ACK comes back
from the browser host at which point it starts returning the view_screen_rect_
again, but it never sets the view_screen_rect_. I now set the view_screen_rect_
to the pending_window_rect_ once we get the last Move ACK.

BUG=600507
==========

[bokan, 2016-04-21 22:02:44]

bokan@chromium.org changed reviewers:
+ sievers@chromium.org

[bokan, 2016-04-21 22:04:44]

Hey sievers@, here's the cleanup I decided to split off before landing
https://codereview.chromium.org/1844013002/

https://codereview.chromium.org/1894333002/diff/60001/content/renderer/render...
File content/renderer/render_widget.cc (right):

https://codereview.chromium.org/1894333002/diff/60001/content/renderer/render...
content/renderer/render_widget.cc:692: view_screen_rect_ = pending_window_rect_;
In making the changes to WebPagePopupImpl, I noticed that when we setWindowRect,
we set pending_window_rect_ to the given rect. Then in windowRect we return the
pending_window_rect_ until we get back a MoveAck and thereafter we return the
view_screen_rect_. However, we never actually update the view_screen_rect_!

This code is quite long lived (last touched by Darin, in fact) so I'm surprised
to have found such a glaring bug, perhaps I just don't understand the whole
picture. Are you familiar with this code or know someone who is? Maybe it's just
used for popups, and the popup kept it's own copy of the rect until now so it
didn't come up as a problem...

https://codereview.chromium.org/1894333002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebPagePopupImpl.cpp (right):

https://codereview.chromium.org/1894333002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebPagePopupImpl.cpp:433:
m_widgetClient->didInvalidateRect(WebRect(0, 0, newSize.width, newSize.height));
The FrameView and VisualViewport calls above have their own short circuits if
the size isn't changed. The only difference here is we might invalidate in cases
where we might not need to. We could cache the size and avoid that but I don't
see this as a problem worth adding complexity for, we're not likely to get a
large number of resizes with the same size. In this case, we'd gratuitously
invalidate sometimes when going in/out of fullscreen and such events but that
seems ok.

In any case, if you're ok with the change I'll loop in someone who owns this
code to make sure.

[no sievers, 2016-04-22 21:58:48]

content looks fine, but you need a blink reviewer

[bokan, 2016-04-25 13:12:27]

bokan@chromium.org changed reviewers:
+ tkent@chromium.org

[bokan, 2016-04-25 13:12:28]

+tkent@, looks like you have at least some familiarity with this code. Could you
take a look or add someone more appropriate? Thanks.

[tkent, 2016-04-25 23:05:36]

third_party/WebKit lgtm

[bokan, 2016-04-26 21:59:29]

sievers@, WebKit is stamped, ptal.

[no sievers, 2016-04-26 23:21:47]

On 2016/04/26 21:59:29, bokan wrote:
> sievers@, WebKit is stamped, ptal.

lgtm

[bokan, 2016-04-27 01:51:19]

The CQ bit was checked by bokan@chromium.org

[bokan, 2016-04-27 01:51:19]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org,
sievers@chromium.org
Link to the patchset: https://codereview.chromium.org/1894333002/#ps80001
(title: "Rebase")

[commit-bot: I haz the power, 2016-04-27 01:51:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1894333002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1894333002/80001

[commit-bot: I haz the power, 2016-04-27 03:45:31]

Description was changed from

==========
Make WebWidget::resize early out if the size doesn't change.

This was already done in most of WebWidget's descendants except for
render_widget_fullscreen_pepper and WebWidgetPopupImpl. It makes more sense to
check this inside the call rather than relying on the caller
(RenderWidget::Resize in this case) and this simplifies some changes I'm making
here in another CL: https://codereview.chromium.org/1844013002/

WebWidgetPopupImpl turned out to be caching the window rect itself. I removed
this cached variable and let it just query the RenderWidget's rect instead. In
doing this, I believe RenderWidget has a bug in how it handles setWindowRect.
It caches the given rect and uses that when queried until a Move ACK comes back
from the browser host at which point it starts returning the view_screen_rect_
again, but it never sets the view_screen_rect_. I now set the view_screen_rect_
to the pending_window_rect_ once we get the last Move ACK.

BUG=600507
==========

to

==========
Make WebWidget::resize early out if the size doesn't change.

This was already done in most of WebWidget's descendants except for
render_widget_fullscreen_pepper and WebWidgetPopupImpl. It makes more sense to
check this inside the call rather than relying on the caller
(RenderWidget::Resize in this case) and this simplifies some changes I'm making
here in another CL: https://codereview.chromium.org/1844013002/

WebWidgetPopupImpl turned out to be caching the window rect itself. I removed
this cached variable and let it just query the RenderWidget's rect instead. In
doing this, I believe RenderWidget has a bug in how it handles setWindowRect.
It caches the given rect and uses that when queried until a Move ACK comes back
from the browser host at which point it starts returning the view_screen_rect_
again, but it never sets the view_screen_rect_. I now set the view_screen_rect_
to the pending_window_rect_ once we get the last Move ACK.

BUG=600507
==========

[commit-bot: I haz the power, 2016-04-27 03:45:32]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-04-27 03:46:41]

Description was changed from

==========
Make WebWidget::resize early out if the size doesn't change.

This was already done in most of WebWidget's descendants except for
render_widget_fullscreen_pepper and WebWidgetPopupImpl. It makes more sense to
check this inside the call rather than relying on the caller
(RenderWidget::Resize in this case) and this simplifies some changes I'm making
here in another CL: https://codereview.chromium.org/1844013002/

WebWidgetPopupImpl turned out to be caching the window rect itself. I removed
this cached variable and let it just query the RenderWidget's rect instead. In
doing this, I believe RenderWidget has a bug in how it handles setWindowRect.
It caches the given rect and uses that when queried until a Move ACK comes back
from the browser host at which point it starts returning the view_screen_rect_
again, but it never sets the view_screen_rect_. I now set the view_screen_rect_
to the pending_window_rect_ once we get the last Move ACK.

BUG=600507
==========

to

==========
Make WebWidget::resize early out if the size doesn't change.

This was already done in most of WebWidget's descendants except for
render_widget_fullscreen_pepper and WebWidgetPopupImpl. It makes more sense to
check this inside the call rather than relying on the caller
(RenderWidget::Resize in this case) and this simplifies some changes I'm making
here in another CL: https://codereview.chromium.org/1844013002/

WebWidgetPopupImpl turned out to be caching the window rect itself. I removed
this cached variable and let it just query the RenderWidget's rect instead. In
doing this, I believe RenderWidget has a bug in how it handles setWindowRect.
It caches the given rect and uses that when queried until a Move ACK comes back
from the browser host at which point it starts returning the view_screen_rect_
again, but it never sets the view_screen_rect_. I now set the view_screen_rect_
to the pending_window_rect_ once we get the last Move ACK.

BUG=600507

Committed: https://crrev.com/71cb5b185f274ee0f96b346ce8a515073e9e2975
Cr-Commit-Position: refs/heads/master@{#389984}
==========

[commit-bot: I haz the power, 2016-04-27 03:46:42]

Patchset 5 (id:??) landed as
https://crrev.com/71cb5b185f274ee0f96b346ce8a515073e9e2975
Cr-Commit-Position: refs/heads/master@{#389984}