Adding support for Smart GO NEXT feature in Android Chrome

Source/web/WebViewImpl.cpp

 /*
  * Copyright (C) 2011, 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 98 matching lines @@
 #include "platform/graphics/Color.h"
 #include "platform/graphics/FirstPaintInvalidationTracking.h"
 #include "platform/graphics/Image.h"
 #include "platform/graphics/ImageBuffer.h"
 #include "platform/scroll/ScrollbarTheme.h"
 #include "platform/weborigin/SchemeRegistry.h"
 #include "public/platform/Platform.h"
 #include "public/platform/WebCompositeAndReadbackAsyncCallback.h"
 #include "public/platform/WebDragData.h"
 #include "public/platform/WebFloatPoint.h"
+#include "public/platform/WebFocusType.h"
 #include "public/platform/WebGestureCurve.h"
 #include "public/platform/WebImage.h"
 #include "public/platform/WebLayerTreeView.h"
 #include "public/platform/WebURLRequest.h"
 #include "public/platform/WebVector.h"
 #include "public/web/WebAXObject.h"
 #include "public/web/WebActiveWheelFlingParameters.h"
 #include "public/web/WebAutofillClient.h"
 #include "public/web/WebBeginFrameArgs.h"
 #include "public/web/WebFrameClient.h"
@@ skipping 2383 matching lines @@
                 flags |= WebTextInputFlagAutocapitalizeCharacters;
             else if (autocapitalize == words)
                 flags |= WebTextInputFlagAutocapitalizeWords;
             else if (autocapitalize == sentences)
                 flags |= WebTextInputFlagAutocapitalizeSentences;
             else
                 ASSERT_NOT_REACHED();
         }
     }
 
+    if (wantEnterEvents(element))
+        flags |= WebTextInputFlagWantEnterEvents;
+
+    if (haveNextInput(element, WebFocusTypeForward))
+        flags |= WebTextInputFlagHaveNextInput;
+
+    if (haveNextInput(element, WebFocusTypeBackward))
+        flags |= WebTextInputFlagHavePreviousInput;
+
     return flags;
 }
 
+Element* WebViewImpl::haveNextInput(Element* element, WebFocusType focusType)

[tkent, 2015/04/14 23:27:19]

The function name is not reasonable.
- The name looks like it returns a bool, but it returns an element.  The name
should be |nextInput|.
- The function returns not only <input>, but also <textarea> and contenteditable
elements.  The name should be |nextEditableElement|.
- A TODO comment says this will be expanded to other form controls.  The name
should be |nextFocusableElement| or something.

[AKV, 2015/04/16 19:05:19]

Done. nextFocusableElement is ideal. Thanks :)

+{
+    if (!element->isFormControlElement())

[tkent, 2015/04/14 23:27:19]

Do you want to reject a contenteditable element though this function can return
a contenteditable element?

[AKV, 2015/04/16 19:05:19]

I care about elements which are inside the form. If the ContentEditable is child
of a form, then also isFormControlElement() function returns false ? Could you
please clarify on this ?

[tkent, 2015/04/17 04:12:35]

IsFormControlElement() return false for contenteditable elements.  They are not
form controls.

+        return nullptr;
+
+    HTMLFormControlElement* formControlElement = toHTMLFormControlElement(element);
+    Node* parentFormNode = reinterpret_cast<Node*>(formControlElement->formOwner());

[tkent, 2015/04/14 23:27:19]

|parentFormNode| is not a good name because it's not a parent, and it's an
element.  |formOwner| is a good candidate.
You don't need to use reinterpret_cast<>.

[AKV, 2015/04/16 19:05:19]

Done.

+    if (!parentFormNode)
+        return nullptr;
+
+    Node* nextNode = element;
+    while ((nextNode = page()->focusController().findFocusableNode(focusType, *nextNode))) {
+        if (!nextNode->isDescendantOf(parentFormNode))

[tkent, 2015/04/14 23:27:19]

This doesn't work for the following case.

<form id=f1>
<input >  <!-- This is the current focused element -->
</form>
<input form=f1>

We should check HTMLFormControlElement::formOwner() equivalence.

[kochi, 2015/04/15 01:09:18]

Hmm, I was not aware of this way of declaring form element outside
<form></form>.
It means that until the focus controller reaches browser chrome (out of
content),
we have to search for a focusable element associated with the current form.

IIUC this is not a performance-critical path, so it would not matter much for
iterating nodes exhaustively. It might be worth exploring the solution to
iterate through HTMLFormElement::associatedElements().

[AKV, 2015/04/16 19:05:19]

Thanks. I have taken care this case by checking formOwner.

[AKV, 2015/04/16 19:05:19]

Thanks. I have taken care this case by checking formOwner.

+            return nullptr;
+        LayoutObject* layout = nextNode->layoutObject();
+        if (nextNode->isContentEditable() || (layout && layout->isTextControl())) {
+            // TODO(AKV) Extend it for Select element, Radio button and Check boxes
+            if (nextNode->isElementNode())

[tkent, 2015/04/14 23:27:19]

No need to check isElementNode.  Only elements are focusable.

[AKV, 2015/04/16 19:05:19]

Done.

+                return toElement(nextNode);
+        }
+    }
+    return nullptr;
+}
+
+bool WebViewImpl::wantEnterEvents(Element* element)

[tkent, 2015/04/14 23:27:19]

The argument should be |const Element&|.

The function name and the implementation doesn't match.  The name should be
|isListeningToKeyboardEvents| if we follow the implementation.  What's the
intention of this function?

[AKV, 2015/04/16 19:05:19]

I wanted to check whether the element has any listener for "Enter Key". If no
enter key listener, we don't want to show Enter key from IME side. I will check
specific case of Enter Key instead of whole key events. Do you have any
suggestion to check Enter key listener is present or not ? Thanks.

[tkent, 2015/04/17 04:12:35]

No way to check only Enter key.

BTW, If we don't show the enter key in IME, it's impossible for users to submit
a form without a submit button like:

<form action="... ">
<input name=foo>
</form>

+{
+    if (!element->isFormControlElement())
+        return false;
+
+    return (element->hasEventListeners(EventTypeNames::keydown) || element->hasEventListeners(EventTypeNames::keypress) || element->hasEventListeners(EventTypeNames::keystatuseschange) || element->hasEventListeners(EventTypeNames::keyup));

[tkent, 2015/04/14 23:27:19]

Why do you check keystatuschange event, which is for encrypted media?

[AKV, 2015/04/16 19:05:19]

Right. this is not required.

+}
+
+void WebViewImpl::advanceFocusToNextInputField(WebFocusType focusType)
+{
+    Element* element = focusedElement();
+    if (!element)
+        return;
+
+    Element* nextElement = haveNextInput(element, focusType);
+    if (!nextElement)
+        return;
+
+    nextElement->scrollIntoViewIfNeeded(true /*centerIfNeeded*/);
+    nextElement->dispatchSimulatedClick(0, SendMouseUpDownEvents);

[tkent, 2015/04/14 23:27:19]

0 -> nullptr.

Why do you need to dispatch a click event?

[AKV, 2015/04/16 19:05:19]

Not required. Thanks.

+    nextElement->focus(false, focusType);

[tkent, 2015/04/14 23:27:19]

The click event handler can delete |nextElement|.  We need to make |nextElement|
|RefPtrWIllBeRawPtr<Element>|.

[AKV, 2015/04/16 19:05:19]

Thanks. I corrected it.

+}
+
 WebString WebViewImpl::inputModeOfFocusedElement()
 {
     if (!RuntimeEnabledFeatures::inputModeAttributeEnabled())
         return WebString();
 
     Element* element = focusedElement();
     if (!element)
         return WebString();
 
     if (isHTMLInputElement(*element)) {
@@ skipping 1977 matching lines @@
     return mainFrameImpl()->frameView()->layoutSize().width() == m_size.width
         || (constraints.minimumScale == constraints.maximumScale && constraints.minimumScale != -1);
 }
 
 void WebViewImpl::forceNextWebGLContextCreationToFail()
 {
     WebGLRenderingContext::forceNextWebGLContextCreationToFail();
 }
 
 } // namespace blink