Clear FocusController::focused_frame_ when RemoteFrame is detached.

Before this CL, after RemoteFrame is detached, it would be kept alive
by FocusController::focused_frame_.

Before this CL, LocalFrame::DetachImpl would clear
FocusController::focused_frame_ if needed.  After this CL this is done
from Frame::Detach (and so covers both LocalFrame and RemoteFrame).

Before and after this CL, FocusController::focused_frame_ stores
either a LocalFrame or RemoteFrame, but FocusController::FocusedFrame
only exposes LocalFrame.  This CL preserves this approach of handling
RemoteFrame internally within FocusController, by introducing
FocusController::FrameDetached(Frame* detached_frame) method
which can directly compare |detached_frame| with |focused_frame_|
(without going through local-vs-remote considerations and/or casts).

Bug: 906809
Change-Id: I47f3a725db87f3a887ad96c7a4739b4fc02496ed
Reviewed-on: https://chromium-review.googlesource.com/c/1344254
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609870}

third_party/WebKit/LayoutTests/LeakExpectations

@@ -61,34 +61,6 @@ crbug.com/765721 [ Linux ] virtual/stable/http/tests/navigation/navigation-inter
 crbug.com/809609 [ Linux ] editing/pasteboard/drop-file-svg.html [ Pass Leak ]
 crbug.com/809609 [ Linux ] editing/inserting/insert_div_with_style.html [ Pass Leak ]
 
-# -----------------------------------------------------------------
-# Not revert suspected CL at lukasza@ request.  Suspecting that this is
-# an OOPIF-related issue with leak detection in the test harness, rather
-# than a real leak.
-#
-# Leaks that repro only with site-per-process:
-crbug.com/906809 external/wpt/html/browsers/the-window-object/security-window/window-security.https.html [ Pass Leak ]
-crbug.com/906809 http/tests/feature-policy/fullscreen-allowed-by-container-policy-relocate.html [ Pass Leak ]
-crbug.com/906809 http/tests/feature-policy/fullscreen-allowed-by-container-policy.html [ Pass Leak ]
-crbug.com/906809 http/tests/feature-policy/fullscreen-disabled.php [ Pass Leak ]
-crbug.com/906809 http/tests/feature-policy/fullscreen-enabledforall.php [ Pass Leak ]
-crbug.com/906809 http/tests/feature-policy/fullscreen-enabledforself.php [ Pass Leak ]
-crbug.com/906809 http/tests/html/validation-bubble-oopif.html [ Pass Leak ]
-crbug.com/906809 http/tests/media/autoplay/webaudio-autoplay-iframe-with-gesture.html [ Pass Leak ]
-crbug.com/906809 http/tests/security/referrer-policy-redirect-link.html [ Pass Leak ]
-crbug.com/906809 http/tests/security/vibration/vibrate-in-cross-origin-iframe-with-user-gesture-allowed.html [ Pass Leak ]
-crbug.com/906809 http/tests/security/xss-DENIED-window-open-javascript-url-with-spaces.html [ Pass Leak ]
-crbug.com/906809 http/tests/security/xss-DENIED-window-open-javascript-url.html [ Pass Leak ]
-crbug.com/906809 virtual/outofblink-cors-ns/http/tests/security/referrer-policy-redirect-link.html [ Pass Leak ]
-crbug.com/906809 virtual/outofblink-cors-ns/http/tests/security/vibration/vibrate-in-cross-origin-iframe-with-user-gesture-allowed.html [ Pass Leak ]
-crbug.com/906809 virtual/outofblink-cors-ns/http/tests/security/xss-DENIED-window-open-javascript-url-with-spaces.html [ Pass Leak ]
-crbug.com/906809 virtual/outofblink-cors-ns/http/tests/security/xss-DENIED-window-open-javascript-url.html [ Pass Leak ]
-crbug.com/906809 virtual/outofblink-cors/http/tests/security/referrer-policy-redirect-link.html [ Pass Leak ]
-crbug.com/906809 virtual/outofblink-cors/http/tests/security/vibration/vibrate-in-cross-origin-iframe-with-user-gesture-allowed.html [ Pass Leak ]
-crbug.com/906809 virtual/outofblink-cors/http/tests/security/xss-DENIED-window-open-javascript-url-with-spaces.html [ Pass Leak ]
-crbug.com/906809 virtual/outofblink-cors/http/tests/security/xss-DENIED-window-open-javascript-url.html [ Pass Leak ]
-crbug.com/906809 virtual/user-activation-v2/http/tests/media/autoplay/webaudio-autoplay-iframe-with-gesture.html [ Pass Leak ]
-
 # -----------------------------------------------------------------
 # Sheriff 2018-04-23
 # -----------------------------------------------------------------

third_party/blink/renderer/core/frame/frame.cc

@@ -79,6 +79,10 @@ void Frame::Detach(FrameDetachType type) {
   lifecycle_.AdvanceTo(FrameLifecycle::kDetaching);
 
   DetachImpl(type);
+
+  if (GetPage())
+    GetPage()->GetFocusController().FrameDetached(this);
+
   // Due to re-entrancy, |this| could have completed detaching already.
   // TODO(dcheng): This DCHECK is not always true. See https://crbug.com/838348.
   DCHECK(IsDetached() == !client_);

third_party/blink/renderer/core/frame/local_frame.cc

@@ -30,7 +30,9 @@
 
 #include "third_party/blink/renderer/core/frame/local_frame.h"
 
+#include <limits>
 #include <memory>
+#include <utility>
 
 #include "services/network/public/cpp/features.h"
 #include "services/service_manager/public/cpp/connector.h"
@@ -426,9 +428,6 @@ void LocalFrame::DetachImpl(FrameDetachType type) {
 
   DomWindow()->FrameDestroyed();
 
-  if (GetPage() && GetPage()->GetFocusController().FocusedFrame() == this)
-    GetPage()->GetFocusController().SetFocusedFrame(nullptr);
-
   probe::frameDetachedFromParent(this);
 
   supplements_.clear();

third_party/blink/renderer/core/page/focus_controller.cc

@@ -26,6 +26,8 @@
 
 #include "third_party/blink/renderer/core/page/focus_controller.h"
 
+#include <limits>
+
 #include "third_party/blink/renderer/core/accessibility/ax_object_cache.h"
 #include "third_party/blink/renderer/core/dom/container_node.h"
 #include "third_party/blink/renderer/core/dom/document.h"
@@ -58,8 +60,6 @@
 #include "third_party/blink/renderer/core/page/slot_scoped_traversal.h"
 #include "third_party/blink/renderer/core/page/spatial_navigation.h"
 
-#include <limits>
-
 namespace blink {
 
 namespace {
@@ -843,10 +843,7 @@ void FocusController::FocusDocumentView(Frame* frame, bool notify_embedder) {
 }
 
 LocalFrame* FocusController::FocusedFrame() const {
-  // TODO(alexmos): Strengthen this to DCHECK that whoever called this really
-  // expected a LocalFrame. Refactor call sites so that the rare cases that
-  // need to know about focused RemoteFrames use a separate accessor (to be
-  // added).
+  // All callsites only care about *local* focused frames.
   if (focused_frame_ && focused_frame_->IsRemoteFrame())
     return nullptr;
   return ToLocalFrame(focused_frame_.Get());
@@ -856,9 +853,10 @@ Frame* FocusController::FocusedOrMainFrame() const {
   if (LocalFrame* frame = FocusedFrame())
     return frame;
 
-  // FIXME: This is a temporary hack to ensure that we return a LocalFrame, even
-  // when the mainFrame is remote.  FocusController needs to be refactored to
-  // deal with RemoteFrames cross-process focus transfers.
+  // TODO(dcheng, alexmos): https://crbug.com/820786: This is a temporary hack
+  // to ensure that we return a LocalFrame, even when the mainFrame is remote.
+  // FocusController needs to be refactored to deal with RemoteFrames
+  // cross-process focus transfers.
   for (Frame* frame = &page_->MainFrame()->Tree().Top(); frame;
        frame = frame->Tree().TraverseNext()) {
     if (frame->IsLocalFrame() && ToLocalFrame(frame)->IsLocalRoot())
@@ -868,6 +866,11 @@ Frame* FocusController::FocusedOrMainFrame() const {
   return page_->MainFrame();
 }
 
+void FocusController::FrameDetached(Frame* detached_frame) {
+  if (detached_frame == focused_frame_)
+    SetFocusedFrame(nullptr);
+}
+
 HTMLFrameOwnerElement* FocusController::FocusedFrameOwnerElement(
     LocalFrame& current_frame) const {
   Frame* focused_frame = focused_frame_.Get();

third_party/blink/renderer/core/page/focus_controller.h

@@ -62,6 +62,9 @@ class CORE_EXPORT FocusController final
   LocalFrame* FocusedFrame() const;
   Frame* FocusedOrMainFrame() const;
 
+  // Clears |focused_frame_| if it's been detached.
+  void FrameDetached(Frame* detached_frame);
+
   // Finds the focused HTMLFrameOwnerElement, if any, in the provided frame.
   // An HTMLFrameOwnerElement is considered focused if the frame it owns, or
   // one of its descendant frames, is currently focused.