Should not call FocusController::setFocusedFrame() for detached frame

third_party/WebKit/Source/core/page/FocusController.cpp

Index: third_party/WebKit/Source/core/page/FocusController.cpp
diff --git a/third_party/WebKit/Source/core/page/FocusController.cpp b/third_party/WebKit/Source/core/page/FocusController.cpp
index c7d11a226f30bdcff86214c2462d43f70a5ee4cc..2c2534ebe74041031cfe925614a8211dd0fe307c 100644
--- a/third_party/WebKit/Source/core/page/FocusController.cpp
+++ b/third_party/WebKit/Source/core/page/FocusController.cpp
@@ -772,6 +772,11 @@ void FocusController::focusDocumentView(Frame* frame, bool notifyEmbedder)
             dispatchFocusEvent(*document, *focusedElement);
     }
 
+    // dispatchBlurEvent/dispatchFocusEvent could have changed the focused frame, or
+    // detached the frame.
+    if (newFocusedFrame && !newFocusedFrame->view())
+        return;
+
     setFocusedFrame(frame, notifyEmbedder);
 }
 
@@ -1115,9 +1120,12 @@ void FocusController::setActive(bool active)
         // Invalidate all custom scrollbars because they support the CSS
         // window-active attribute. This should be applied to the entire page so
         // we invalidate from the root FrameView instead of just the focused.
-        if (FrameView* view = toLocalFrame(frame)->localFrameRoot()->document()->view())
+        LocalFrame* localFrame = toLocalFrame(frame);
+        if (FrameView* view = localFrame->localFrameRoot()->document()->view()) {
             view->invalidateAllCustomScrollbarsOnActiveChanged();
-        toLocalFrame(frame)->selection().pageActivationChanged();
+            if (localFrame->selection().isAvailable())

[yosin_UTC9, 2016/07/21 07:22:36]

This check should be parallel of
|localFrame->localFrameRoot()->document()->view()| as original code and to
support |frame| in remote frame.

[yosin_UTC9, 2016/07/21 07:30:28]

BTW, do we really need to change this for crbug.com/626750?
Does attached layout test covert this change?

[kochi, 2016/07/21 09:00:33]

Originally this was added for the case that appeared in clusterfuzz's backtrace,
which calls blink::WebViewImpl::setIsActive() (which calls this function)
directly from IPC message pump, that could bypass the code change in
focusDocumentView().

But as far as I tested, neither of the original fuzzer test case nor the layout
test
crashed without this chunk.  So I think the IPC happens after setFocusedFrame()
on the detached frame.

Ideally we should check all the callsites of setFocusedFrame(), but will do in
later
CL if necessary.  Let's fix the failing case for clusterfuzz.

+                localFrame->selection().pageActivationChanged();
+        }
     }
 }