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

Issue 732936 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug-Regression



Sign in to add a comment

AndroidWebView: Crash Report - blink::FocusController::AdvanceFocusDirectionallyInContainer

Project Member Reported by aluo@chromium.org, Jun 13 2017

Issue description

Only seen in 60.0.3112.20 so far.

Stack:
0xdebac2c2	(libwebviewchromium.so -FocusController.cpp:1347 )	blink::FocusController::AdvanceFocusDirectionallyInContainer(blink::Node*, blink::LayoutRect const&, blink::WebFocusType)
0xdebac3fb	(libwebviewchromium.so -FocusController.cpp:1389 )	blink::FocusController::AdvanceFocusDirectionally(blink::WebFocusType)
0xde9febee	(libwebviewchromium.so -FocusController.h:77 )	blink::KeyboardEventManager::DefaultArrowEventHandler(blink::KeyboardEvent*, blink::Node*)
0x0defacec		
0xde9fef21	(libwebviewchromium.so -KeyboardEventManager.cpp:309 )	blink::KeyboardEventManager::DefaultKeyboardEventHandler(blink::KeyboardEvent*, blink::Node*)
0xde9f99c8	(libwebviewchromium.so -EventHandler.cpp:1992 )	blink::EventHandler::DefaultKeyboardEventHandler(blink::KeyboardEvent*)
0xde7e808d	(libwebviewchromium.so -Node.cpp:2310 )	blink::Node::DefaultEventHandler(blink::Event*)
0xde930a25	(libwebviewchromium.so -HTMLElement.cpp:1120 )	blink::HTMLElement::DefaultEventHandler(blink::Event*)
0xde8a37ce	(libwebviewchromium.so -EventDispatcher.cpp:287 )	blink::EventDispatcher::DispatchEventPostProcess(blink::EventDispatchHandlingState*)
0xde8a3b69	(libwebviewchromium.so -EventDispatcher.cpp:159 )	blink::EventDispatcher::Dispatch()
0xde8a327c	(libwebviewchromium.so -EventDispatchMediator.cpp:51 )	blink::EventDispatchMediator::DispatchEvent(blink::EventDispatcher&) const
0xde8a3570	(libwebviewchromium.so -EventDispatcher.cpp:59 )	blink::EventDispatcher::DispatchEvent(blink::Node&, blink::EventDispatchMediator*)
0xde7e470d	(libwebviewchromium.so -Node.cpp:2181 )	blink::Node::DispatchEventInternal(blink::Event*)
0xde8a8120	(libwebviewchromium.so -EventTarget.cpp:483 )	blink::EventTarget::DispatchEvent(blink::Event*)
0xde9ff285	(libwebviewchromium.so -KeyboardEventManager.cpp:238 )	blink::KeyboardEventManager::KeyEvent(blink::WebKeyboardEvent const&)
0xde9f998e	(libwebviewchromium.so -EventHandler.cpp:1987 )	blink::EventHandler::KeyEvent(blink::WebKeyboardEvent const&)
0xddbd1d77	(libwebviewchromium.so -WebViewImpl.cpp:1164 )	blink::WebViewImpl::HandleKeyEvent(blink::WebKeyboardEvent const&)
0xddb938d1	(libwebviewchromium.so -PageWidgetDelegate.cpp:182 )	blink::PageWidgetDelegate::HandleInputEvent(blink::PageWidgetEventHandler&, blink::WebCoalescedInputEvent const&, blink::LocalFrame*)
0xddbce450	(libwebviewchromium.so -WebViewImpl.cpp:2249 )	blink::WebViewImpl::HandleInputEvent(blink::WebCoalescedInputEvent const&)
0xddbc6376	(libwebviewchromium.so -WebViewFrameWidget.cpp:94 )	blink::WebViewFrameWidget::HandleInputEvent(blink::WebCoalescedInputEvent const&)
0xdef0b22c	(libwebviewchromium.so -render_widget_input_handler.cc:315 )	content::RenderWidgetInputHandler::HandleInputEvent(blink::WebCoalescedInputEvent const&, ui::LatencyInfo const&, content::InputEventDispatchType)
0xdef77517	(libwebviewchromium.so -render_widget.cc:831 )	content::RenderWidget::HandleInputEvent(blink::WebCoalescedInputEvent const&, ui::LatencyInfo const&, content::InputEventDispatchType)
0xdef6e373	(libwebviewchromium.so -render_view_impl.cc:2686 )	content::RenderViewImpl::HandleInputEvent(blink::WebCoalescedInputEvent const&, ui::LatencyInfo const&, content::InputEventDispatchType)
0xdef08534	(libwebviewchromium.so -main_thread_event_queue.cc:522 )	content::MainThreadEventQueue::HandleEventOnMainThread(blink::WebCoalescedInputEvent const&, ui::LatencyInfo const&, content::InputEventDispatchType)
0xdef08670	(libwebviewchromium.so -main_thread_event_queue.cc:125 )	content::QueuedWebInputEvent::Dispatch(content::MainThreadEventQueue*)
0xdef08fbb	(libwebviewchromium.so -main_thread_event_queue.cc:395 )	content::MainThreadEventQueue::DispatchEvents()
0xdef0824d	(libwebviewchromium.so -bind_internal.h:214 )	base::internal::Invoker<base::internal::BindState<void (content::MainThreadEventQueue::*)(), scoped_refptr<content::MainThreadEventQueue> >, void ()>::Run(base::internal::BindStateBase*)
0xdbc96d09	(libwebviewchromium.so -callback.h:91 )	base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*)
0xdd971457	(libwebviewchromium.so -task_queue_manager.cc:531 )	blink::scheduler::TaskQueueManager::ProcessTaskFromWorkQueue(blink::scheduler::internal::WorkQueue*, bool, blink::scheduler::LazyNow, base::TimeTicks*)
0xdd971e74	(libwebviewchromium.so -task_queue_manager.cc:329 )	blink::scheduler::TaskQueueManager::DoWork(bool)
0xdd96e57e	(libwebviewchromium.so -bind_internal.h:214 )	base::internal::Invoker<base::internal::BindState<void (blink::scheduler::TaskQueueManager::*)(bool), base::WeakPtr<blink::scheduler::TaskQueueManager>, bool>, void ()>::Run(base::internal::BindStateBase*)
0x0defacec		
0xdbc96d09	(libwebviewchromium.so -callback.h:91 )	base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*)
0xdbcbab3f	(libwebviewchromium.so -message_loop.cc:409 )	base::MessageLoop::RunTask(base::PendingTask*)
0xdbcbb369	(libwebviewchromium.so -message_loop.cc:420 )	base::MessageLoop::DeferOrRunPendingTask(base::PendingTask)
0xdbcbb858	(libwebviewchromium.so -message_loop.cc:508 )	base::MessageLoop::DoWork()
0xdbcbcec8	(libwebviewchromium.so -message_pump_default.cc:33 )	base::MessagePumpDefault::Run(base::MessagePump::Delegate*)
0xdbcb94e6	(libwebviewchromium.so -message_loop.cc:360 )	base::MessageLoop::Run()
0xdbce217f	(libwebviewchromium.so -run_loop.cc:111 )	base::RunLoop::Run()
0xdbd12c7b	(libwebviewchromium.so -thread.cc:255 )	base::Thread::Run(base::RunLoop*)
0xdbd13654	(libwebviewchromium.so -thread.cc:338 )	base::Thread::ThreadMain()
0xdbd0b5e5	(libwebviewchromium.so -platform_thread_posix.cc:71 )	ThreadFunc
0xf1d57384	(libc.so + 0x00084384 )

All reports:
https://crash.corp.google.com/browse?q=product.name%3D%27AndroidWebView%27%20%20AND%20custom_data.ChromeCrashProto.magic_signature_1.name%3D%27blink%3A%3AFocusController%3A%3AAdvanceFocusDirectionallyInContainer%27&ignore_case=true&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D#samplereports

Product name: AndroidWebView
Magic Signature: blink::FocusController::AdvanceFocusDirectionallyInContainer

Current link:
https://crash.corp.google.com/browse?q=product.name%3D'AndroidWebView'%20%20AND%20custom_data.ChromeCrashProto.magic_signature_1.name%3D'blink%3A%3AFocusController%3A%3AAdvanceFocusDirectionallyInContainer'%20AND%20ReportID%3D'e5b7066e40000000'&ignore_case=true&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D#3


Search properties:
product.name: AndroidWebView
custom_data.chromecrashproto.magic_signature_1.name: blink::FocusController::AdvanceFocusDirectionallyInContainer
reportid: e5b7066e40000000

Metadata :
Product Name: AndroidWebView
Product Version: 60.0.3112.20
Report ID: e5b7066e40000000
Report Time: Tue, 13 Jun 2017 07:13:25 GMT
Uptime: 459894 ms
Cumulative Uptime: 0 ms
User Email: 
OS Name: Android
OS Version: 0.0.0 Linux 3.10.20-gfa77dae7e53 #1 SMP PREEMPT Fri Mar 24 23:11:04 UTC 2017 i686
CPU Architecture: x86
CPU Info:  family 0 model 0 stepping 0


 

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

Owner: aelias@chromium.org
Status: Assigned (was: Untriaged)
Looks like IME/focus event related
Components: -Blink Blink>Input

Comment 3 by aelias@chromium.org, Jun 15 2017

Cc: aelias@chromium.org
Owner: hu...@opera.com
Crashing on new lines added in https://codereview.chromium.org/2616623002 "Do not send redundant selectionchange-events (decouple focus)"

Comment 4 by aelias@chromium.org, Jun 15 2017

Labels: -Restrict-View-Google

Comment 5 by hu...@opera.com, Jun 16 2017

Cc: yosin@chromium.org joth@chromium.org
Owner: ----
Status: Available (was: Assigned)
So here some non-touch Android device that ran WebView crashed... Maybe an Android TV?

mSpatialNavigationEnabled = !context.getPackageManager().hasSystemFeature(
  PackageManager.FEATURE_TOUCHSCREEN);

Seems like the pointer given by FocusedFrame() is invalid? It's hard for me to guess the root cause here (I cannot access Google's internal stats page).

If this bug is critical and we cannot find its the root cause, we could delete these line:

  if (!element->IsTextControl() && !HasEditableStyle(*element->ToNode())) {
    // To fulfill the expectation of spatial-navigation/snav-input.html
    // we clear selection when spatnav moves focus away from a text-field.
    FocusedFrame()->Selection().Clear();
  }

... and live with snav-textarea.html failing* for a little while.

We do want to get rid of this call to Clear() anyway. It was added only to make this test case pass. We would then need to fix snav-textarea.html properly - but that's a less critical bug that I guess can be done later? 

* When deleting the block I notice all tests but snav-textarea.html pass:
  trunk/src$ python third_party/WebKit/Tools/Scripts/run-webkit-tests -t PcRelease LayoutTests/fast/spatial-navigation
  Regressions: Unexpected text-only failures (1)
    fast/spatial-navigation/snav-textarea.html [ Failure ]

Comment 6 by qin...@chromium.org, Jun 16 2017

Owner: yosin@chromium.org
assining to yosin since he is one of the reviewer of that CL

Comment 7 by yosin@chromium.org, Jun 19 2017

Owner: kochi@chromium.org
Status: Assigned (was: Available)
Is this causes by the patch[1]?

[1] http://crrev.com/2839993002: [Android] Adding Smart GO/NEXT feature in Chrome

Comment 8 by kochi@chromium.org, Jun 19 2017

Components: -Blink>Input Blink>Focus
Might be, but the patch[1] should only be in M61 for a few days.
So I believe this is different because it happens for M60.

Comment 9 by kochi@chromium.org, Jun 19 2017

Cc: kolc...@opera.com
As this seems related to spatial navigation, 

Comment 10 by kochi@chromium.org, Jun 19 2017

^^ adding kolczyk@opera in cc.

Comment 11 by hu...@opera.com, Jun 19 2017

(kolczyk@ is on summer vacation now.)

Comment 12 by kochi@chromium.org, Jun 19 2017

Looked at the crash data, and they are mostly coming from Android_WebView
and Chrome_Android. Some are from Android TV, some from Nexus Player.
But most of the data doesn't have crashing URL so we have to guess
the condition.
Many of them are crashing with SIGSEGV @ 0x0000008c , so FocusedFrame()
seems returning nullptr in these cases.

I'm not familiar with the spatial navigation, but does changing it to
  if (FocusedFrame())
    FocusedFrame()->Selection().Clear();
cause any problem?

Some functions called from AdvanceFocusDirectionallyInContainer() like
FindFocusCandidateInContainer() checks null of FocusedFrame(), so
I guess there are conditions that this function is called without
current focused element/frame.

hugoh@, what do you think?

Comment 13 by hu...@opera.com, Jun 19 2017

Cc: hu...@opera.com
Adding a null check sounds OK. It should not cause any problem to spatnav. 

Comment 14 by kochi@chromium.org, Jun 20 2017

Status: Fixed (was: Assigned)
CL for adding null check landed.
https://chromium-review.googlesource.com/c/539358/

The issue should be fixed for builds from now on.
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-60; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-60 label, otherwise remove Merge-TBD label. Thanks.

Comment 16 by aluo@chromium.org, Jun 20 2017

Labels: -Merge-TBD Merge-Request-60
Project Member

Comment 17 by sheriffbot@chromium.org, Jun 20 2017

Labels: -Merge-Request-60 Hotlist-Merge-Review Merge-Review-60
This bug requires manual review: M60 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-60 Merge-Approved-60
Merge approved for M60 branch 3112.
Project Member

Comment 19 by bugdroid1@chromium.org, Jun 21 2017

Labels: -merge-approved-60 merge-merged-3112
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5d015f9c3212d7ed28f55a875a09f0c398a1cef4

commit 5d015f9c3212d7ed28f55a875a09f0c398a1cef4
Author: Takayoshi Kochi <kochi@chromium.org>
Date: Wed Jun 21 08:35:26 2017

Null check FocusedFrame in spatial navigation

Fixes null pointer dereference.

Bug:  732936 
Change-Id: I4ffc475dba4f0268faaaa7720ac27558d73ce8bb
Reviewed-on: https://chromium-review.googlesource.com/539358
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Takayoshi Kochi <kochi@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#480691}
Review-Url: https://codereview.chromium.org/2948063002 .
Cr-Commit-Position: refs/branch-heads/3112@{#419}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}

[modify] https://crrev.com/5d015f9c3212d7ed28f55a875a09f0c398a1cef4/third_party/WebKit/Source/core/page/FocusController.cpp

Comment 20 by aluo@chromium.org, Aug 15 2017

Status: Verified (was: Fixed)
Crashes stopped after, 60.0.3112.33, marking as verified.
Components: Blink>HTML>Focus
Components: -Blink>Focus

Sign in to add a comment