Move asserts in FocusNavigationController.

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 346 matching lines @@
 inline bool hasCustomFocusLogic(const Element& element)
 {
     return element.isHTMLElement() && toHTMLElement(element).hasCustomFocusLogic();
 }
 
 inline bool isShadowHostWithoutCustomFocusLogic(const Element& element)
 {
     return isShadowHost(element) && !hasCustomFocusLogic(element);
 }
 
-#if ENABLE(ASSERT)
-inline bool isNonFocusableShadowHost(const Element& element)
-{
-    return isShadowHostWithoutCustomFocusLogic(element) && !element.isFocusable();
-}
-#endif
-
 inline bool isNonKeyboardFocusableShadowHost(const Element& element)
 {
     return isShadowHostWithoutCustomFocusLogic(element) && !(element.shadowRootIfV1() ? element.isFocusable() : element.isKeyboardFocusable());
 }
 
 inline bool isKeyboardFocusableShadowHost(const Element& element)
 {
     return isShadowHostWithoutCustomFocusLogic(element) && element.isKeyboardFocusable();
 }
 
@@ skipping 255 matching lines @@
             break;
         ASSERT(element != foundElement);
         element = foundElement;
     }
     return element;
 }
 
 Element* findFocusableElementAcrossFocusScopesForward(ScopedFocusNavigation& scope)
 {
     Element* current = scope.currentElement();
-    ASSERT(!current || !isNonFocusableShadowHost(*current));
     Element* found;
     if (current && isShadowHostWithoutCustomFocusLogic(*current)) {
         ScopedFocusNavigation innerScope = ScopedFocusNavigation::ownedByShadowHost(*current);
         Element* foundInInnerFocusScope = findFocusableElementRecursivelyForward(innerScope);
         found = foundInInnerFocusScope ? foundInInnerFocusScope : findFocusableElementRecursivelyForward(scope);
     } else {
         found = findFocusableElementRecursivelyForward(scope);
     }
 
     // If there's no focusable element to advance to, move up the focus scopes until we find one.
     ScopedFocusNavigation currentScope = scope;
     while (!found) {
         Element* owner = currentScope.owner();
         if (!owner)
             break;
         currentScope = ScopedFocusNavigation::createFor(*owner);
         found = findFocusableElementRecursivelyForward(currentScope);
     }
     return findFocusableElementDescendingDownIntoFrameDocument(WebFocusTypeForward, found);
 }
 
 Element* findFocusableElementAcrossFocusScopesBackward(ScopedFocusNavigation& scope)
 {
-    ASSERT(!scope.currentElement() || !isNonFocusableShadowHost(*scope.currentElement()));
     Element* found = findFocusableElementRecursivelyBackward(scope);
 
     // If there's no focusable element to advance to, move up the focus scopes until we find one.
     ScopedFocusNavigation currentScope = scope;
     while (!found) {
         Element* owner = currentScope.owner();
         if (!owner)
             break;
         currentScope = ScopedFocusNavigation::createFor(*owner);
         if (isKeyboardFocusableShadowHost(*owner) && !isShadowHostDelegatesFocus(*owner)) {
@@ skipping 222 matching lines @@
 }
 
 bool FocusController::advanceFocusInDocumentOrder(LocalFrame* frame, Element* start, WebFocusType type, bool initialFocus, InputDeviceCapabilities* sourceCapabilities)
 {
     ASSERT(frame);
     Document* document = frame->document();
     document->updateDistribution();
 
     Element* current = start;
     if (!current && !initialFocus)
         current = document->sequentialFocusNavigationStartingPoint(type);

[hayato, 2016/05/11 12:44:44]

I think we can distinguish special cases, where `current` is not focusable,
around here.

It looks that L907 - L913 are the source of the exceptional cases.

If we can distinguish exceptional cases and usual cases (where element is
focusable) by using bool flag around here, we do not have to remove ASSERTION
for usual cases.

Could you try to preserve ASSERTION?

[kochi, 2016/05/12 02:09:21]

Done.

 
     // FIXME: Not quite correct when it comes to focus transitions leaving/entering the WebView itself
     bool caretBrowsing = frame->settings() && frame->settings()->caretBrowsingEnabled();
 
     if (caretBrowsing && !current)
         current = adjustToElement(frame->selection().start().anchorNode(), type);
 
     document->updateLayoutIgnorePendingStylesheets();
     ScopedFocusNavigation scope = current ? ScopedFocusNavigation::createFor(*current) : ScopedFocusNavigation::createForDocument(*document);
     Element* element = findFocusableElementAcrossFocusScopes(type, scope);
@@ skipping 45 matching lines @@
         // If contentFrame is remote, continue the search for focusable
         // elements in that frame's process.
         // clearFocusedElement() fires events that might detach the
         // contentFrame, hence the need to null-check it again.
         if (owner->contentFrame() && owner->contentFrame()->isRemoteFrame())
             toRemoteFrame(owner->contentFrame())->advanceFocus(type, frame);
 
         return true;
     }
 
+    ASSERT(element->isFocusable());
+
     // FIXME: It would be nice to just be able to call setFocusedElement(element)
     // here, but we can't do that because some elements (e.g. HTMLInputElement
     // and HTMLTextAreaElement) do extra work in their focus() methods.
     Document& newDocument = element->document();
 
     if (&newDocument != document) {
         // Focus is going away from this document, so clear the focused element.
         document->clearFocusedElement();
     }
 
@@ skipping 314 matching lines @@
     return consumed;
 }
 
 DEFINE_TRACE(FocusController)
 {
     visitor->trace(m_page);
     visitor->trace(m_focusedFrame);
 }
 
 } // namespace blink