summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMikhail Naganov <mnaganov@google.com>2014-09-08 10:23:33 +0100
committerMikhail Naganov <mnaganov@google.com>2014-09-08 10:23:33 +0100
commita648f0a2a2db1c831cd3760bba309908e69697e7 (patch)
tree97a53ed293b9530669a6e86ca5bc8670faa7912f
parentc622a8f4bfc6aaf5ef2ccfe28cc3249c35af26ef (diff)
downloadchromium_org-a648f0a2a2db1c831cd3760bba309908e69697e7.tar.gz
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
-rw-r--r--android_webview/android_webview.gyp3
-rw-r--r--android_webview/browser/renderer_host/aw_render_view_host_ext.cc4
-rw-r--r--android_webview/browser/renderer_host/aw_render_view_host_ext.h2
-rw-r--r--android_webview/common/render_view_messages.h4
-rw-r--r--android_webview/java/src/org/chromium/android_webview/AwContents.java10
-rw-r--r--android_webview/javatests/src/org/chromium/android_webview/test/LoadUrlTest.java70
-rw-r--r--android_webview/native/aw_contents.cc5
-rw-r--r--android_webview/native/aw_contents.h1
-rw-r--r--android_webview/renderer/aw_content_renderer_client.cc13
-rw-r--r--android_webview/renderer/aw_content_renderer_client.h9
-rw-r--r--android_webview/renderer/aw_execution_termination_filter.cc76
-rw-r--r--android_webview/renderer/aw_execution_termination_filter.h64
12 files changed, 5 insertions, 256 deletions
diff --git a/android_webview/android_webview.gyp b/android_webview/android_webview.gyp
index f8821c00b3..a3faee3f80 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 676616af78..0428baa24f 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 @@ void AwRenderViewHostExt::SetJsOnlineProperty(bool network_up) {
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 773e504a6a..635c7fce38 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 @@ class AwRenderViewHostExt : public content::WebContentsObserver,
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 7592fb7327..9354db5d82 100644
--- a/android_webview/common/render_view_messages.h
+++ b/android_webview/common/render_view_messages.h
@@ -74,10 +74,6 @@ IPC_MESSAGE_ROUTED1(AwViewMsg_SetBackgroundColor,
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 a1728fef2d..cd4b145a36 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 @@ public class AwContents {
* @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 @@ public class AwContents {
}
}
- 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 @@ public class AwContents {
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 77fb3704fd..4a6bab55c7 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 @@ public class LoadUrlTest extends AwTestBase {
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 4c8a4f288a..ed44b77176 100644
--- a/android_webview/native/aw_contents.cc
+++ b/android_webview/native/aw_contents.cc
@@ -1143,11 +1143,6 @@ void AwContents::SetExtraHeadersForUrl(JNIEnv* env, jobject obj,
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 da13d6f0b8..a390375b88 100644
--- a/android_webview/native/aw_contents.h
+++ b/android_webview/native/aw_contents.h
@@ -127,7 +127,6 @@ class AwContents : public FindHelper::Listener,
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 f4f0b1efb5..9ae387b3a5 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 @@ void AwContentRendererClient::RenderThreadStarted() {
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 dc02cce981..30f44b440f 100644
--- a/android_webview/renderer/aw_content_renderer_client.h
+++ b/android_webview/renderer/aw_content_renderer_client.h
@@ -16,10 +16,7 @@ class VisitedLinkSlave;
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 @@ class AwContentRendererClient : public content::ContentRendererClient,
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 75f130dbcb..0000000000
--- 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 14b88b1c50..0000000000
--- 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_