[Android WebView] Terminate execution of stuck JS code on navigation requests

[Android WebView] Terminate execution of stuck JS code on navigation requests

Allow WebView.loadUrl-family functions to terminate execution of JS code that
seems to be stuck in a loop.

BUG=390906
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=281715

[benm (inactive), 2014-07-02 14:07:00]

On 2014/07/02 13:51:08, Mikhail Naganov (Cr) wrote:

Is it possible to move this into the android_webview layer? I think we can
install message filters from AwContentBrowserClient::RenderProcessWillLaunch.

[benm (inactive), 2014-07-02 14:43:39]

https://codereview.chromium.org/366913006/diff/1/content/renderer/android/exe...
File content/renderer/android/execution_termination_filter.cc (right):

https://codereview.chromium.org/366913006/diff/1/content/renderer/android/exe...
content/renderer/android/execution_termination_filter.cc:50:
main_message_loop_->PostTask(
can you add a comment as to why we don't post to the io thread directly?

https://codereview.chromium.org/366913006/diff/1/content/renderer/android/exe...
content/renderer/android/execution_termination_filter.cc:69:
v8::V8::TerminateExecution(render_thread_isolate_);
nit: shall we add a log message here (maybe a console message too?) to note to
developers/bugreport readers that we've terminated a run away script?

[mnaganov (inactive), 2014-07-02 16:39:28]

OK, I have moved everything under android_webview.

https://codereview.chromium.org/366913006/diff/1/content/renderer/android/exe...
File content/renderer/android/execution_termination_filter.cc (right):

https://codereview.chromium.org/366913006/diff/1/content/renderer/android/exe...
content/renderer/android/execution_termination_filter.cc:50:
main_message_loop_->PostTask(
On 2014/07/02 14:43:39, benm wrote:
> can you add a comment as to why we don't post to the io thread directly?

To ensure that render thread is responsive. I've added a comment. There is also
a class-level comment describing how it works.

https://codereview.chromium.org/366913006/diff/1/content/renderer/android/exe...
content/renderer/android/execution_termination_filter.cc:69:
v8::V8::TerminateExecution(render_thread_isolate_);
On 2014/07/02 14:43:39, benm wrote:
> nit: shall we add a log message here (maybe a console message too?) to note to
> developers/bugreport readers that we've terminated a run away script?

Added a LOG message. I'm not sure console message will work, because renderer
thread is unresponsive at the moment.

[benm (inactive), 2014-07-03 16:40:28]

nice! looks great overall.

https://codereview.chromium.org/366913006/diff/20001/android_webview/java/src...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/366913006/diff/20001/android_webview/java/src...
android_webview/java/src/org/chromium/android_webview/AwContents.java:1073: if
(mNativeAwContents != 0) {
I wonder if we should have an early out at the top of the function if
nNativeAwContents is null. The two blocks together like this (on line 1067) look
a little strange ...

[mnaganov (inactive), 2014-07-03 17:06:54]

https://codereview.chromium.org/366913006/diff/20001/android_webview/java/src...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/366913006/diff/20001/android_webview/java/src...
android_webview/java/src/org/chromium/android_webview/AwContents.java:1073: if
(mNativeAwContents != 0) {
On 2014/07/03 16:40:28, benm wrote:
> I wonder if we should have an early out at the top of the function if
> nNativeAwContents is null. The two blocks together like this (on line 1067)
look
> a little strange ... 

OK. As mNativeAwContents can be 0 either on early initialization stages or after
a call to destroy(), it really doesn't make much sense to try doing anything in
this case.

[mnaganov (inactive), 2014-07-03 17:07:49]

palmer@: Can you please look at android_webview/common/render_view_messages.h?

[benm (inactive), 2014-07-03 21:31:55]

lgtm

[palmer, 2014-07-07 17:47:33]

IPC security LGTM.

[mnaganov (inactive), 2014-07-08 08:54:27]

The CQ bit was checked by mnaganov@chromium.org

[commit-bot: I haz the power, 2014-07-08 08:55:21]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mnaganov@chromium.org/366913006/60001

[commit-bot: I haz the power, 2014-07-08 10:46:28]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)

[commit-bot: I haz the power, 2014-07-08 10:57:59]

Change committed as 281715