<webview>: Add test to confirm the TAB key will escape the view.

<webview>: Add test to confirm the TAB key will escape the view.

There is a corner case where the embedder may think the element directly
following the webview has focus prevents focusing moving out of the
webview with the tab key.

This problem is a combination of the embedder not knowing the webview is
focused and some early out logic when landing on the next element
because it is already focused.

BUG=674219
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2732913005
Cr-Commit-Position: refs/heads/master@{#470855}
Committed: https://chromium.googlesource.com/chromium/src/+/9edd151ead7afa9dc3f8cb3f4620adf949eb119c

[avallee, 2017-03-06 21:15:45]

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

[commit-bot: I haz the power, 2017-03-06 21:16:24]

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, 2017-03-06 21:58:20]

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

[commit-bot: I haz the power, 2017-03-06 21:58:20]

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

[avallee, 2017-03-08 21:32:16]

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

[commit-bot: I haz the power, 2017-03-08 21:32:46]

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, 2017-03-08 22:50:00]

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

[commit-bot: I haz the power, 2017-03-08 22:50:01]

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

[avallee, 2017-03-09 17:22:39]

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

[commit-bot: I haz the power, 2017-03-09 17:23:03]

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

[avallee, 2017-03-09 17:36:01]

Description was changed from

==========
<webview>: Add test to confirm the TAB key will escape the view.

There is a corner case where the embedder may think the element directly
following the webview has focus prevents focusing moving out of the
webview with the tab key.

This problem is a combination of the embedder not knowing the webview is
focused and some early out logic when landing on the next element
because it is already focused.

BUG=694682
==========

to

==========
<webview>: Add test to confirm the TAB key will escape the view.

There is a corner case where the embedder may think the element directly
following the webview has focus prevents focusing moving out of the
webview with the tab key.

This problem is a combination of the embedder not knowing the webview is
focused and some early out logic when landing on the next element
because it is already focused.

BUG=674219
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[commit-bot: I haz the power, 2017-03-09 19:35:27]

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

[commit-bot: I haz the power, 2017-03-09 19:35:28]

Dry run: This issue passed the CQ dry run.

[avallee, 2017-03-09 19:41:18]

avallee@chromium.org changed reviewers:
+ alexmos@chromium.org

[avallee, 2017-03-09 19:41:19]

PTAL: This seems to pass all blink tests and fixes the issue where focusing the
webview does not unfocus other elements in the embedder.

wjm,lfg: FYI

[alexmos, 2017-03-10 07:08:33]

https://codereview.chromium.org/2732913005/diff/40001/chrome/browser/apps/gue...
File chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc (right):

https://codereview.chromium.org/2732913005/diff/40001/chrome/browser/apps/gue...
chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc:673:
IN_PROC_BROWSER_TEST_P(WebViewFocusInteractiveTest, Focus_FocusTakeFocus) {
Since this is also a problem for OOPIFs without <webview>, it'd be great to also
have a pure OOPIF test for this, i.e., something along the lines of
SitePerProcessInteractiveBrowserTest.SequentialFocusNavigation.

https://codereview.chromium.org/2732913005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/page/FocusController.cpp (right):

https://codereview.chromium.org/2732913005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/FocusController.cpp:789: newFrame =
newFrame->client()->parent()) {
Why do we need to walk up the ancestor chain?  Is this for the case that
intermediate frames don't have any focusable elements?

https://codereview.chromium.org/2732913005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/FocusController.cpp:795: if
(focusedFrameIsAncestor)
What about the reverse case, where an <input> that has focus is before the
OOPIF, then the OOPIF is clicked, and then you shift-tab upwards?  Does that
case also work?

https://codereview.chromium.org/2732913005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/FocusController.cpp:796:
document->clearFocusedElement();
Will |frame| always be a RemoteFrame when this is needed?  Also, this could use
a comment with an explanation.

https://codereview.chromium.org/2732913005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/FocusController.cpp:798:
dispatchBlurEvent(*document, *focusedElement);
Just to check, does clearFocusedElement also dispatch this blur event?

[avallee, 2017-03-13 19:18:11]

https://codereview.chromium.org/2732913005/diff/40001/chrome/browser/apps/gue...
File chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc (right):

https://codereview.chromium.org/2732913005/diff/40001/chrome/browser/apps/gue...
chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc:673:
IN_PROC_BROWSER_TEST_P(WebViewFocusInteractiveTest, Focus_FocusTakeFocus) {
On 2017/03/10 07:08:33, alexmos wrote:
> Since this is also a problem for OOPIFs without <webview>, it'd be great to
also
> have a pure OOPIF test for this, i.e., something along the lines of
> SitePerProcessInteractiveBrowserTest.SequentialFocusNavigation.

Let me add this in then. Just to be clear, I think you're saying similar to the
SequentialFocusNavigation test, except that you click into the oopif and try to
navigate out (and maybe also click outside and try to navigate in).

https://codereview.chromium.org/2732913005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/page/FocusController.cpp (right):

https://codereview.chromium.org/2732913005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/FocusController.cpp:789: newFrame =
newFrame->client()->parent()) {
On 2017/03/10 07:08:33, alexmos wrote:
> Why do we need to walk up the ancestor chain?  Is this for the case that
> intermediate frames don't have any focusable elements?

I'm not sure how exiting a frame works. In the top > iframe > webview structure,
I'm not sure if there were no focusable elements in the iframe if the return to
top from webview is done in 1 or 2 steps. If it's done in 2 steps with focus
momentarily passing through the iframe, then this should not be needed. If
however, the focus passes straight through, it's possible that the top frame now
tries to advance and has an element focused.

https://codereview.chromium.org/2732913005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/FocusController.cpp:795: if
(focusedFrameIsAncestor)
On 2017/03/10 07:08:33, alexmos wrote:
> What about the reverse case, where an <input> that has focus is before the
> OOPIF, then the OOPIF is clicked, and then you shift-tab upwards?  Does that
> case also work?

This was doing the wrong thing when the browser notified you that the guest has
become focused. Tab/Shift-tab would be sent to a renderer who might decide to
initiate a focus change into a remote parent or child frame. When this renderer
initiate the focus change, it will be in a consistent state.

https://codereview.chromium.org/2732913005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/FocusController.cpp:796:
document->clearFocusedElement();
On 2017/03/10 07:08:33, alexmos wrote:
> Will |frame| always be a RemoteFrame when this is needed?  Also, this could
use
> a comment with an explanation.

I think the issue would only happen with remote frames. On a local frame, the
change happens within the renderer and things are set correctly at that point.

Here we were reacting to a remote renderer changing focus.

https://codereview.chromium.org/2732913005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/FocusController.cpp:798:
dispatchBlurEvent(*document, *focusedElement);
On 2017/03/10 07:08:33, alexmos wrote:
> Just to check, does clearFocusedElement also dispatch this blur event?

Yes, it calls setFocusedElement(nullptr, /*some other stuff*/) and then
dispatched on the oldFocusedElement:
http://google3/third_party/chromium_headless/blink/Source/core/dom/Document.c...

[alexmos, 2017-03-16 17:37:11]

+site-isolation-reviews, since this concerns OOPIF focus as well.  

Sorry for the delay!  Took me a while to think through the issues here, but some
thoughts below.

https://codereview.chromium.org/2732913005/diff/40001/chrome/browser/apps/gue...
File chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc (right):

https://codereview.chromium.org/2732913005/diff/40001/chrome/browser/apps/gue...
chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc:673:
IN_PROC_BROWSER_TEST_P(WebViewFocusInteractiveTest, Focus_FocusTakeFocus) {
On 2017/03/13 19:18:10, avallee wrote:
> On 2017/03/10 07:08:33, alexmos wrote:
> > Since this is also a problem for OOPIFs without <webview>, it'd be great to
> also
> > have a pure OOPIF test for this, i.e., something along the lines of
> > SitePerProcessInteractiveBrowserTest.SequentialFocusNavigation.
> 
> Let me add this in then. Just to be clear, I think you're saying similar to
the
> SequentialFocusNavigation test, except that you click into the oopif and try
to
> navigate out (and maybe also click outside and try to navigate in).

Yes, exactly.  I'm just thinking of following the repro steps in 674219. 
Thanks!

https://codereview.chromium.org/2732913005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/page/FocusController.cpp (right):

https://codereview.chromium.org/2732913005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/FocusController.cpp:787: bool
focusedFrameIsAncestor = false;
I think this is really newFocusedFrameIsDescendant, but it doesn't matter -- see
below.

https://codereview.chromium.org/2732913005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/FocusController.cpp:789: newFrame =
newFrame->client()->parent()) {
On 2017/03/13 19:18:10, avallee wrote:
> On 2017/03/10 07:08:33, alexmos wrote:
> > Why do we need to walk up the ancestor chain?  Is this for the case that
> > intermediate frames don't have any focusable elements?
> 
> I'm not sure how exiting a frame works. In the top > iframe > webview
structure,
> I'm not sure if there were no focusable elements in the iframe if the return
to
> top from webview is done in 1 or 2 steps. If it's done in 2 steps with focus
> momentarily passing through the iframe, then this should not be needed. If
> however, the focus passes straight through, it's possible that the top frame
now
> tries to advance and has an element focused.

I was really confused here and what this has to do with exiting a frame, but I
think I understand things better now after following some code around and
patching in your CL.  I mostly thought about this with just OOPIFs and no
webviews, since that's easier and the fix is the same for both.

So, the intent of this code is really to clear the focused element when we learn
that a remote frame has become focused.  I.e., in the bug repro, we have:
  <iframe A>
    <iframe B>
      <input 2>
    </iframe>
    <input 1>  <-- focused initially
  </iframe>

The <input 1> is focused initially, so frame A is the focused frame. You then
click on <input 2>.  At that point, the B renderer makes frame B the focused
frame and forwards the focused frame change to renderer A, which will end up
calling focusDocumentView() on the RemoteFrame for B.  At this point, you get to
this code and want to clear focused element (<input>).

A few concerns about this approach:

1.  I'm not convinced this should be gated on the ancestor walk.  Consider this
example: 

  <iframe A>
    <input 1>  <-- click this second
    <iframe B>
      <input 2> <-- click this first
    </iframe>
  </iframe>

  Then press tab.  Wouldn't that have the same problem unless you clear the
focused element (<input 2>) in frame B?  The current code won't do it, since
it'll get here in renderer B, with A being the new focused frame, i.e., B will
be the descendant not an ancestor of the new focused frame.

  In practice, I tried this out on
http://csreis.github.io/tests/cross-site-iframe-simple.html with
--site-per-process:
    a. in devtools,
document.querySelector("h1").appendChild(document.createElement("input"))
    b. click first <input> in iframe
    c. click the main frame's <input> created in a)
    d. press tab
  Focus should've switch to the first <input> in the iframe, but instead it
shifts to the *second* input there.  I don't quite understand why it does that,
and whether that's due to not calling clearFocusedElement() in B when we clicked
on the <input 1> in A, or a different issue.  Thoughts?

(It'd be good to add a test for this case as well.)

2.  I was thinking if we should instead just call clearFocusedElement when
|frame| is a remote frame.  That should fix 1 and any similar cases (sibling
iframes?).  But I can't convince myself that it's safe.

Thinking more carefully about how this works without any OOPIFs today, it seems
that normally clearFocusedElement is called as part of
FocusController::setFocusedElement if the old focused element was in a different
frame, as part of mouse click dispatch (EventHandler::handleMousePressEvent ->
MouseEventManager::handleMouseFocus -> FocusController::setFocusedElement ->
Document::clearFocusedElement).  However, setFocusedElement is not called
unconditionally in handleMouseFocus; there are a few
WebInputEventResult::NotHandled early returns that I don't fully understand
(e.g., for clicking on scrollbar, some drag stuff).  

What I'm wondering is whether it's possible to click on a frame and focus that
frame, but follow one of those early returns and not focus any elements in that
frame (which would leave the focused element intact in the old focused frame). 
If not, maybe we can get away with clearFocusedElement when |frame| is a remote
frame.  If yes, we may need to send a separate IPC to clear the focused element
in the old focused frame only when needed.  Maybe you could try seeing if any of
those early returns matter in practice?  

FWIW, although usually FocusController::setFocusedFrame is called as part of
FocusController::setFocusedElement, it's also called again via
MouseEventManager::handleMousePressEvent -> MouseEventManager::focusDocumentView
-> FocusController::focusDocumentView -> setFocusedFrame (which is usually a
noop), but could matter in case we end up not calling setFocusedElement.

Regarding your question on how tab traversal exits a frame, I think it's a
one-step process.  If you clicked on a remote grandchild frame nested in a
middle frame with no focusable elements, and then pressing tab to get out to the
root frame,  advanceFocusInDocumentOrder and
findFocusableElementAcrossFocusScopes will look for elements to focus in all
possible local frames, including ancestor frames if they exist, in one step
before handing the search over to the remote parent of the local root.

https://codereview.chromium.org/2732913005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/FocusController.cpp:795: if
(focusedFrameIsAncestor)
On 2017/03/13 19:18:10, avallee wrote:
> On 2017/03/10 07:08:33, alexmos wrote:
> > What about the reverse case, where an <input> that has focus is before the
> > OOPIF, then the OOPIF is clicked, and then you shift-tab upwards?  Does that
> > case also work?
> 
> This was doing the wrong thing when the browser notified you that the guest
has
> become focused. Tab/Shift-tab would be sent to a renderer who might decide to
> initiate a focus change into a remote parent or child frame. When this
renderer
> initiate the focus change, it will be in a consistent state.

I'm not sure I followed your explanation, but I think the case I asked about
works. :)  It doesn't matter where the previously focused element is, we'll
clear it regardless when we click on a descendant of a currently focused frame,
whether it's an OOPIF or webview.  So whether we get to the element via tab or
shift-tab later doesn't matter.

https://codereview.chromium.org/2732913005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/FocusController.cpp:796:
document->clearFocusedElement();
On 2017/03/13 19:18:10, avallee wrote:
> On 2017/03/10 07:08:33, alexmos wrote:
> > Will |frame| always be a RemoteFrame when this is needed?  Also, this could
> use
> > a comment with an explanation.
> 
> I think the issue would only happen with remote frames. On a local frame, the
> change happens within the renderer and things are set correctly at that point.
> 
> Here we were reacting to a remote renderer changing focus.

See the big comment above regarding this.

https://codereview.chromium.org/2732913005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/FocusController.cpp:798:
dispatchBlurEvent(*document, *focusedElement);
On 2017/03/13 19:18:10, avallee wrote:
> On 2017/03/10 07:08:33, alexmos wrote:
> > Just to check, does clearFocusedElement also dispatch this blur event?
> 
> Yes, it calls setFocusedElement(nullptr, /*some other stuff*/) and then
> dispatched on the oldFocusedElement:
>
http://google3/third_party/chromium_headless/blink/Source/core/dom/Document.c...

Acknowledged.

[alexmos, 2017-03-16 17:59:47]

https://codereview.chromium.org/2732913005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/page/FocusController.cpp (right):

https://codereview.chromium.org/2732913005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/FocusController.cpp:789: newFrame =
newFrame->client()->parent()) {
On 2017/03/16 17:37:11, alexmos wrote:
> On 2017/03/13 19:18:10, avallee wrote:
> > On 2017/03/10 07:08:33, alexmos wrote:
> > > Why do we need to walk up the ancestor chain?  Is this for the case that
> > > intermediate frames don't have any focusable elements?
> > 
> > I'm not sure how exiting a frame works. In the top > iframe > webview
> structure,
> > I'm not sure if there were no focusable elements in the iframe if the return
> to
> > top from webview is done in 1 or 2 steps. If it's done in 2 steps with focus
> > momentarily passing through the iframe, then this should not be needed. If
> > however, the focus passes straight through, it's possible that the top frame
> now
> > tries to advance and has an element focused.
> 
> I was really confused here and what this has to do with exiting a frame, but I
> think I understand things better now after following some code around and
> patching in your CL.  I mostly thought about this with just OOPIFs and no
> webviews, since that's easier and the fix is the same for both.
> 
> So, the intent of this code is really to clear the focused element when we
learn
> that a remote frame has become focused.  I.e., in the bug repro, we have:
>   <iframe A>
>     <iframe B>
>       <input 2>
>     </iframe>
>     <input 1>  <-- focused initially
>   </iframe>
> 
> The <input 1> is focused initially, so frame A is the focused frame. You then
> click on <input 2>.  At that point, the B renderer makes frame B the focused
> frame and forwards the focused frame change to renderer A, which will end up
> calling focusDocumentView() on the RemoteFrame for B.  At this point, you get
to
> this code and want to clear focused element (<input>).
> 
> A few concerns about this approach:
> 
> 1.  I'm not convinced this should be gated on the ancestor walk.  Consider
this
> example: 
> 
>   <iframe A>
>     <input 1>  <-- click this second
>     <iframe B>
>       <input 2> <-- click this first
>     </iframe>
>   </iframe>
> 
>   Then press tab.  Wouldn't that have the same problem unless you clear the
> focused element (<input 2>) in frame B?  The current code won't do it, since
> it'll get here in renderer B, with A being the new focused frame, i.e., B will
> be the descendant not an ancestor of the new focused frame.
> 
>   In practice, I tried this out on
> http://csreis.github.io/tests/cross-site-iframe-simple.html with
> --site-per-process:
>     a. in devtools,
> document.querySelector("h1").appendChild(document.createElement("input"))
>     b. click first <input> in iframe
>     c. click the main frame's <input> created in a)
>     d. press tab
>   Focus should've switch to the first <input> in the iframe, but instead it
> shifts to the *second* input there.  I don't quite understand why it does
that,
> and whether that's due to not calling clearFocusedElement() in B when we
clicked
> on the <input 1> in A, or a different issue.  Thoughts?
> 
> (It'd be good to add a test for this case as well.)
> 
> 2.  I was thinking if we should instead just call clearFocusedElement when
> |frame| is a remote frame.  That should fix 1 and any similar cases (sibling
> iframes?).  But I can't convince myself that it's safe.
> 
> Thinking more carefully about how this works without any OOPIFs today, it
seems
> that normally clearFocusedElement is called as part of
> FocusController::setFocusedElement if the old focused element was in a
different
> frame, as part of mouse click dispatch (EventHandler::handleMousePressEvent ->
> MouseEventManager::handleMouseFocus -> FocusController::setFocusedElement ->
> Document::clearFocusedElement).  However, setFocusedElement is not called
> unconditionally in handleMouseFocus; there are a few
> WebInputEventResult::NotHandled early returns that I don't fully understand
> (e.g., for clicking on scrollbar, some drag stuff).  
> 
> What I'm wondering is whether it's possible to click on a frame and focus that
> frame, but follow one of those early returns and not focus any elements in
that
> frame (which would leave the focused element intact in the old focused frame).

> If not, maybe we can get away with clearFocusedElement when |frame| is a
remote
> frame.  If yes, we may need to send a separate IPC to clear the focused
element
> in the old focused frame only when needed.  Maybe you could try seeing if any
of
> those early returns matter in practice?  
> 
> FWIW, although usually FocusController::setFocusedFrame is called as part of
> FocusController::setFocusedElement, it's also called again via
> MouseEventManager::handleMousePressEvent ->
MouseEventManager::focusDocumentView
> -> FocusController::focusDocumentView -> setFocusedFrame (which is usually a
> noop), but could matter in case we end up not calling setFocusedElement.
> 
> Regarding your question on how tab traversal exits a frame, I think it's a
> one-step process.  If you clicked on a remote grandchild frame nested in a
> middle frame with no focusable elements, and then pressing tab to get out to
the
> root frame,  advanceFocusInDocumentOrder and
> findFocusableElementAcrossFocusScopes will look for elements to focus in all
> possible local frames, including ancestor frames if they exist, in one step
> before handing the search over to the remote parent of the local root.

I'll also note that at some point, when replicating the new focused frame  I
tried using setFocusedElement(nullptr, newFocusedFrame) instead of
focusDocumentView, which would've avoided this bug since it clears the old
focused element.  I can't remember all the details, but I think I ran into
issues with that, possibly because of window.focus called on remote frames?  See
https://codereview.chromium.org/1423053002/#msg4 and PS1->PS2.  (Just mentioning
this as that's another alternative for a fix.)

[avallee, 2017-05-10 18:39:00]

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

[commit-bot: I haz the power, 2017-05-10 18:40:03]

Dry run: CQ is trying da patch.

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

[avallee, 2017-05-10 18:42:16]

avallee@chromium.org changed reviewers:
+ wjmaclean@chromium.org

[avallee, 2017-05-10 18:42:17]

wjmaclean: PTAL.

https://codereview.chromium.org/2732913005/diff/40001/chrome/browser/apps/gue...
File chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc (right):

https://codereview.chromium.org/2732913005/diff/40001/chrome/browser/apps/gue...
chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc:673:
IN_PROC_BROWSER_TEST_P(WebViewFocusInteractiveTest, Focus_FocusTakeFocus) {
On 2017/03/16 17:37:11, alexmos wrote:
> On 2017/03/13 19:18:10, avallee wrote:
> > On 2017/03/10 07:08:33, alexmos wrote:
> > > Since this is also a problem for OOPIFs without <webview>, it'd be great
to
> > also
> > > have a pure OOPIF test for this, i.e., something along the lines of
> > > SitePerProcessInteractiveBrowserTest.SequentialFocusNavigation.
> > 
> > Let me add this in then. Just to be clear, I think you're saying similar to
> the
> > SequentialFocusNavigation test, except that you click into the oopif and try
> to
> > navigate out (and maybe also click outside and try to navigate in).
> 
> Yes, exactly.  I'm just thinking of following the repro steps in 674219. 
> Thanks!

Test created and enabled (flaky but quite rarely) with https://crbug.com/713977

[commit-bot: I haz the power, 2017-05-10 20:23:42]

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

[commit-bot: I haz the power, 2017-05-10 20:23:43]

Dry run: This issue passed the CQ dry run.

[wjmaclean, 2017-05-10 23:51:10]

lgtm

[avallee, 2017-05-11 06:57:15]

The CQ bit was checked by avallee@chromium.org

[commit-bot: I haz the power, 2017-05-11 06:57:55]

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, 2017-05-11 07:04:56]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1494485835694340,
"parent_rev": "d864c36cdca36cf097210057df806da44d98291a", "commit_rev":
"9edd151ead7afa9dc3f8cb3f4620adf949eb119c"}

[commit-bot: I haz the power, 2017-05-11 07:05:11]

Description was changed from

==========
<webview>: Add test to confirm the TAB key will escape the view.

There is a corner case where the embedder may think the element directly
following the webview has focus prevents focusing moving out of the
webview with the tab key.

This problem is a combination of the embedder not knowing the webview is
focused and some early out logic when landing on the next element
because it is already focused.

BUG=674219
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
<webview>: Add test to confirm the TAB key will escape the view.

There is a corner case where the embedder may think the element directly
following the webview has focus prevents focusing moving out of the
webview with the tab key.

This problem is a combination of the embedder not knowing the webview is
focused and some early out logic when landing on the next element
because it is already focused.

BUG=674219
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2732913005
Cr-Commit-Position: refs/heads/master@{#470855}
Committed:
https://chromium.googlesource.com/chromium/src/+/9edd151ead7afa9dc3f8cb3f4620...
==========

[commit-bot: I haz the power, 2017-05-11 07:05:15]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/9edd151ead7afa9dc3f8cb3f4620...