New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 733222 link

Starred by 3 users
Status: Fixed
Owner:
ajit...@samsung.com
Closed: Aug 2017
Cc:
ifratric@google.com
█ kochi@chromium.org
█ msrchandra@chromium.org
changwan@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug

Blocking:
issue 410785



Sign in to add a comment

Null-dereference READ in blink::FocusController::NextFocusableElementInForm

Project Member Reported by ClusterFuzz, Jun 14 2017 
Detailed report: https://clusterfuzz.com/testcase?key=6639436485099520

Fuzzer: ifratric-browserfuzzer-v3
Job Type: windows_asan_chrome_no_sandbox
Platform Id: windows

Crash Type: Null-dereference READ
Crash Address: 0x00000000
Crash State:
  blink::FocusController::NextFocusableElementInForm
  blink::InputMethodController::TextInputFlags
  blink::InputMethodController::TextInputInfo
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=windows_asan_chrome_no_sandbox&range=479114:479249

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6639436485099520


Issue filed automatically.

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

Comment 1 by msrchandra@chromium.org, Jun 15 2017

Cc: msrchandra@chromium.org
Components: Blink>Editing
Labels: Test-Predator-Correct-CLs
Owner: ajith.v@chromium.org
Status: Assigned (was: Untriaged)
Assigning to the concern owner from Predator results --
The result is a list of CLs that change the crashed files. 

Author: ajith.v
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/8d80ad1410dcb23f55c323a97e57eed750a611dc
Time: Tue Jun 13 20:21:13 2017
Lines 1072 of file FocusController.cpp which potentially caused crash are changed in this cl (frame #2, "blink::FocusController::NextFocusableElementInForm"). 

Lines 1139-1146 of file InputMethodController.cpp which potentially caused crash are changed in this cl (frame #3, "blink::InputMethodController::TextInputFlags").
Minimum distance from crash line to modified line: 0. (file: InputMethodController.cpp, crashed on: 1139, modified: 1139).

@ajith.v  -- Could you please look into the issue, kindly re-assign if this is not related to your changes.
Thank You.

Comment 2 by yosin@chromium.org, Jun 16 2017

Components: -Blink>Editing Blink>Focus
Owner: kochi@chromium.org
Change components to Blink>Focus since null-deref is occurred in FocusController::NextFocusableElementInForm().

It seems this is yet another GO/NEXT related bug.

Comment 3 by kochi@chromium.org, Jun 16 2017

Blocking: 410785
Cc: kochi@chromium.org
Owner: ajit...@samsung.com
I think as the original CL was reverted this doesn't happen for now, but
Ajith, could you take a look?

Comment 4 by kochi@chromium.org, Jun 16 2017

Cc: changwan@chromium.org

Comment 5 by ajit...@samsung.com, Jun 21 2017 

This will gets fixed with https://codereview.chromium.org/2948593002/

Comment 6 by ajit...@samsung.com, Jun 21 2017

Status: Started (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 23 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4eb6812cc312e4ea919518a4e3087944e31b3b72

commit 4eb6812cc312e4ea919518a4e3087944e31b3b72
Author: ajith.v <ajith.v@samsung.com>
Date: Fri Jun 23 13:38:16 2017

Relanding [Android] Adding Smart GO/NEXT feature in Chrome

Smart Go/Next brings better user experience to the user during form submitting applications.
For navigating between form elements, user can use NEXT/PREVIOUS button from IME
without touching on individual fields. This will avoid unnecessary form submissions before
filling or visiting all fields in the form.

Additionally it will save user time and avoid redundant network requests before actually
filling/attending entire fields in the form

Design Document: https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSPNg/edit?usp=sharing

Initial patch is reviewed @ https://codereview.chromium.org/2839993002/

BUG= 410785 , 648986 ,  733222 

Review-Url: https://codereview.chromium.org/2948593002
Cr-Commit-Position: refs/heads/master@{#481868}

[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/content/browser/android/ime_adapter_android.cc
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/content/browser/android/ime_adapter_android.h
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/content/common/frame_messages.h
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestInputMethodManagerWrapper.java
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/content/renderer/render_frame_impl.h
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/content/test/data/android/input/input_forms.html
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/Source/core/editing/InputMethodController.cpp
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/Source/core/exported/WebViewTest.cpp
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/Source/core/page/FocusController.cpp
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/Source/core/page/FocusController.h
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/Source/core/page/FocusControllerTest.cpp
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[add] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_disabled_and_readonly_elements.html
[add] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_key_event_listeners.html
[add] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_tabindex_elements.html
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/public/BUILD.gn
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/public/platform/WebFocusType.h
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/public/platform/WebTextInputType.h
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/public/web/WebLocalFrame.h

Comment 8 by ajit...@samsung.com, Jun 27 2017

Status: Fixed (was: Started)
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 28 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fa75e2abb23205caa6d7f61edc89e672e83f6089

commit fa75e2abb23205caa6d7f61edc89e672e83f6089
Author: Takayoshi Kochi <kochi@chromium.org>
Date: Wed Jun 28 08:22:13 2017

Revert of Relanding [Android] Adding Smart GO/NEXT feature in Chrome (patchset #4 id:80001 of https://codereview.chromium.org/2948593002/ )

Reason for revert:
This caused perf regression on browse_social_facebook_infinite_scroll
test on most platforms.

See  crbug.com/737388  for details.

Original issue's description:
> Relanding [Android] Adding Smart GO/NEXT feature in Chrome
>
> Smart Go/Next brings better user experience to the user during form submitting applications.
> For navigating between form elements, user can use NEXT/PREVIOUS button from IME
> without touching on individual fields. This will avoid unnecessary form submissions before
> filling or visiting all fields in the form.
>
> Additionally it will save user time and avoid redundant network requests before actually
> filling/attending entire fields in the form
>
> Design Document: https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSPNg/edit?usp=sharing
>
> Initial patch is reviewed @ https://codereview.chromium.org/2839993002/
>
> BUG= 410785 , 648986 ,  733222 
>
> Review-Url: https://codereview.chromium.org/2948593002
> Cr-Commit-Position: refs/heads/master@{#481868}
> Committed: https://chromium.googlesource.com/chromium/src/+/4eb6812cc312e4ea919518a4e3087944e31b3b72

TBR=yosin@chromium.org,aelias@chromium.org,changwan@chromium.org,dcheng@chromium.org,kochi@chromium.org,tedchoc@chromium.org,tkent@chromium.org,yfriedman@chromium.org,nasko@chromium.org,ajith.v@samsung.com
BUG= 410785 , 648986 ,  733222 

patch from issue 2955283002 at patchset 1 (http://crrev.com/2955283002#ps1)

Change-Id: Idd861244a71d079581f7a1a6337c1ebad245ca71
Reviewed-on: https://chromium-review.googlesource.com/551697
Reviewed-by: Takayoshi Kochi <kochi@chromium.org>
Commit-Queue: Takayoshi Kochi <kochi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#482918}
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/content/browser/android/ime_adapter_android.cc
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/content/browser/android/ime_adapter_android.h
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/content/common/frame_messages.h
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestInputMethodManagerWrapper.java
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/content/renderer/render_frame_impl.h
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/content/test/data/android/input/input_forms.html
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/third_party/WebKit/Source/core/editing/InputMethodController.cpp
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/third_party/WebKit/Source/core/exported/WebViewTest.cpp
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/third_party/WebKit/Source/core/page/FocusController.cpp
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/third_party/WebKit/Source/core/page/FocusController.h
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/third_party/WebKit/Source/core/page/FocusControllerTest.cpp
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[delete] https://crrev.com/b919d95a7f8be9ca291d6bbff942cfb42d08d5a1/third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_disabled_and_readonly_elements.html
[delete] https://crrev.com/b919d95a7f8be9ca291d6bbff942cfb42d08d5a1/third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_key_event_listeners.html
[delete] https://crrev.com/b919d95a7f8be9ca291d6bbff942cfb42d08d5a1/third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_tabindex_elements.html
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/third_party/WebKit/public/BUILD.gn
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/third_party/WebKit/public/platform/WebFocusType.h
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/third_party/WebKit/public/platform/WebTextInputType.h
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/third_party/WebKit/public/web/WebLocalFrame.h

Comment 10 by ajit...@samsung.com, Jun 29 2017

Status: Assigned (was: Fixed)
CL reverted, hence opening the bug.
Project Member

Comment 11 by bugdroid1@chromium.org, Jul 3 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/933aca75233fbb5145c29c1e5a06e5f36c308cc0

commit 933aca75233fbb5145c29c1e5a06e5f36c308cc0
Author: ajith.v <ajith.v@samsung.com>
Date: Mon Jul 03 17:05:26 2017

Relanding [Android] Adding Smart GO/NEXT feature in Chrome

Smart Go/Next brings better user experience to the user during form submitting applications.
For navigating between form elements, user can use NEXT/PREVIOUS button from IME
without touching on individual fields. This will avoid unnecessary form submissions before
filling or visiting all fields in the form.

Additionally it will save user time and avoid redundant network requests before actually
filling/attending entire fields in the form

Design Document: https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSPNg/edit?usp=sharing

Initial patch is reviewed @ https://codereview.chromium.org/2839993002/ and
https://codereview.chromium.org/2948593002/

Scrolling performance is getting effected due to call to
NextFocusableElementInForm() from InputMethodController. hence splitting it into multiple patches.
This is the initial landing without enabling functionality.
In next patch functionality will be enabled with necessary test coverage.

BUG= 410785 ,  648986 ,  733222 

Review-Url: https://codereview.chromium.org/2967493002
Cr-Commit-Position: refs/heads/master@{#484020}

[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/content/browser/android/ime_adapter_android.cc
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/content/browser/android/ime_adapter_android.h
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/content/common/frame_messages.h
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/content/renderer/render_frame_impl.h
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/third_party/WebKit/Source/core/page/FocusController.cpp
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/third_party/WebKit/Source/core/page/FocusController.h
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/third_party/WebKit/Source/core/page/FocusControllerTest.cpp
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/third_party/WebKit/public/BUILD.gn
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/third_party/WebKit/public/platform/WebFocusType.h
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/third_party/WebKit/public/platform/WebTextInputType.h
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/third_party/WebKit/public/web/WebLocalFrame.h

Comment 12 by ajit...@samsung.com, Jul 4 2017

Status: Started (was: Assigned)
Project Member

Comment 13 by bugdroid1@chromium.org, Aug 8 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/041c0b075176fb9bca94fd67fc1b6053a4df2e1c

commit 041c0b075176fb9bca94fd67fc1b6053a4df2e1c
Author: AJITH KUMAR V <ajith.v@samsung.com>
Date: Tue Aug 08 10:39:20 2017

[Android] Relanding Smart GO NEXT feature in Android Chrome 2/2

This is second patch of Smart GO NEXT feature. Initial patch is
landed @ https://codereview.chromium.org/2967493002/

Design Document: https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSPNg/edit?usp=sharing

Performance regression is getting tackled using the triggering of Focus Controller
call only if element focus is changed, otherwise continue to use previously cached value.
This will save unwanted tree traversal in every frame update.

BUG= 410785 ,  648986 ,  733222 ,  737388 

Change-Id: Ib2c7343a6ec7dea18c7cfa5ac283ac4d29e3a4cb
Reviewed-on: https://chromium-review.googlesource.com/574514
Commit-Queue: AJITH KUMAR V <ajith.v@samsung.com>
Reviewed-by: Changwan Ryu <changwan@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Takayoshi Kochi <kochi@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492588}
[modify] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java
[modify] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java
[modify] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestInputMethodManagerWrapper.java
[modify] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/content/renderer/render_widget.cc
[modify] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/content/renderer/render_widget.h
[modify] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/content/test/data/android/input/input_forms.html
[modify] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/third_party/WebKit/Source/core/editing/InputMethodController.cpp
[modify] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/third_party/WebKit/Source/core/editing/InputMethodController.h
[modify] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/third_party/WebKit/Source/core/exported/WebInputMethodControllerImpl.cpp
[modify] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/third_party/WebKit/Source/core/exported/WebInputMethodControllerImpl.h
[modify] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/third_party/WebKit/Source/core/exported/WebViewTest.cpp
[add] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/third_party/WebKit/Source/core/testing/data/advance_focus_in_form_with_disabled_and_readonly_elements.html
[add] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/third_party/WebKit/Source/core/testing/data/advance_focus_in_form_with_key_event_listeners.html
[add] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/third_party/WebKit/Source/core/testing/data/advance_focus_in_form_with_tabindex_elements.html
[modify] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/third_party/WebKit/public/web/WebInputMethodController.h

Comment 14 by ajit...@samsung.com, Aug 8 2017

Status: Fixed (was: Started)

Comment 15 by benhenry@chromium.org, Sep 29 2017

Components: Blink>HTML>Focus

Comment 16 by benhenry@chromium.org, Sep 29 2017

Components: -Blink>Focus
►

Sign in to add a comment