Fix element visibility check for validation bubbles and SELECT popups.

Fix element visibility check for validation bubbles and SELECT popups.

- Do not use FrameView::boundsRect() for element visibility check
  It's scaled by minimum-scale. We should use VisualViewport::size().

- Introduce Element::visibleBoundsInViewport() to merge duplicated code in
  HTMLSelectElement and ValidationMessageClientImpl.

BUG=633457
Committed: https://crrev.com/48565e7338d3f8d0cbfdb082f51e609b7bfd4624
Cr-Commit-Position: refs/heads/master@{#410567}

[tkent, 2016-08-05 09:37:27]

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

[tkent, 2016-08-05 09:37:32]

Description was changed from

==========
Fix element visibility check for validation bubbles and SELECT popups.

BUG=
==========

to

==========
Fix element visibility check for validation bubbles and SELECT popups.

BUG=632561
==========

[commit-bot: I haz the power, 2016-08-05 09:37:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tkent, 2016-08-05 09:37:49]

Patchset #1 (id:1) has been deleted

[tkent, 2016-08-05 09:41:08]

Description was changed from

==========
Fix element visibility check for validation bubbles and SELECT popups.

BUG=632561
==========

to

==========
Fix element visibility check for validation bubbles and SELECT popups.

- Do not use FrameView::boundsRect() for element visibility check
  It's scaled by minimum-scale.

- Introduce Element::visibleBoundsInViewport() to merge duplicated code in
  HTMLSelectElement and ValidationMessageClientImpl.

BUG=632561
==========

[tkent, 2016-08-05 09:41:53]

Description was changed from

==========
Fix element visibility check for validation bubbles and SELECT popups.

- Do not use FrameView::boundsRect() for element visibility check
  It's scaled by minimum-scale.

- Introduce Element::visibleBoundsInViewport() to merge duplicated code in
  HTMLSelectElement and ValidationMessageClientImpl.

BUG=632561
==========

to

==========
Fix element visibility check for validation bubbles and SELECT popups.

- Do not use FrameView::boundsRect() for element visibility check
  It's scaled by minimum-scale. We should use VisualViewport::size().

- Introduce Element::visibleBoundsInViewport() to merge duplicated code in
  HTMLSelectElement and ValidationMessageClientImpl.

BUG=632561
==========

[commit-bot: I haz the power, 2016-08-05 11:21:06]

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

[commit-bot: I haz the power, 2016-08-05 11:21:07]

Dry run: This issue passed the CQ dry run.

[tkent, 2016-08-05 12:58:06]

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

[tkent, 2016-08-05 12:58:07]

bokan@, would you review this please?

[bokan, 2016-08-08 15:23:00]

Sorry for the delay. Change is lgtm. I just have some naming and test
suggestions.

https://codereview.chromium.org/2215333002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Element.h (right):

https://codereview.chromium.org/2215333002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Element.h:224: // viewport rectangle, in the
viewport coordinate. This function is used to
s/viewport/visual viewport

https://codereview.chromium.org/2215333002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Element.h:225: // show popups besides this
elment.
Nit: besides->beside

https://codereview.chromium.org/2215333002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Element.h:226: IntRect
visibleBoundsInViewport() const;
"viewport" is ambiguous. Please call this visibleBoundsInVisualViewport or just
boundsInVisualViewport

https://codereview.chromium.org/2215333002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/VisualViewportTest.cpp (right):

https://codereview.chromium.org/2215333002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/VisualViewportTest.cpp:1556:
EXPECT_FALSE(element->visibleBoundsInViewport().isEmpty());
Please add the converse of this check above before the setInitialFocus. i.e.
EXPECT_TRUE(element->visibleBoundsInViewport().isEmpty())

https://codereview.chromium.org/2215333002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/data/viewport-select.html (right):

https://codereview.chromium.org/2215333002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/data/viewport-select.html:4: <meta
name="viewport" content="minimum-scale=2.0">
The test would be more clear if you remove this meta tag and just use
webViewImpl->setPageScaleFactor(2.0f) in the test itself.

[tkent, 2016-08-09 01:26:16]

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

[commit-bot: I haz the power, 2016-08-09 01:26:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-09 01:33:27]

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

[commit-bot: I haz the power, 2016-08-09 01:33:28]

Dry run: Try jobs failed on following builders:
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)

[tkent, 2016-08-09 03:17:12]

Thank you for reviewing.

https://codereview.chromium.org/2215333002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Element.h (right):

https://codereview.chromium.org/2215333002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Element.h:224: // viewport rectangle, in the
viewport coordinate. This function is used to
On 2016/08/08 at 15:22:59, bokan wrote:
> s/viewport/visual viewport

Done.

https://codereview.chromium.org/2215333002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Element.h:225: // show popups besides this
elment.
On 2016/08/08 at 15:22:59, bokan wrote:
> Nit: besides->beside

Done.  Also, elment -> element

https://codereview.chromium.org/2215333002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Element.h:226: IntRect
visibleBoundsInViewport() const;
On 2016/08/08 at 15:22:59, bokan wrote:
> "viewport" is ambiguous. Please call this visibleBoundsInVisualViewport or
just boundsInVisualViewport

Done.

https://codereview.chromium.org/2215333002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/VisualViewportTest.cpp (right):

https://codereview.chromium.org/2215333002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/VisualViewportTest.cpp:1556:
EXPECT_FALSE(element->visibleBoundsInViewport().isEmpty());
On 2016/08/08 at 15:22:59, bokan wrote:
> Please add the converse of this check above before the setInitialFocus. i.e.
EXPECT_TRUE(element->visibleBoundsInViewport().isEmpty())

Done.  I improved the test scenario to make its purpose clearer.

https://codereview.chromium.org/2215333002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/data/viewport-select.html (right):

https://codereview.chromium.org/2215333002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/data/viewport-select.html:4: <meta
name="viewport" content="minimum-scale=2.0">
On 2016/08/08 at 15:23:00, bokan wrote:
> The test would be more clear if you remove this meta tag and just use
webViewImpl->setPageScaleFactor(2.0f) in the test itself.

We need to apply minimum-scale for crbug.com/632561, and
WebViewImpl::setDefaultPageScaleLimits(2.0, ...) didn't reproduce the problem.

[tkent, 2016-08-09 03:17:19]

The CQ bit was checked by tkent@chromium.org

[tkent, 2016-08-09 03:17:19]

The patchset sent to the CQ was uploaded after l-g-t-m from bokan@chromium.org
Link to the patchset: https://codereview.chromium.org/2215333002/#ps40001
(title: "Rename to visibleBoundsInVisualViewport(), etc.")

[commit-bot: I haz the power, 2016-08-09 03:17:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tkent, 2016-08-09 03:17:59]

Description was changed from

==========
Fix element visibility check for validation bubbles and SELECT popups.

- Do not use FrameView::boundsRect() for element visibility check
  It's scaled by minimum-scale. We should use VisualViewport::size().

- Introduce Element::visibleBoundsInViewport() to merge duplicated code in
  HTMLSelectElement and ValidationMessageClientImpl.

BUG=632561
==========

to

==========
Fix element visibility check for validation bubbles and SELECT popups.

- Do not use FrameView::boundsRect() for element visibility check
  It's scaled by minimum-scale. We should use VisualViewport::size().

- Introduce Element::visibleBoundsInViewport() to merge duplicated code in
  HTMLSelectElement and ValidationMessageClientImpl.

BUG=633457
==========

[commit-bot: I haz the power, 2016-08-09 03:43:33]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-08-09 03:45:55]

Description was changed from

==========
Fix element visibility check for validation bubbles and SELECT popups.

- Do not use FrameView::boundsRect() for element visibility check
  It's scaled by minimum-scale. We should use VisualViewport::size().

- Introduce Element::visibleBoundsInViewport() to merge duplicated code in
  HTMLSelectElement and ValidationMessageClientImpl.

BUG=633457
==========

to

==========
Fix element visibility check for validation bubbles and SELECT popups.

- Do not use FrameView::boundsRect() for element visibility check
  It's scaled by minimum-scale. We should use VisualViewport::size().

- Introduce Element::visibleBoundsInViewport() to merge duplicated code in
  HTMLSelectElement and ValidationMessageClientImpl.

BUG=633457

Committed: https://crrev.com/48565e7338d3f8d0cbfdb082f51e609b7bfd4624
Cr-Commit-Position: refs/heads/master@{#410567}
==========

[commit-bot: I haz the power, 2016-08-09 03:45:57]

Patchset 2 (id:??) landed as
https://crrev.com/48565e7338d3f8d0cbfdb082f51e609b7bfd4624
Cr-Commit-Position: refs/heads/master@{#410567}