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

Issue 719395 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocking:
issue 592556



Sign in to add a comment

[JNI crashes] Properly throw Java Exceptions from android_webview::AwContentsClientBridge::AllowCertificateError

Project Member Reported by gsennton@chromium.org, May 8 2017

Issue description


This does happen on a Java-backed thread, and the call stack doesn't look very big:

-logging.cc:759 )	logging::LogMessage::~LogMessage()
-jni_android.cc:243 )	base::android::CheckException(_JNIEnv*)
-jni_generator_helper.h:42 )	android_webview::AwContentsClientBridge::AllowCertificateError(...)
-aw_content_browser_client.cc:393 )	android_webview::AwContentBrowserClient::AllowCertificateError(...)
-ssl_manager.cc:363 )	content::SSLManager::OnCertErrorInternal(...)
-ssl_manager.cc:312 )	content::SSLManager::OnCertError(...)
-ssl_manager.cc:113 )	HandleSSLErrorOnUI
-bind_internal.h:164 )	base::internal::Invoker<base::internal::BindState<void (*)(base::Callback<content::WebContents* (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&, base::WeakPtr<content::SSLErrorHandler::Delegate> const&, content::ResourceType, GURL const&, net::SSLInfo const&, bool), base::Callback<content::WebContents* (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1>, base::WeakPtr<content::SSLErrorHandler::Delegate>, content::ResourceType, GURL, net::SSLInfo, bool>, void ()>::Run(base::internal::BindStateBase*)
-callback.h:68 )	base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*)
-message_loop.cc:423 )	base::MessageLoop::RunTask(base::PendingTask*)
-message_loop.cc:434 )	base::MessageLoop::DeferOrRunPendingTask(base::PendingTask)
-message_loop.cc:527 )	base::MessageLoop::DoWork()
-message_pump_android.cc:44 )	Java_org_chromium_base_SystemMessageHandler_nativeDoRunLoopOnce


so this one might be fairly easy to fix.
It accounts for about 5% of the current JNI exceptions on 58.
 
Owner: gsennton@chromium.org
Status: Assigned (was: Untriaged)
Actually, this one seems to be the easiest one to fix, so I'm gonna go ahead with it.

Comment 2 by sgu...@chromium.org, May 17 2017

actually looking at the stats there are huge spikes of that crash in M53 and M54 where the cert transparency thing happened. So I am guessing there are a lot of apps handling it very wrongly in the code so that they would crash if this call was triggered more often.

Comment 3 by torne@chromium.org, May 17 2017

Yeah, that seems likely; you won't get SSL errors very often during testing.
Project Member

Comment 4 by bugdroid1@chromium.org, May 18 2017

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

commit a98edc8510b39155ec0e0efa1db29b195d487d7e
Author: gsennton <gsennton@chromium.org>
Date: Thu May 18 09:54:33 2017

[Android WebView] Propagate Java exceptions thrown in OnReceivedSslError

The OnReceivedSslError callback is accessed through a JNI call. By
default, JNI calls in Clank and WebView are handled so that when a Java
exception is thrown inside a JNI call we will print the stack trace for
that exception to the logcat when the JNI call returns to native. We
will then intentionally crash in native to avoid any undefined behaviour
caused by the pending Java exception.

So when an app throws a Java exception inside OnReceivedSslError, that
exception is not correctly propagated to Android's feedback mechanism.
With this CL we fix this by posting the callback call to the
current Java handler - so that when an exception is thrown it won't have
to propagate back through JNI and a native stack, instead it propagates
directly through Java to Android's UncaughtExceptionHandler, and from
there on to Android's Feedback mechanism.

BUG= 719395 

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

[modify] https://crrev.com/a98edc8510b39155ec0e0efa1db29b195d487d7e/android_webview/java/src/org/chromium/android_webview/AwContentsClientBridge.java

Status: Fixed (was: Assigned)

Sign in to add a comment