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

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

 /*
  * Copyright (C) 2006, 2007 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Nuanti Ltd.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 754 matching lines @@
     }
 
     LocalFrame* newFocusedFrame = (frame && frame->isLocalFrame()) ? toLocalFrame(frame) : nullptr;
     if (newFocusedFrame && newFocusedFrame->view()) {
         Document* document = newFocusedFrame->document();
         Element* focusedElement = document ? document->focusedElement() : nullptr;
         if (focusedElement)
             dispatchFocusEvent(*document, *focusedElement);
     }
 
+    // dispatchBlurEvent/dispatchFocusEvent could have changed the focused frame, or
+    // detached the frame.
+    if (newFocusedFrame && !newFocusedFrame->view())
+        return;
+
     setFocusedFrame(frame, notifyEmbedder);
 }
 
 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).
     if (m_focusedFrame && m_focusedFrame->isRemoteFrame())
@@ skipping 323 matching lines @@
     if (m_isActive == active)
         return;
 
     m_isActive = active;
 
     Frame* frame = focusedOrMainFrame();
     if (frame->isLocalFrame()) {
         // 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();
+        }
     }
 }
 
 static void updateFocusCandidateIfNeeded(WebFocusType type, const FocusCandidate& current, FocusCandidate& candidate, FocusCandidate& closest)
 {
     DCHECK(candidate.visibleNode->isElementNode());
     DCHECK(candidate.visibleNode->layoutObject());
 
     // Ignore iframes that don't have a src attribute
     if (frameOwnerElement(candidate) && (!frameOwnerElement(candidate)->contentFrame() || candidate.rect.isEmpty()))
@@ skipping 177 matching lines @@
     return consumed;
 }
 
 DEFINE_TRACE(FocusController)
 {
     visitor->trace(m_page);
     visitor->trace(m_focusedFrame);
 }
 
 } // namespace blink