Skip to content

GitLab

T
tangled
Commit a4afac27 authored by Mikhail Naganov's avatar Mikhail Naganov
Browse files
Options

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

This reverts commit 69538c3b.

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/534093004

Cr-Commit-Position: refs/heads/master@{#293146}
parent b8745f68
Hide whitespace changes
Inline Side-by-side
Showing with 5 additions and 256 deletions
android_webview/android_webview.gyp
View file @ a4afac27
......@@ -128,7 +128,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',
......@@ -250,8 +249,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',
......
android_webview/browser/renderer_host/aw_render_view_host_ext.cc
View file @ a4afac27
......@@ -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(),
......
android_webview/browser/renderer_host/aw_render_view_host_ext.h
View file @ a4afac27
......@@ -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;
......
android_webview/common/render_view_messages.h
View file @ a4afac27
......@@ -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.
......
android_webview/java/src/org/chromium/android_webview/AwContents.java
View file @ a4afac27
......@@ -1065,8 +1065,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
......@@ -1105,11 +1103,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.
......@@ -2497,7 +2496,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);
......
android_webview/javatests/src/org/chromium/android_webview/test/LoadUrlTest.java
View file @ a4afac27
......@@ -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));
}
}
android_webview/native/aw_contents.cc
View file @ a4afac27
......@@ -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) {
......
android_webview/native/aw_contents.h
View file @ a4afac27
......@@ -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);
......
android_webview/renderer/aw_content_renderer_client.cc
View file @ a4afac27
......@@ -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(
......
android_webview/renderer/aw_content_renderer_client.h
View file @ a4afac27
......@@ -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
......
android_webview/renderer/aw_execution_termination_filter.cc deleted 100644 → 0
View file @ b8745f68
// 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
android_webview/renderer/aw_execution_termination_filter.h deleted 100644 → 0
View file @ b8745f68
// 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_
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment