Skip to content

Commit

Permalink
Remove V8RecrusionScope, cleanup call sites.
Browse files Browse the repository at this point in the history
- blink-side usages migrated to v8::MicrotasksScope;
- clients of WebScopedMicrotaskSuppression migrated to v8::MicrotasksScope;
- moved Microtask.{h,cpp} to bindings/core/v8;
- removed extra wrappers from V8DebuggerClient;
- fixed a couple of inspector tests which relied on trace event not emitted from debugger anymore.

BUG=585949

Review URL: https://codereview.chromium.org/1769273004

Cr-Commit-Position: refs/heads/master@{#381103}
  • Loading branch information
dgozman authored and Commit bot committed Mar 14, 2016
1 parent 42655dd commit 133a157
Show file tree
Hide file tree
Showing 76 changed files with 260 additions and 536 deletions.
10 changes: 6 additions & 4 deletions chrome/test/base/v8_unit_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include "base/strings/stringprintf.h"
#include "chrome/common/chrome_paths.h"
#include "third_party/WebKit/public/web/WebKit.h"
#include "third_party/WebKit/public/web/WebScopedMicrotaskSuppression.h"

namespace {

Expand Down Expand Up @@ -96,7 +95,8 @@ bool V8UnitTest::RunJavascriptTestF(const std::string& test_fixture,
v8::Local<v8::Context> context =
v8::Local<v8::Context>::New(isolate, context_);
v8::Context::Scope context_scope(context);
blink::WebScopedMicrotaskSuppression microtasks_scope;
v8::MicrotasksScope microtasks(
isolate, v8::MicrotasksScope::kDoNotRunMicrotasks);

v8::Local<v8::Value> function_property =
context->Global()->Get(v8::String::NewFromUtf8(isolate, "runTest"));
Expand Down Expand Up @@ -211,7 +211,8 @@ void V8UnitTest::ExecuteScriptInContext(const base::StringPiece& script_source,
v8::Local<v8::Context> context =
v8::Local<v8::Context>::New(isolate, context_);
v8::Context::Scope context_scope(context);
blink::WebScopedMicrotaskSuppression microtasks_scope;
v8::MicrotasksScope microtasks(
isolate, v8::MicrotasksScope::kDoNotRunMicrotasks);
v8::Local<v8::String> source =
v8::String::NewFromUtf8(isolate,
script_source.data(),
Expand Down Expand Up @@ -260,7 +261,8 @@ void V8UnitTest::TestFunction(const std::string& function_name) {
v8::Local<v8::Context> context =
v8::Local<v8::Context>::New(isolate, context_);
v8::Context::Scope context_scope(context);
blink::WebScopedMicrotaskSuppression microtasks_scope;
v8::MicrotasksScope microtasks(
isolate, v8::MicrotasksScope::kDoNotRunMicrotasks);

v8::Local<v8::Value> function_property = context->Global()->Get(
v8::String::NewFromUtf8(isolate, function_name.c_str()));
Expand Down
6 changes: 3 additions & 3 deletions components/guest_view/renderer/guest_view_container.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include "content/public/renderer/render_frame.h"
#include "content/public/renderer/render_frame_observer.h"
#include "content/public/renderer/render_view.h"
#include "third_party/WebKit/public/web/WebScopedMicrotaskSuppression.h"

namespace {

Expand Down Expand Up @@ -154,7 +153,7 @@ void GuestViewContainer::HandlePendingResponseCallback(
void GuestViewContainer::RunDestructionCallback(bool embedder_frame_destroyed) {
// Do not attempt to run |destruction_callback_| if the embedder frame was
// destroyed. Trying to invoke callback on RenderFrame destruction results in
// assertion failure when calling WebScopedMicrotaskSuppression.
// assertion failure when calling v8::MicrotasksScope.
if (embedder_frame_destroyed)
return;

Expand All @@ -168,7 +167,8 @@ void GuestViewContainer::RunDestructionCallback(bool embedder_frame_destroyed) {
return;

v8::Context::Scope context_scope(context);
blink::WebScopedMicrotaskSuppression suppression;
v8::MicrotasksScope microtasks(
destruction_isolate_, v8::MicrotasksScope::kDoNotRunMicrotasks);

callback->Call(context->Global(), 0 /* argc */, nullptr);
}
Expand Down
4 changes: 2 additions & 2 deletions components/guest_view/renderer/guest_view_request.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include "content/public/renderer/render_view.h"
#include "third_party/WebKit/public/web/WebLocalFrame.h"
#include "third_party/WebKit/public/web/WebRemoteFrame.h"
#include "third_party/WebKit/public/web/WebScopedMicrotaskSuppression.h"
#include "third_party/WebKit/public/web/WebView.h"

namespace guest_view {
Expand Down Expand Up @@ -43,7 +42,8 @@ void GuestViewRequest::ExecuteCallbackIfAvailable(
return;

v8::Context::Scope context_scope(context);
blink::WebScopedMicrotaskSuppression suppression;
v8::MicrotasksScope microtasks(
isolate(), v8::MicrotasksScope::kDoNotRunMicrotasks);

callback->Call(context->Global(), argc, argv.get());
}
Expand Down
22 changes: 14 additions & 8 deletions content/child/v8_value_converter_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include "base/values.h"
#include "content/child/v8_value_converter_impl.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/WebKit/public/web/WebScopedMicrotaskSuppression.h"
#include "v8/include/v8.h"

namespace content {
Expand Down Expand Up @@ -288,7 +287,8 @@ TEST_F(V8ValueConverterImplTest, ObjectExceptions) {
v8::Local<v8::Context> context =
v8::Local<v8::Context>::New(isolate_, context_);
v8::Context::Scope context_scope(context);
blink::WebScopedMicrotaskSuppression microtasks_scope;
v8::MicrotasksScope microtasks(
isolate_, v8::MicrotasksScope::kDoNotRunMicrotasks);

// Set up objects to throw when reading or writing 'foo'.
const char* source =
Expand Down Expand Up @@ -331,7 +331,8 @@ TEST_F(V8ValueConverterImplTest, ArrayExceptions) {
v8::Local<v8::Context> context =
v8::Local<v8::Context>::New(isolate_, context_);
v8::Context::Scope context_scope(context);
blink::WebScopedMicrotaskSuppression microtasks_scope;
v8::MicrotasksScope microtasks(
isolate_, v8::MicrotasksScope::kDoNotRunMicrotasks);

const char* source = "(function() {"
"var arr = [];"
Expand Down Expand Up @@ -408,7 +409,8 @@ TEST_F(V8ValueConverterImplTest, Prototype) {
v8::Local<v8::Context> context =
v8::Local<v8::Context>::New(isolate_, context_);
v8::Context::Scope context_scope(context);
blink::WebScopedMicrotaskSuppression microtasks_scope;
v8::MicrotasksScope microtasks(
isolate_, v8::MicrotasksScope::kDoNotRunMicrotasks);

const char* source = "(function() {"
"Object.prototype.foo = 'foo';"
Expand All @@ -433,7 +435,8 @@ TEST_F(V8ValueConverterImplTest, StripNullFromObjects) {
v8::Local<v8::Context> context =
v8::Local<v8::Context>::New(isolate_, context_);
v8::Context::Scope context_scope(context);
blink::WebScopedMicrotaskSuppression microtasks_scope;
v8::MicrotasksScope microtasks(
isolate_, v8::MicrotasksScope::kDoNotRunMicrotasks);

const char* source = "(function() {"
"return { foo: undefined, bar: null };"
Expand Down Expand Up @@ -492,7 +495,8 @@ TEST_F(V8ValueConverterImplTest, WeirdProperties) {
v8::Local<v8::Context> context =
v8::Local<v8::Context>::New(isolate_, context_);
v8::Context::Scope context_scope(context);
blink::WebScopedMicrotaskSuppression microtasks_scope;
v8::MicrotasksScope microtasks(
isolate_, v8::MicrotasksScope::kDoNotRunMicrotasks);

const char* source = "(function() {"
"return {"
Expand Down Expand Up @@ -531,7 +535,8 @@ TEST_F(V8ValueConverterImplTest, ArrayGetters) {
v8::Local<v8::Context> context =
v8::Local<v8::Context>::New(isolate_, context_);
v8::Context::Scope context_scope(context);
blink::WebScopedMicrotaskSuppression microtasks_scope;
v8::MicrotasksScope microtasks(
isolate_, v8::MicrotasksScope::kDoNotRunMicrotasks);

const char* source = "(function() {"
"var a = [0];"
Expand All @@ -556,7 +561,8 @@ TEST_F(V8ValueConverterImplTest, UndefinedValueBehavior) {
v8::Local<v8::Context> context =
v8::Local<v8::Context>::New(isolate_, context_);
v8::Context::Scope context_scope(context);
blink::WebScopedMicrotaskSuppression microtasks_scope;
v8::MicrotasksScope microtasks(
isolate_, v8::MicrotasksScope::kDoNotRunMicrotasks);

v8::Local<v8::Object> object;
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#include "content/common/android/gin_java_bridge_value.h"
#include "content/renderer/java/gin_java_bridge_value_converter.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/WebKit/public/web/WebScopedMicrotaskSuppression.h"
#include "v8/include/v8.h"

namespace content {
Expand Down Expand Up @@ -100,7 +99,8 @@ TEST_F(GinJavaBridgeValueConverterTest, TypedArrays) {
v8::Local<v8::Context> context =
v8::Local<v8::Context>::New(isolate_, context_);
v8::Context::Scope context_scope(context);
blink::WebScopedMicrotaskSuppression microtasks_scope;
v8::MicrotasksScope microtasks_scope(
isolate_, v8::MicrotasksScope::kDoNotRunMicrotasks);

scoped_ptr<GinJavaBridgeValueConverter> converter(
new GinJavaBridgeValueConverter());
Expand Down
4 changes: 2 additions & 2 deletions content/renderer/pepper/v8_var_converter_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include "ppapi/shared_impl/var.h"
#include "ppapi/shared_impl/var_tracker.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/WebKit/public/web/WebScopedMicrotaskSuppression.h"
#include "v8/include/v8.h"

using ppapi::ArrayBufferVar;
Expand Down Expand Up @@ -404,7 +403,8 @@ TEST_F(V8VarConverterTest, StrangeDictionaryKeyTest) {
v8::Local<v8::Context> context =
v8::Local<v8::Context>::New(isolate_, context_);
v8::Context::Scope context_scope(context);
blink::WebScopedMicrotaskSuppression microtasks_scope;
v8::MicrotasksScope microtasks(
isolate_, v8::MicrotasksScope::kDoNotRunMicrotasks);

const char* source =
"(function() {"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#include "base/values.h"
#include "extensions/renderer/activity_log_converter_strategy.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/WebKit/public/web/WebScopedMicrotaskSuppression.h"
#include "v8/include/v8.h"

using content::V8ValueConverter;
Expand Down Expand Up @@ -123,7 +122,8 @@ TEST_F(ActivityLogConverterStrategyTest, ConversionTest) {
"};"
"})();";

blink::WebScopedMicrotaskSuppression microtasks_scope;
v8::MicrotasksScope microtasks(
isolate_, v8::MicrotasksScope::kDoNotRunMicrotasks);
v8::Local<v8::Script> script(
v8::Script::Compile(v8::String::NewFromUtf8(isolate_, source)));
v8::Local<v8::Object> v8_object = script->Run().As<v8::Object>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#include "extensions/renderer/guest_view/extensions_guest_view_container.h"

#include "content/public/renderer/render_frame.h"
#include "third_party/WebKit/public/web/WebScopedMicrotaskSuppression.h"
#include "ui/gfx/geometry/size.h"

namespace extensions {
Expand Down Expand Up @@ -56,7 +55,8 @@ void ExtensionsGuestViewContainer::CallElementResizeCallback(
v8::Integer::New(element_resize_isolate_, new_size.height())};

v8::Context::Scope context_scope(context);
blink::WebScopedMicrotaskSuppression suppression;
v8::MicrotasksScope microtasks(
element_resize_isolate_, v8::MicrotasksScope::kDoNotRunMicrotasks);

callback->Call(context->Global(), argc, argv);
}
Expand Down
1 change: 0 additions & 1 deletion extensions/renderer/messaging_bindings.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
#include "extensions/renderer/v8_helpers.h"
#include "third_party/WebKit/public/web/WebDocument.h"
#include "third_party/WebKit/public/web/WebLocalFrame.h"
#include "third_party/WebKit/public/web/WebScopedMicrotaskSuppression.h"
#include "third_party/WebKit/public/web/WebScopedUserGesture.h"
#include "third_party/WebKit/public/web/WebScopedWindowFocusAllowedIndicator.h"
#include "third_party/WebKit/public/web/WebUserGestureIndicator.h"
Expand Down
1 change: 0 additions & 1 deletion extensions/renderer/module_system.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#include "extensions/renderer/v8_helpers.h"
#include "gin/modules/module_registry.h"
#include "third_party/WebKit/public/web/WebFrame.h"
#include "third_party/WebKit/public/web/WebScopedMicrotaskSuppression.h"

namespace extensions {

Expand Down
4 changes: 2 additions & 2 deletions extensions/renderer/safe_builtins.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include "base/strings/stringprintf.h"
#include "extensions/renderer/script_context.h"
#include "extensions/renderer/v8_helpers.h"
#include "third_party/WebKit/public/web/WebScopedMicrotaskSuppression.h"

namespace extensions {

Expand Down Expand Up @@ -201,7 +200,8 @@ class ExtensionImpl : public v8::Extension {
return;
}

blink::WebScopedMicrotaskSuppression microtasks_scope;
v8::MicrotasksScope microtasks(
info.GetIsolate(), v8::MicrotasksScope::kDoNotRunMicrotasks);
v8::Local<v8::Value> return_value;
if (function->Call(context, recv, argc, argv.get()).ToLocal(&return_value))
info.GetReturnValue().Set(return_value);
Expand Down
7 changes: 4 additions & 3 deletions extensions/renderer/script_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include "third_party/WebKit/public/web/WebDocument.h"
#include "third_party/WebKit/public/web/WebFrame.h"
#include "third_party/WebKit/public/web/WebLocalFrame.h"
#include "third_party/WebKit/public/web/WebScopedMicrotaskSuppression.h"
#include "third_party/WebKit/public/web/WebView.h"
#include "v8/include/v8.h"

Expand Down Expand Up @@ -185,7 +184,8 @@ v8::Local<v8::Value> ScriptContext::CallFunction(
v8::EscapableHandleScope handle_scope(isolate());
v8::Context::Scope scope(v8_context());

blink::WebScopedMicrotaskSuppression suppression;
v8::MicrotasksScope microtasks(
isolate(), v8::MicrotasksScope::kDoNotRunMicrotasks);
if (!is_valid_) {
return handle_scope.Escape(
v8::Local<v8::Primitive>(v8::Undefined(isolate())));
Expand Down Expand Up @@ -434,7 +434,8 @@ v8::Local<v8::Value> ScriptContext::RunScript(
return v8::Undefined(isolate());
}

blink::WebScopedMicrotaskSuppression suppression;
v8::MicrotasksScope microtasks(
isolate(), v8::MicrotasksScope::kDoNotRunMicrotasks);
v8::TryCatch try_catch(isolate());
try_catch.SetCaptureMessage(true);
v8::ScriptOrigin origin(
Expand Down
1 change: 0 additions & 1 deletion extensions/renderer/utils_native_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include "base/macros.h"
#include "base/strings/stringprintf.h"
#include "extensions/renderer/script_context.h"
#include "third_party/WebKit/public/web/WebScopedMicrotaskSuppression.h"
#include "third_party/WebKit/public/web/WebSerializedScriptValue.h"

namespace extensions {
Expand Down
4 changes: 2 additions & 2 deletions extensions/renderer/v8_helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include <string.h>

#include "base/strings/string_number_conversions.h"
#include "third_party/WebKit/public/web/WebScopedMicrotaskSuppression.h"
#include "v8/include/v8.h"

namespace extensions {
Expand Down Expand Up @@ -156,7 +155,8 @@ inline bool CallFunction(v8::Local<v8::Context> context,
int argc,
v8::Local<v8::Value> argv[],
v8::Local<v8::Value>* out) {
blink::WebScopedMicrotaskSuppression microtasks_scope;
v8::MicrotasksScope microtasks_scope(
context->GetIsolate(), v8::MicrotasksScope::kDoNotRunMicrotasks);
return function->Call(context, recv, argc, argv).ToLocal(out);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
function wrapCallFunctionForTimeline(f)
{
var script = document.createElement("script");
script.textContent = "(" + f.toString() + ")()\n//# sourceURL=wrapCallFunctionForTimeline.js";
document.body.appendChild(script);
}

var initialize_Timeline = function() {

InspectorTest.preloadPanel("timeline");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@
++knownEvents[event.name];
}
assertGreaterOrEqual(events.length, 10, "Too few trace events recorded");
assertGreaterOrEqual(knownEvents["FunctionCall"], 1, "Too few FunctionCall events");
assertGreaterOrEqual(knownEvents["UpdateLayoutTree"], 1, "Too few UpdateLayoutTree events");
assertGreaterOrEqual(knownEvents["Layout"], 1, "Too few Layout events");
InspectorTest.log("Event sanity test done");
Expand Down
1 change: 0 additions & 1 deletion third_party/WebKit/LayoutTests/inspector/tracing.html
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@
++knownEvents[event.name];
}
InspectorTest.assertGreaterOrEqual(events.length, 100, "Too few trace events recorded");
InspectorTest.assertGreaterOrEqual(knownEvents["v8.callFunction"], 1, "Too few v8.callFunction");
InspectorTest.assertGreaterOrEqual(knownEvents["UpdateLayoutTree"], 1, "Too few UpdateLayoutTree");
InspectorTest.assertGreaterOrEqual(knownEvents["FrameView::layout"], 1, "Too few FrameView::layout");
InspectorTest.assertGreaterOrEqual(phaseComplete, 50, "Too few begin events");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
Tests extracting information about original functions from bound ones


FunctionCall :1
FunctionCall timeline-bound-function.html:7

Original file line number Diff line number Diff line change
@@ -1,17 +1 @@
Tests the Timeline API function call record for InjectedScript.eval call feature.

FunctionCall Properties:
{
data : {
frame : <string>
scriptId : <string>
scriptLine : <number>
scriptName : <string>
}
endTime : <number>
frameId : <string>
startTime : <number>
type : "FunctionCall"
}
Text details for FunctionCall: undefined

Tests the Timeline API function call is not recorded for InjectedScript.eval.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

<body onload="runTest()">
<p>
Tests the Timeline API function call record for InjectedScript.eval call feature.
Tests the Timeline API function call is not recorded for InjectedScript.eval.
</p>

</body>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@

function performActions()
{
invalidateAndForceLayout(document.getElementById("invalidate1"));
invalidateAndForceLayout(document.getElementById("invalidate2"));
wrapCallFunctionForTimeline(() => invalidateAndForceLayout(document.getElementById("invalidate1")));
wrapCallFunctionForTimeline(() => invalidateAndForceLayout(document.getElementById("invalidate2")));
}

function test()
Expand Down
Loading

0 comments on commit 133a157

Please sign in to comment.