Cherry-pick: Revert "Merge 281715 "[Android WebView] Terminate execution of stuck JS ...""
The approach implemented in the patch is prone to crashing Blink bindings code.
See more comments in the bugs.
Bug: 13509205, 17175122, 17173843
Original description:
Revert "Merge 281715 "[Android WebView] Terminate execution of stuck JS ...""
This reverts commit 8508a2f18909b0768d273bb9161aee749f47beca.
As described in the bug, the proposed approach is invalid, as Blink's bindings
code isn't ready to deal with terminated scripts properly, resulting in random
crashes. Hence reverting the patch.
BUG=390906
Review URL: https://codereview.chromium.org/536593004
Cr-Commit-Position: refs/branch-heads/2062@{#587}
Cr-Branched-From: 2e531f7c26d0d9e2aa0cced17a35eea6687dc58c-refs/heads/master@{#278856}
Change-Id: I6805a85cefef55492d5b3934b4fbd9263c1d004e
diff --git a/android_webview/android_webview.gyp b/android_webview/android_webview.gyp
index f8821c0..a3faee3 100644
--- a/android_webview/android_webview.gyp
+++ b/android_webview/android_webview.gyp
@@ -116,7 +116,6 @@
'../printing/printing.gyp:printing',
'../skia/skia.gyp:skia',
'../third_party/WebKit/public/blink.gyp:blink',
- '../v8/tools/gyp/v8.gyp:v8',
'../ui/gl/gl.gyp:gl',
'../ui/shell_dialogs/shell_dialogs.gyp:shell_dialogs',
'../webkit/common/gpu/webkit_gpu.gyp:webkit_gpu',
@@ -238,8 +237,6 @@
'public/browser/draw_gl.h',
'renderer/aw_content_renderer_client.cc',
'renderer/aw_content_renderer_client.h',
- 'renderer/aw_execution_termination_filter.cc',
- 'renderer/aw_execution_termination_filter.h',
'renderer/aw_key_systems.cc',
'renderer/aw_key_systems.h',
'renderer/aw_permission_client.cc',
diff --git a/android_webview/browser/renderer_host/aw_render_view_host_ext.cc b/android_webview/browser/renderer_host/aw_render_view_host_ext.cc
index 676616a..0428baa 100644
--- a/android_webview/browser/renderer_host/aw_render_view_host_ext.cc
+++ b/android_webview/browser/renderer_host/aw_render_view_host_ext.cc
@@ -99,10 +99,6 @@
Send(new AwViewMsg_SetJsOnlineProperty(network_up));
}
-void AwRenderViewHostExt::SendCheckRenderThreadResponsiveness() {
- Send(new AwViewMsg_CheckRenderThreadResponsiveness());
-}
-
void AwRenderViewHostExt::RenderViewCreated(
content::RenderViewHost* render_view_host) {
Send(new AwViewMsg_SetBackgroundColor(web_contents()->GetRoutingID(),
diff --git a/android_webview/browser/renderer_host/aw_render_view_host_ext.h b/android_webview/browser/renderer_host/aw_render_view_host_ext.h
index 773e504..635c7fc 100644
--- a/android_webview/browser/renderer_host/aw_render_view_host_ext.h
+++ b/android_webview/browser/renderer_host/aw_render_view_host_ext.h
@@ -77,8 +77,6 @@
void SetBackgroundColor(SkColor c);
void SetJsOnlineProperty(bool network_up);
- void SendCheckRenderThreadResponsiveness();
-
private:
// content::WebContentsObserver implementation.
virtual void RenderViewCreated(content::RenderViewHost* view_host) OVERRIDE;
diff --git a/android_webview/common/render_view_messages.h b/android_webview/common/render_view_messages.h
index 7592fb7..9354db5 100644
--- a/android_webview/common/render_view_messages.h
+++ b/android_webview/common/render_view_messages.h
@@ -74,10 +74,6 @@
IPC_MESSAGE_CONTROL1(AwViewMsg_SetJsOnlineProperty,
bool /* network_up */)
-// Sent prior to making a navigation via loadUrl to make sure that
-// render thread isn't stuck in a loop induced by JavaScript code.
-IPC_MESSAGE_CONTROL0(AwViewMsg_CheckRenderThreadResponsiveness)
-
//-----------------------------------------------------------------------------
// RenderView messages
// These are messages sent from the renderer to the browser process.
diff --git a/android_webview/java/src/org/chromium/android_webview/AwContents.java b/android_webview/java/src/org/chromium/android_webview/AwContents.java
index a1728fe..cd4b145 100644
--- a/android_webview/java/src/org/chromium/android_webview/AwContents.java
+++ b/android_webview/java/src/org/chromium/android_webview/AwContents.java
@@ -1043,8 +1043,6 @@
* @param params Parameters for this load.
*/
public void loadUrl(LoadUrlParams params) {
- if (mNativeAwContents == 0) return;
-
if (params.getLoadUrlType() == LoadUrlParams.LOAD_TYPE_DATA &&
!params.isBaseUrlDataScheme()) {
// This allows data URLs with a non-data base URL access to file:///android_asset/ and
@@ -1083,11 +1081,12 @@
}
}
- nativeSetExtraHeadersForUrl(
- mNativeAwContents, params.getUrl(), params.getExtraHttpRequestHeadersString());
+ if (mNativeAwContents != 0) {
+ nativeSetExtraHeadersForUrl(
+ mNativeAwContents, params.getUrl(), params.getExtraHttpRequestHeadersString());
+ }
params.setExtraHeaders(new HashMap<String, String>());
- nativeSendCheckRenderThreadResponsiveness(mNativeAwContents);
mContentViewCore.loadUrl(params);
// The behavior of WebViewClassic uses the populateVisitedLinks callback in WebKit.
@@ -2475,7 +2474,6 @@
private native void nativeClearView(long nativeAwContents);
private native void nativeSetExtraHeadersForUrl(long nativeAwContents,
String url, String extraHeaders);
- private native void nativeSendCheckRenderThreadResponsiveness(long nativeAwContents);
private native void nativeInvokeGeolocationCallback(
long nativeAwContents, boolean value, String requestingFrame);
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/LoadUrlTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/LoadUrlTest.java
index 77fb370..4a6bab5 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/LoadUrlTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/LoadUrlTest.java
@@ -4,12 +4,9 @@
package org.chromium.android_webview.test;
-import android.test.suitebuilder.annotation.LargeTest;
import android.test.suitebuilder.annotation.SmallTest;
import android.util.Pair;
-import junit.framework.Assert;
-
import org.apache.http.Header;
import org.apache.http.HttpRequest;
import org.chromium.android_webview.AwContents;
@@ -334,71 +331,4 @@
if (webServer != null) webServer.shutdown();
}
}
-
- private static class TestController {
- private final Object mLock = new Object();
- private boolean mIsReady = false;
- public void notifyPageIsReady() {
- synchronized (mLock) {
- mIsReady = true;
- mLock.notify();
- }
- }
- public void waitUntilIsReady() {
- synchronized (mLock) {
- while (!mIsReady) {
- try {
- mLock.wait(WAIT_TIMEOUT_MS);
- } catch (Exception e) {
- continue;
- }
- if (!mIsReady) {
- Assert.fail("Wait timed out");
- }
- }
- mIsReady = false;
- }
- }
- }
-
- // Verify that it is possible to interrupt JS scripts stuck in an infinite loop
- // by calling loadUrl on a WebView.
- @LargeTest
- @Feature({"AndroidWebView"})
- public void testLoadUrlInterruptsLoopedScripts() throws Throwable {
- final String infiniteLoopPage =
- "<html><head>" +
- " <script>" +
- " function infiniteLoop() {" +
- " test.notifyPageIsReady();" +
- " while(1);" +
- " }" +
- " </script>" +
- "</head><body onload='setTimeout(infiniteLoop, 0)'>" +
- "</body></html>";
- final String simplePage = "<html><body onload='test.notifyPageIsReady()'></body></html>";
- final String expectedTitle = "PASS";
- final String pageWithTitle = "<html><body onload='document.title=\"" + expectedTitle +
- "\"; test.notifyPageIsReady()'></body></html>";
-
- final AwTestContainerView testContainerView =
- createAwTestContainerViewOnMainSync(new TestAwContentsClient());
- final AwContents awContents = testContainerView.getAwContents();
- getAwSettingsOnUiThread(awContents).setJavaScriptEnabled(true);
- final TestController testController = new TestController();
- getInstrumentation().runOnMainSync(new Runnable() {
- @Override
- public void run() {
- awContents.addPossiblyUnsafeJavascriptInterface(testController, "test", null);
- }
- });
- loadDataAsync(awContents, infiniteLoopPage, "text/html", false);
- testController.waitUntilIsReady();
- loadDataAsync(awContents, simplePage, "text/html", false);
- testController.waitUntilIsReady();
- // Load another page that runs JS to make sure that the WebView is still functional.
- loadDataAsync(awContents, pageWithTitle, "text/html", false);
- testController.waitUntilIsReady();
- assertEquals(expectedTitle, getTitleOnUiThread(awContents));
- }
}
diff --git a/android_webview/native/aw_contents.cc b/android_webview/native/aw_contents.cc
index 4c8a4f2..ed44b77 100644
--- a/android_webview/native/aw_contents.cc
+++ b/android_webview/native/aw_contents.cc
@@ -1143,11 +1143,6 @@
extra_headers);
}
-void AwContents::SendCheckRenderThreadResponsiveness(JNIEnv* env, jobject obj) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- render_view_host_ext_->SendCheckRenderThreadResponsiveness();
-}
-
void AwContents::SetJsOnlineProperty(JNIEnv* env,
jobject obj,
jboolean network_up) {
diff --git a/android_webview/native/aw_contents.h b/android_webview/native/aw_contents.h
index da13d6f..a390375 100644
--- a/android_webview/native/aw_contents.h
+++ b/android_webview/native/aw_contents.h
@@ -127,7 +127,6 @@
void ClearView(JNIEnv* env, jobject obj);
void SetExtraHeadersForUrl(JNIEnv* env, jobject obj,
jstring url, jstring extra_headers);
- void SendCheckRenderThreadResponsiveness(JNIEnv* env, jobject obj);
void DrawGL(AwDrawGLInfo* draw_info);
diff --git a/android_webview/renderer/aw_content_renderer_client.cc b/android_webview/renderer/aw_content_renderer_client.cc
index f4f0b1e..9ae387b 100644
--- a/android_webview/renderer/aw_content_renderer_client.cc
+++ b/android_webview/renderer/aw_content_renderer_client.cc
@@ -7,7 +7,6 @@
#include "android_webview/common/aw_resource.h"
#include "android_webview/common/render_view_messages.h"
#include "android_webview/common/url_constants.h"
-#include "android_webview/renderer/aw_execution_termination_filter.h"
#include "android_webview/renderer/aw_key_systems.h"
#include "android_webview/renderer/aw_permission_client.h"
#include "android_webview/renderer/aw_render_frame_ext.h"
@@ -32,7 +31,6 @@
#include "third_party/WebKit/public/platform/WebURLError.h"
#include "third_party/WebKit/public/platform/WebURLRequest.h"
#include "third_party/WebKit/public/web/WebFrame.h"
-#include "third_party/WebKit/public/web/WebKit.h"
#include "third_party/WebKit/public/web/WebNavigationType.h"
#include "third_party/WebKit/public/web/WebSecurityPolicy.h"
#include "url/gurl.h"
@@ -63,17 +61,6 @@
visited_link_slave_.reset(new visitedlink::VisitedLinkSlave);
thread->AddObserver(visited_link_slave_.get());
-
- execution_termination_filter_ = new AwExecutionTerminationFilter(
- thread->GetIOMessageLoopProxy(),
- thread->GetMessageLoop()->message_loop_proxy());
- thread->AddFilter(execution_termination_filter_.get());
- thread->AddObserver(this);
-}
-
-void AwContentRendererClient::WebKitInitialized() {
- execution_termination_filter_->SetRenderThreadIsolate(
- blink::mainThreadIsolate());
}
bool AwContentRendererClient::HandleNavigation(
diff --git a/android_webview/renderer/aw_content_renderer_client.h b/android_webview/renderer/aw_content_renderer_client.h
index dc02cce..30f44b4 100644
--- a/android_webview/renderer/aw_content_renderer_client.h
+++ b/android_webview/renderer/aw_content_renderer_client.h
@@ -16,10 +16,7 @@
namespace android_webview {
-class AwExecutionTerminationFilter;
-
-class AwContentRendererClient : public content::ContentRendererClient,
- public content::RenderProcessObserver {
+class AwContentRendererClient : public content::ContentRendererClient {
public:
AwContentRendererClient();
virtual ~AwContentRendererClient();
@@ -53,13 +50,9 @@
blink::WebNavigationPolicy default_policy,
bool is_redirect) OVERRIDE;
- // content::RenderProcessObserver implementation.
- virtual void WebKitInitialized() OVERRIDE;
-
private:
scoped_ptr<AwRenderProcessObserver> aw_render_process_observer_;
scoped_ptr<visitedlink::VisitedLinkSlave> visited_link_slave_;
- scoped_refptr<AwExecutionTerminationFilter> execution_termination_filter_;
};
} // namespace android_webview
diff --git a/android_webview/renderer/aw_execution_termination_filter.cc b/android_webview/renderer/aw_execution_termination_filter.cc
deleted file mode 100644
index 75f130d..0000000
--- a/android_webview/renderer/aw_execution_termination_filter.cc
+++ /dev/null
@@ -1,76 +0,0 @@
-// Copyright 2014 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "android_webview/renderer/aw_execution_termination_filter.h"
-
-#include <v8.h>
-
-#include "android_webview/common/render_view_messages.h"
-#include "base/message_loop/message_loop_proxy.h"
-
-namespace {
-const int kTerminationTimeoutInSeconds = 3;
-} // namespace
-
-namespace android_webview {
-
-AwExecutionTerminationFilter::AwExecutionTerminationFilter(
- const scoped_refptr<base::MessageLoopProxy>& io_message_loop,
- const scoped_refptr<base::MessageLoopProxy>& main_message_loop)
- : io_message_loop_(io_message_loop),
- main_message_loop_(main_message_loop),
- render_thread_isolate_(NULL) {
-}
-
-AwExecutionTerminationFilter::~AwExecutionTerminationFilter() {
-}
-
-void AwExecutionTerminationFilter::SetRenderThreadIsolate(
- v8::Isolate* isolate) {
- render_thread_isolate_ = isolate;
-}
-
-bool AwExecutionTerminationFilter::OnMessageReceived(
- const IPC::Message& message) {
- bool handled = true;
- IPC_BEGIN_MESSAGE_MAP(AwExecutionTerminationFilter, message)
- IPC_MESSAGE_HANDLER(AwViewMsg_CheckRenderThreadResponsiveness,
- OnCheckRenderThreadResponsiveness)
- IPC_MESSAGE_UNHANDLED(handled = false)
- IPC_END_MESSAGE_MAP()
- return handled;
-}
-
-void AwExecutionTerminationFilter::OnCheckRenderThreadResponsiveness() {
- termination_timer_.Start(
- FROM_HERE,
- base::TimeDelta::FromSeconds(kTerminationTimeoutInSeconds),
- base::Bind(&AwExecutionTerminationFilter::TerminateExecution, this));
- // Post a request to stop the timer via render thread's message loop
- // to ensure that render thread is responsive.
- main_message_loop_->PostTask(
- FROM_HERE,
- base::Bind(&AwExecutionTerminationFilter::StopTimerOnMainThread,
- this));
-}
-
-void AwExecutionTerminationFilter::StopTimerOnMainThread() {
- io_message_loop_->PostTask(
- FROM_HERE,
- base::Bind(&AwExecutionTerminationFilter::StopTimer, this));
-}
-
-void AwExecutionTerminationFilter::StopTimer() {
- termination_timer_.Stop();
-}
-
-void AwExecutionTerminationFilter::TerminateExecution() {
- if (render_thread_isolate_) {
- LOG(WARNING) << "Trying to terminate JavaScript execution because "
- "renderer is unresponsive";
- v8::V8::TerminateExecution(render_thread_isolate_);
- }
-}
-
-} // namespace android_webview
diff --git a/android_webview/renderer/aw_execution_termination_filter.h b/android_webview/renderer/aw_execution_termination_filter.h
deleted file mode 100644
index 14b88b1..0000000
--- a/android_webview/renderer/aw_execution_termination_filter.h
+++ /dev/null
@@ -1,64 +0,0 @@
-// Copyright 2014 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef ANDROID_WEBVIEW_RENDERER_AW_EXECUTION_TERMINATION_FILTER_H_
-#define ANDROID_WEBVIEW_RENDERER_AW_EXECUTION_TERMINATION_FILTER_H_
-
-#include "base/memory/scoped_ptr.h"
-#include "base/timer/timer.h"
-#include "ipc/message_filter.h"
-
-namespace base {
-class MessageLoopProxy;
-}
-
-namespace v8 {
-class Isolate;
-}
-
-namespace android_webview {
-
-// The purpose of AwExecutionTerminationFilter is to attempt to terminate
-// any JavaScript code that is stuck in a loop before doing a navigation
-// originating from a Andoird WebView URL loading functions.
-//
-// This is how it works. AwExecutionTerminationFilter is created on render
-// thread. It listens on IO thread for navigation requests coming from
-// AwContents.loadUrl calls. On each such a request, it posts a delayed
-// cancellable task on the IO thread's message loop and, at the same time, posts
-// a cancellation task on the render thread's message loop. If render thread
-// is not stuck, the cancellation task runs and cancels the delayed task.
-// Otherwise, the delayed task runs and terminates execution of JS code
-// from the IO thread.
-class AwExecutionTerminationFilter : public IPC::MessageFilter {
- public:
- AwExecutionTerminationFilter(
- const scoped_refptr<base::MessageLoopProxy>& io_message_loop,
- const scoped_refptr<base::MessageLoopProxy>& main_message_loop);
-
- void SetRenderThreadIsolate(v8::Isolate* isolate);
-
- private:
- virtual ~AwExecutionTerminationFilter();
-
- // IPC::MessageFilter methods.
- virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
-
- void OnCheckRenderThreadResponsiveness();
- void StopTimerOnMainThread();
- void StopTimer();
- void TerminateExecution();
-
- const scoped_refptr<base::MessageLoopProxy> io_message_loop_;
- const scoped_refptr<base::MessageLoopProxy> main_message_loop_;
-
- v8::Isolate* render_thread_isolate_;
- base::OneShotTimer<AwExecutionTerminationFilter> termination_timer_;
-
- DISALLOW_COPY_AND_ASSIGN(AwExecutionTerminationFilter);
-};
-
-} // namespace android_webview
-
-#endif // ANDROID_WEBVIEW_RENDERER_AW_EXECUTION_TERMINATION_FILTER_H_
