The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/86b13b4626266c45ee364474bcf86208d4e2e4a2

commit 86b13b4626266c45ee364474bcf86208d4e2e4a2
Author: Hugo Holgersson <hugoh@vewd.com>
Date: Tue Oct 03 11:58:34 2017

Don't clear selection when spatial navigation moves focus

We kept a Selection.Clear() to not break the test "snav-textarea.html".

<textarea>'s behavior is the same, no matter whether
--enable-spatial-navigation is used or not, so instead of clearing
the <textarea>'s selection we just need to adapt the test.

This is a step towards our goal of simplifying the spatial navigation code.

Bug:  734552 , 770147
Change-Id: If82c99658e44aee776afa87af823e005259415e0
Reviewed-on: https://chromium-review.googlesource.com/695103
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Takayoshi Kochi <kochi@chromium.org>
Reviewed-by: Fredrik Söderquist <fs@opera.com>
Commit-Queue: Hugo Holgersson <hugoh@vewd.com>
Cr-Commit-Position: refs/heads/master@{#506010}
[modify] https://crrev.com/86b13b4626266c45ee364474bcf86208d4e2e4a2/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-textarea-expected.txt
[modify] https://crrev.com/86b13b4626266c45ee364474bcf86208d4e2e4a2/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-textarea.html
[modify] https://crrev.com/86b13b4626266c45ee364474bcf86208d4e2e4a2/third_party/WebKit/Source/core/page/FocusController.cpp

Let's call the argument "direction" instead "type" in spatial navigation.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0cd743c3d12dbe729fcb6154cbe5ab4e2cceccd2

commit 0cd743c3d12dbe729fcb6154cbe5ab4e2cceccd2
Author: JunHo Seo <junho0924.seo@lge.com>
Date: Wed Nov 15 06:24:16 2017

Introduce IsNavigableContainer() to Spatial Navigation

Previously CanScrollInDirection() is used to check whether a node can
be container. This is ambiguous because CanScrollInDirection() is also
used when do actually scrolling.

IsNavigableContainer() is explicitly used when we need to check whether
a node can be container.

Bug: 770147
Change-Id: I7b893795d945fc6cd60ea40d491e86af0d4a53ff
Reviewed-on: https://chromium-review.googlesource.com/749572
Reviewed-by: Hayato Ito <hayato@chromium.org>
Reviewed-by: Hugo Holgersson <hugoh@vewd.com>
Commit-Queue: Takayoshi Kochi <kochi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516610}
[modify] https://crrev.com/0cd743c3d12dbe729fcb6154cbe5ab4e2cceccd2/third_party/WebKit/Source/core/BUILD.gn
[modify] https://crrev.com/0cd743c3d12dbe729fcb6154cbe5ab4e2cceccd2/third_party/WebKit/Source/core/page/FocusController.cpp
[modify] https://crrev.com/0cd743c3d12dbe729fcb6154cbe5ab4e2cceccd2/third_party/WebKit/Source/core/page/SpatialNavigation.cpp
[modify] https://crrev.com/0cd743c3d12dbe729fcb6154cbe5ab4e2cceccd2/third_party/WebKit/Source/core/page/SpatialNavigation.h
[add] https://crrev.com/0cd743c3d12dbe729fcb6154cbe5ab4e2cceccd2/third_party/WebKit/Source/core/page/SpatialNavigationTest.cpp

nit: Can we rename this to will_remain_offscreen_after_a_scroll ?
We haven't yet scrolled anything at this point. "after scrolling" makes me think that we have...

Can you please refactor this slightly:

while (container) {
  consumed = ...;
  if (consumed)
    break;
Just a small optimization to avoid an unnecessary layout if the next container is a document.

https://chromium-review.googlesource.com/c/chromium/src/+/781326/21/third_party/WebKit/Source/core/page/FocusController.cpp#1417

You should consider, in a future patch, delegating the scrolling behavior to ScrollManager::LogicalScroll, to avoid code duplication and potential differences from the default arrow key scrolling behavior.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/69520020e34d2c98ebcc342f852ccda11ebd2f74

commit 69520020e34d2c98ebcc342f852ccda11ebd2f74
Author: JunHo Seo <junho0924.seo@lge.com>
Date: Wed Jan 17 14:01:09 2018

Snav: Keep the search start point constant throughout the search

Background:
If snav fails to find a focusable element in the first search
scope (activeElement's enclosing container C), snav expands
the scope to C's parent container.

starting_rect =
   visual rectangle from where snav looks for new focusables.

Focusables close to the starting_rect gets priority.

Problem:
As snav expands the scope, starting_rect changes from being
the enclosing container's visual rectangle to being the
visual rectangle of the parent container =>
This makes focus go to an undesirable element because snav's
use of a faulty "base" when evaluating distances to focusables.

Solution:
Keep the initial starting_rect throughout the search, even
if the search scope changes.

While here, add an early break that avoids an unnecessary layout.

TBR=szager@chromium.org, this was already discussed in:
https://chromium-review.googlesource.com/c/chromium/src/+/781326

Bug: 770147,  802998 
Change-Id: I00690a93d6a329ca30aa14432b9a7d25f3a3d65f
Reviewed-on: https://chromium-review.googlesource.com/870029
Commit-Queue: Hugo Holgersson <hugoh@vewd.com>
Reviewed-by: Hugo Holgersson <hugoh@vewd.com>
Cr-Commit-Position: refs/heads/master@{#529730}
[add] https://crrev.com/69520020e34d2c98ebcc342f852ccda11ebd2f74/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-iframe-with-outside-focusable-element-expected.txt
[add] https://crrev.com/69520020e34d2c98ebcc342f852ccda11ebd2f74/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-iframe-with-outside-focusable-element.html
[modify] https://crrev.com/69520020e34d2c98ebcc342f852ccda11ebd2f74/third_party/WebKit/Source/core/page/FocusController.cpp

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b1eaec2e1fe9eb6bc37b6a43a574a081d914b2af

commit b1eaec2e1fe9eb6bc37b6a43a574a081d914b2af
Author: Hugo Holgersson <hugoh@vewd.com>
Date: Thu Jan 18 16:56:24 2018

Snav: Search containers that reached their scroll limit

Problem:
A fully scrolled container might still have nearby
focusables. Before this CL, those were ignored.

Solution:
This adds a new helper IsScrollableAreaOrDocument() that
we can use in ScrollableEnclosingBoxOrParentFrameOf()
to find a node's "container":

Q: What is a "container"?
A: A node's _container_ is its enclosing scrollable area
   (= its first scrollable ancestor) or its document
   (possibly unscrollable).

In other words, we don't need a direction to determine
a node's container. This is a simplification that's
needed to fix  bug 770103 .

Bug: 770147,  770103 
Change-Id: I439d532631e9d424e40e801873ef330cd15ffe7f
Reviewed-on: https://chromium-review.googlesource.com/866790
Commit-Queue: Hugo Holgersson <hugoh@vewd.com>
Reviewed-by: Stefan Zager <szager@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530169}
[add] https://crrev.com/b1eaec2e1fe9eb6bc37b6a43a574a081d914b2af/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-stay-in-overflow-div-expected.txt
[add] https://crrev.com/b1eaec2e1fe9eb6bc37b6a43a574a081d914b2af/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-stay-in-overflow-div.html
[modify] https://crrev.com/b1eaec2e1fe9eb6bc37b6a43a574a081d914b2af/third_party/WebKit/Source/core/page/FocusController.cpp
[modify] https://crrev.com/b1eaec2e1fe9eb6bc37b6a43a574a081d914b2af/third_party/WebKit/Source/core/page/SpatialNavigation.cpp
[modify] https://crrev.com/b1eaec2e1fe9eb6bc37b6a43a574a081d914b2af/third_party/WebKit/Source/core/page/SpatialNavigation.h
[modify] https://crrev.com/b1eaec2e1fe9eb6bc37b6a43a574a081d914b2af/third_party/WebKit/Source/core/page/SpatialNavigationTest.cpp

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/604e3794f34769458dd7bc0791f583d061dd9cd3

commit 604e3794f34769458dd7bc0791f583d061dd9cd3
Author: JunHo Seo <junho0924.seo@lge.com>
Date: Fri Jan 19 05:27:35 2018

Snav: Remove recursion and ignore non-navigable subtrees

This CL contains a major refactoring.

1. Ignore non-navigable iframes.
Previously, spatnav only dug into the first iframe it found.
Now, in case that is an iframe without focusables, we ignore
it and go look for the next candidate.

Note that additional time and space can be increased in the
worst case (the next closest element is always an iframe that
cannot consume input). Example:
<a href='#'>focused element</a>
<iframe></iframe>
.. many iframes
<iframe></iframe>
<a href='#'>target element</a>

2. Remove recursion.
Now we use a HeapVector and regard it as stack for managing search
order instead the call stack.
Also we cleaned up some dead code related with is_offscreen_after_
scrolling condition statements. The condition never hit because
is_offscreen_after_scrolling==false is one of condition for being
candidate.

3. Don't revisit subtrees we've already searched.
When the search scope is expanded from "current container" C to
C's parent container, AdvanceFocusDirectionallyInContainer()
searched C's tree again. Now we skip already searched subtrees.

Bug: 770147,  784320 
Change-Id: Idcce4e9125ec05d35481ecdc17b1336a85bc968a
Reviewed-on: https://chromium-review.googlesource.com/781326
Commit-Queue: JunHo Seo <junho0924.seo@lge.com>
Reviewed-by: Stefan Zager <szager@chromium.org>
Reviewed-by: Hugo Holgersson <hugoh@vewd.com>
Cr-Commit-Position: refs/heads/master@{#530429}
[add] https://crrev.com/604e3794f34769458dd7bc0791f583d061dd9cd3/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-backtrack-out-of-non-navigable-iframe-expected.txt
[add] https://crrev.com/604e3794f34769458dd7bc0791f583d061dd9cd3/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-backtrack-out-of-non-navigable-iframe.html
[modify] https://crrev.com/604e3794f34769458dd7bc0791f583d061dd9cd3/third_party/WebKit/Source/core/page/FocusController.cpp
[modify] https://crrev.com/604e3794f34769458dd7bc0791f583d061dd9cd3/third_party/WebKit/Source/core/page/FocusController.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/511906d0604cdd92df8aa077b82c51aa1a298525

commit 511906d0604cdd92df8aa077b82c51aa1a298525
Author: Hugo Holgersson <hugoh@vewd.com>
Date: Tue Jan 23 15:54:32 2018

Snav: Always search for focusables in visual viewport first

By initializing starting_rect to one of the visual viewport's
edges we don't need to recalculate this value every time
AdvanceFocusDirectionallyInContainer iterates.

Q: So now we always use the frame's visual viewport when the
   focused element is off screen?

A: Yes. The visual viewport is the default starting point for
   snav's search. This was already the implicit behavior users
   relied on (see bug).
   When the focused element F is off screen (say JS scrolled
   <body> downwards) we want spatnav to first look for already
   visible focusables, i.e not go to focusables nearby F because
   that will cause the window to scroll (bad UX).

This lets users reach things that they can _see_ first.

       snav-ignore-offscreen-focused-imagemap.html (NEW)

Bug: 770147,  796493 ,  804242 
Tests: snav-use-visual-viewport.html,
Change-Id: Iaa861d3f75effe14eac558e2214dbc3af2df365b
Reviewed-on: https://chromium-review.googlesource.com/873645
Commit-Queue: Hugo Holgersson <hugoh@vewd.com>
Reviewed-by: Stefan Zager <szager@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531249}
[add] https://crrev.com/511906d0604cdd92df8aa077b82c51aa1a298525/third_party/WebKit/LayoutTests/fast/spatial-navigation/snav-ignore-offscreen-focused-imagemap.html
[modify] https://crrev.com/511906d0604cdd92df8aa077b82c51aa1a298525/third_party/WebKit/Source/core/page/FocusController.cpp
[modify] https://crrev.com/511906d0604cdd92df8aa077b82c51aa1a298525/third_party/WebKit/Source/core/page/SpatialNavigation.cpp
[modify] https://crrev.com/511906d0604cdd92df8aa077b82c51aa1a298525/third_party/WebKit/Source/core/page/SpatialNavigation.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/39616000e99e01e2946bff0fb580c3cfd126cb71

commit 39616000e99e01e2946bff0fb580c3cfd126cb71
Author: JunHo Seo <junho0924.seo@lge.com>
Date: Tue Apr 10 06:43:03 2018

Snav: Use argument name 'direction' instead 'type'

'type' doesn't give much information. Use more informative name.

Bug: 770147

Change-Id: Iad53dfc11347c149f1d076cc5c326437df2f28c4
Reviewed-on: https://chromium-review.googlesource.com/989387
Commit-Queue: JunHo Seo <junho0924.seo@lge.com>
Reviewed-by: Hugo Holgersson <hugoh@vewd.com>
Reviewed-by: Stefan Zager <szager@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549437}
[modify] https://crrev.com/39616000e99e01e2946bff0fb580c3cfd126cb71/third_party/blink/renderer/core/page/focus_controller.cc
[modify] https://crrev.com/39616000e99e01e2946bff0fb580c3cfd126cb71/third_party/blink/renderer/core/page/spatial_navigation.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f498307fe76198eee05bf40a3167928e1d6a4b35

commit f498307fe76198eee05bf40a3167928e1d6a4b35
Author: Hugo Holgersson <hugoh@vewd.com>
Date: Thu Nov 08 15:56:55 2018

Snav: Remove the redundant CanBeScrolledIntoView()

This simplifies spatnav's inner loop, the evaluation of
DOM nodes when looking for candidates, a tiny bit.

Now that CanBeScrolledIntoView lost its call site
we can remove it completely to reduce spatnav's
code footprint.

Bug: 770147
Test: snav-div-overflow-scroll-hidden.html
Change-Id: Icbf4cb4d0274d63658dbeab1cdae1ce63557576f
Reviewed-on: https://chromium-review.googlesource.com/c/1319678
Commit-Queue: Hugo Holgersson <hugoh@vewd.com>
Reviewed-by: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#606480}
[modify] https://crrev.com/f498307fe76198eee05bf40a3167928e1d6a4b35/third_party/blink/renderer/core/page/focus_controller.cc
[modify] https://crrev.com/f498307fe76198eee05bf40a3167928e1d6a4b35/third_party/blink/renderer/core/page/spatial_navigation.cc
[modify] https://crrev.com/f498307fe76198eee05bf40a3167928e1d6a4b35/third_party/blink/renderer/core/page/spatial_navigation.h

Here's a summary of the rules that were executed: 
 - OnlyMergeApprovedChange: Rule Failed -- Revision 7b86de57679a42b83f2cee3d073a3ed79ba558ae was merged to refs/branch-heads/3626 branch with no merge approval from a TPM! 
Please explain why this change was merged to the branch!
 - AcknowledgeMerge: Notification Required -- 

The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/7b86de57679a42b83f2cee3d073a3ed79ba558ae

Commit: 7b86de57679a42b83f2cee3d073a3ed79ba558ae
Author: changwan@chromium.org
Commiter: changwan@chromium.org
Date: 2019-01-23 00:46:42 +0000 UTC

Revert "Snav: Remove the redundant CanBeScrolledIntoView()"

This reverts commit f498307fe76198eee05bf40a3167928e1d6a4b35.

Reason for revert: caused crbug.com/920578

Original change's description:
> Snav: Remove the redundant CanBeScrolledIntoView()
> 
> This simplifies spatnav's inner loop, the evaluation of
> DOM nodes when looking for candidates, a tiny bit.
> 
> Now that CanBeScrolledIntoView lost its call site
> we can remove it completely to reduce spatnav's
> code footprint.
> 
> Bug: 770147
> Test: snav-div-overflow-scroll-hidden.html
> Change-Id: Icbf4cb4d0274d63658dbeab1cdae1ce63557576f
> Reviewed-on: https://chromium-review.googlesource.com/c/1319678
> Commit-Queue: Hugo Holgersson <hugoh@vewd.com>
> Reviewed-by: Fredrik Söderquist <fs@opera.com>
> Cr-Commit-Position: refs/heads/master@{#606480}

TBR=fs@opera.com,junho0924.seo@lge.com,hugoh@vewd.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 770147
Change-Id: Idecfe96878c70a40ffa58ef7bedc8897e9b28fe8
Reviewed-on: https://chromium-review.googlesource.com/c/1428443
Reviewed-by: Changwan Ryu <changwan@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#767}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}