Skip to content

Commit

Permalink
Convert Callback usage to {Once,Repeating}Callback in V8 values and p…
Browse files Browse the repository at this point in the history
…ost-task

The PostTaskToInProcessRendererAndWait() method can take a OnceCallback
since it runs it once, so we do, and callers are changed use BindOnce().

The V8ValueConverter takes a Callback which it never uses, so remove it
from the parameters instead of converting it to an appropriate type.

A few other simple conversions in content/renderer/worker and service_worker.

R=avi@chromium.org

Bug: 953861
Change-Id: I8e77532b6d5e40b5961b6f5193acde6bc9818389
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1623489
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#662297}
  • Loading branch information
danakj authored and Commit Bot committed May 22, 2019
1 parent 6c8b9d1 commit ab34758
Show file tree
Hide file tree
Showing 21 changed files with 95 additions and 131 deletions.
2 changes: 1 addition & 1 deletion content/browser/tracing/memory_tracing_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class MemoryTracingTest : public ContentBrowserTest {
&MemoryTracingTest::OnGlobalMemoryDumpDone, base::Unretained(this),
base::ThreadTaskRunnerHandle::Get(), closure, request_index);
if (from_renderer_thread) {
PostTaskToInProcessRendererAndWait(base::Bind(
PostTaskToInProcessRendererAndWait(base::BindOnce(
&memory_instrumentation::MemoryInstrumentation::
RequestGlobalDumpAndAppendToTrace,
base::Unretained(
Expand Down
10 changes: 2 additions & 8 deletions content/public/renderer/v8_value_converter.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ class CONTENT_EXPORT V8ValueConverter {
// Extends the default behaviour of V8ValueConverter.
class CONTENT_EXPORT Strategy {
public:
typedef base::Callback<std::unique_ptr<base::Value>(v8::Local<v8::Value>,
v8::Isolate* isolate)>
FromV8ValueCallback;

virtual ~Strategy() {}

// If false is returned, V8ValueConverter proceeds with the default
Expand All @@ -42,17 +38,15 @@ class CONTENT_EXPORT V8ValueConverter {
// the ValueConverter's internal checks for depth and cycles.
virtual bool FromV8Object(v8::Local<v8::Object> value,
std::unique_ptr<base::Value>* out,
v8::Isolate* isolate,
const FromV8ValueCallback& callback);
v8::Isolate* isolate);

// If false is returned, V8ValueConverter proceeds with the default
// behavior.
// Use |callback| to convert any child values, as this will retain
// the ValueConverter's internal checks for depth and cycles.
virtual bool FromV8Array(v8::Local<v8::Array> value,
std::unique_ptr<base::Value>* out,
v8::Isolate* isolate,
const FromV8ValueCallback& callback);
v8::Isolate* isolate);

// If false is returned, V8ValueConverter proceeds with the default
// behavior. v8::Object is passed as ArrayBuffer and ArrayBufferView
Expand Down
15 changes: 8 additions & 7 deletions content/public/test/browser_test_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,11 @@ void DumpStackTraceSignalHandler(int signal) {
}
#endif // defined(OS_POSIX)

void RunTaskOnRendererThread(const base::Closure& task,
const base::Closure& quit_task) {
task.Run();
base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI}, quit_task);
void RunTaskOnRendererThread(base::OnceClosure task,
base::OnceClosure quit_task) {
std::move(task).Run();
base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI},
std::move(quit_task));
}

void TraceStopTracingComplete(const base::Closure& quit,
Expand Down Expand Up @@ -536,7 +537,7 @@ void BrowserTestBase::CreateTestServer(const base::FilePath& test_server_base) {
}

void BrowserTestBase::PostTaskToInProcessRendererAndWait(
const base::Closure& task) {
base::OnceClosure task) {
CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kSingleProcess));

Expand All @@ -546,8 +547,8 @@ void BrowserTestBase::PostTaskToInProcessRendererAndWait(

base::RunLoop run_loop;
renderer_task_runner->PostTask(
FROM_HERE,
base::BindOnce(&RunTaskOnRendererThread, task, run_loop.QuitClosure()));
FROM_HERE, base::BindOnce(&RunTaskOnRendererThread, std::move(task),
run_loop.QuitClosure()));
run_loop.Run();
}

Expand Down
2 changes: 1 addition & 1 deletion content/public/test/browser_test_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ class BrowserTestBase : public testing::Test {
// When the test is running in --single-process mode, runs the given task on
// the in-process renderer thread. A nested run loop is run until it
// returns.
void PostTaskToInProcessRendererAndWait(const base::Closure& task);
void PostTaskToInProcessRendererAndWait(base::OnceClosure task);

// Call this before SetUp() to cause the test to generate pixel output.
void EnablePixelOutput();
Expand Down
7 changes: 3 additions & 4 deletions content/renderer/browser_render_view_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,9 @@ class RenderViewBrowserTest : public ContentBrowserTest {
int* error_code, bool* stale_cache_entry_present) {
bool result = false;

PostTaskToInProcessRendererAndWait(
base::Bind(&RenderViewBrowserTest::GetLatestErrorFromRendererClient0,
renderer_client_, &result, error_code,
stale_cache_entry_present));
PostTaskToInProcessRendererAndWait(base::BindOnce(
&RenderViewBrowserTest::GetLatestErrorFromRendererClient0,
renderer_client_, &result, error_code, stale_cache_entry_present));
return result;
}

Expand Down
44 changes: 22 additions & 22 deletions content/renderer/dom_serializer_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ IN_PROC_BROWSER_TEST_F(MAYBE_DomSerializerTests,
// Load the test file.
NavigateToURL(shell(), file_url);

PostTaskToInProcessRendererAndWait(base::Bind(
PostTaskToInProcessRendererAndWait(base::BindOnce(
&MAYBE_DomSerializerTests::SerializeHTMLDOMWithDocTypeOnRenderer,
base::Unretained(this), file_url));
}
Expand All @@ -657,7 +657,7 @@ IN_PROC_BROWSER_TEST_F(MAYBE_DomSerializerTests,
// Load the test file.
NavigateToURL(shell(), file_url);

PostTaskToInProcessRendererAndWait(base::Bind(
PostTaskToInProcessRendererAndWait(base::BindOnce(
&MAYBE_DomSerializerTests::SerializeHTMLDOMWithoutDocTypeOnRenderer,
base::Unretained(this), file_url));
}
Expand Down Expand Up @@ -686,7 +686,7 @@ IN_PROC_BROWSER_TEST_F(MAYBE_DomSerializerTests,
// Load the test file.
NavigateToURL(shell(), file_url);

PostTaskToInProcessRendererAndWait(base::Bind(
PostTaskToInProcessRendererAndWait(base::BindOnce(
&MAYBE_DomSerializerTests::SerializeXMLDocWithBuiltInEntitiesOnRenderer,
base::Unretained(this), xml_file_url, original_contents));
}
Expand All @@ -711,7 +711,7 @@ IN_PROC_BROWSER_TEST_F(MAYBE_DomSerializerTests,
// Load the test file.
NavigateToURL(shell(), file_url);

PostTaskToInProcessRendererAndWait(base::Bind(
PostTaskToInProcessRendererAndWait(base::BindOnce(
&MAYBE_DomSerializerTests::SerializeHTMLDOMWithAddingMOTWOnRenderer,
base::Unretained(this), file_url, original_contents, false));
}
Expand All @@ -736,7 +736,7 @@ IN_PROC_BROWSER_TEST_F(MAYBE_DomSerializerTests,
// Load the test file.
NavigateToURL(shell(), file_url);

PostTaskToInProcessRendererAndWait(base::BindRepeating(
PostTaskToInProcessRendererAndWait(base::BindOnce(
&MAYBE_DomSerializerTests::SerializeHTMLDOMWithAddingMOTWOnRenderer,
base::Unretained(this), file_url, original_contents, true));
}
Expand All @@ -758,10 +758,10 @@ IN_PROC_BROWSER_TEST_F(
// Load the test file.
NavigateToURL(shell(), file_url);

PostTaskToInProcessRendererAndWait(
base::Bind(&MAYBE_DomSerializerTests::
SerializeHTMLDOMWithNoMetaCharsetInOriginalDocOnRenderer,
base::Unretained(this), file_url));
PostTaskToInProcessRendererAndWait(base::BindOnce(
&MAYBE_DomSerializerTests::
SerializeHTMLDOMWithNoMetaCharsetInOriginalDocOnRenderer,
base::Unretained(this), file_url));
}

// When serializing DOM, if the original document has multiple META charset
Expand All @@ -780,7 +780,7 @@ IN_PROC_BROWSER_TEST_F(
// Load the test file.
NavigateToURL(shell(), file_url);

PostTaskToInProcessRendererAndWait(base::Bind(
PostTaskToInProcessRendererAndWait(base::BindOnce(
&MAYBE_DomSerializerTests::
SerializeHTMLDOMWithMultipleMetaCharsetInOriginalDocOnRenderer,
base::Unretained(this), file_url));
Expand All @@ -794,7 +794,7 @@ IN_PROC_BROWSER_TEST_F(MAYBE_DomSerializerTests,
// from non-file scheme.
NavigateToURL(shell(), GetTestUrl(".", "simple_page.html"));

PostTaskToInProcessRendererAndWait(base::Bind(
PostTaskToInProcessRendererAndWait(base::BindOnce(
&MAYBE_DomSerializerTests::SerializeHTMLDOMWithEntitiesInTextOnRenderer,
base::Unretained(this)));
}
Expand All @@ -810,9 +810,9 @@ IN_PROC_BROWSER_TEST_F(MAYBE_DomSerializerTests,
NavigateToURL(shell(), GetTestUrl(".", "simple_page.html"));

PostTaskToInProcessRendererAndWait(
base::Bind(&MAYBE_DomSerializerTests::
SerializeHTMLDOMWithEntitiesInAttributeValueOnRenderer,
base::Unretained(this)));
base::BindOnce(&MAYBE_DomSerializerTests::
SerializeHTMLDOMWithEntitiesInAttributeValueOnRenderer,
base::Unretained(this)));
}

// Test situation of non-standard HTML entities when serializing HTML DOM.
Expand All @@ -826,9 +826,9 @@ IN_PROC_BROWSER_TEST_F(MAYBE_DomSerializerTests,
NavigateToURL(shell(), file_url);

PostTaskToInProcessRendererAndWait(
base::Bind(&MAYBE_DomSerializerTests::
SerializeHTMLDOMWithNonStandardEntitiesOnRenderer,
base::Unretained(this), file_url));
base::BindOnce(&MAYBE_DomSerializerTests::
SerializeHTMLDOMWithNonStandardEntitiesOnRenderer,
base::Unretained(this), file_url));
}

// Test situation of BASE tag in original document when serializing HTML DOM.
Expand All @@ -851,7 +851,7 @@ IN_PROC_BROWSER_TEST_F(MAYBE_DomSerializerTests, SerializeHTMLDOMWithBaseTag) {
// Load the test file.
NavigateToURL(shell(), file_url);

PostTaskToInProcessRendererAndWait(base::Bind(
PostTaskToInProcessRendererAndWait(base::BindOnce(
&MAYBE_DomSerializerTests::SerializeHTMLDOMWithBaseTagOnRenderer,
base::Unretained(this), file_url, path_dir_url));
}
Expand All @@ -864,7 +864,7 @@ IN_PROC_BROWSER_TEST_F(MAYBE_DomSerializerTests,
// from non-file scheme.
NavigateToURL(shell(), GetTestUrl(".", "simple_page.html"));

PostTaskToInProcessRendererAndWait(base::Bind(
PostTaskToInProcessRendererAndWait(base::BindOnce(
&MAYBE_DomSerializerTests::SerializeHTMLDOMWithEmptyHeadOnRenderer,
base::Unretained(this)));
}
Expand All @@ -877,9 +877,9 @@ IN_PROC_BROWSER_TEST_F(MAYBE_DomSerializerTests,
NavigateToURL(shell(), file_url);

PostTaskToInProcessRendererAndWait(
base::Bind(&MAYBE_DomSerializerTests::
SubResourceForElementsInNonHTMLNamespaceOnRenderer,
base::Unretained(this), file_url));
base::BindOnce(&MAYBE_DomSerializerTests::
SubResourceForElementsInNonHTMLNamespaceOnRenderer,
base::Unretained(this), file_url));
}

} // namespace content
28 changes: 14 additions & 14 deletions content/renderer/fetchers/resource_fetcher_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -309,8 +309,8 @@ IN_PROC_BROWSER_TEST_F(ResourceFetcherTests, ResourceFetcherDownload) {
GURL url(embedded_test_server()->GetURL("/simple_page.html"));

PostTaskToInProcessRendererAndWait(
base::Bind(&ResourceFetcherTests::ResourceFetcherDownloadOnRenderer,
base::Unretained(this), url));
base::BindOnce(&ResourceFetcherTests::ResourceFetcherDownloadOnRenderer,
base::Unretained(this), url));
}

// Test if ResourceFetcher can handle server redirects correctly.
Expand All @@ -324,8 +324,8 @@ IN_PROC_BROWSER_TEST_F(ResourceFetcherTests, ResourceFetcherRedirect) {
embedded_test_server()->GetURL("/server-redirect?" + final_url.spec()));

PostTaskToInProcessRendererAndWait(
base::Bind(&ResourceFetcherTests::ResourceFetcherRedirectOnRenderer,
base::Unretained(this), url, final_url));
base::BindOnce(&ResourceFetcherTests::ResourceFetcherRedirectOnRenderer,
base::Unretained(this), url, final_url));
}

IN_PROC_BROWSER_TEST_F(ResourceFetcherTests, ResourceFetcher404) {
Expand All @@ -337,8 +337,8 @@ IN_PROC_BROWSER_TEST_F(ResourceFetcherTests, ResourceFetcher404) {
GURL url = embedded_test_server()->GetURL("/thisfiledoesntexist.html");

PostTaskToInProcessRendererAndWait(
base::Bind(&ResourceFetcherTests::ResourceFetcher404OnRenderer,
base::Unretained(this), url));
base::BindOnce(&ResourceFetcherTests::ResourceFetcher404OnRenderer,
base::Unretained(this), url));
}

// If this flakes, use http://crbug.com/51622.
Expand All @@ -347,8 +347,8 @@ IN_PROC_BROWSER_TEST_F(ResourceFetcherTests, ResourceFetcherDidFail) {
NavigateToURL(shell(), GURL(url::kAboutBlankURL));

PostTaskToInProcessRendererAndWait(
base::Bind(&ResourceFetcherTests::ResourceFetcherDidFailOnRenderer,
base::Unretained(this)));
base::BindOnce(&ResourceFetcherTests::ResourceFetcherDidFailOnRenderer,
base::Unretained(this)));
}

IN_PROC_BROWSER_TEST_F(ResourceFetcherTests, ResourceFetcherTimeout) {
Expand All @@ -361,8 +361,8 @@ IN_PROC_BROWSER_TEST_F(ResourceFetcherTests, ResourceFetcherTimeout) {
GURL url(embedded_test_server()->GetURL("/slow?1"));

PostTaskToInProcessRendererAndWait(
base::Bind(&ResourceFetcherTests::ResourceFetcherTimeoutOnRenderer,
base::Unretained(this), url));
base::BindOnce(&ResourceFetcherTests::ResourceFetcherTimeoutOnRenderer,
base::Unretained(this), url));
}

IN_PROC_BROWSER_TEST_F(ResourceFetcherTests, ResourceFetcherDeletedInCallback) {
Expand All @@ -374,7 +374,7 @@ IN_PROC_BROWSER_TEST_F(ResourceFetcherTests, ResourceFetcherDeletedInCallback) {
// timeout in 0 sec.
GURL url(embedded_test_server()->GetURL("/slow?1"));

PostTaskToInProcessRendererAndWait(base::Bind(
PostTaskToInProcessRendererAndWait(base::BindOnce(
&ResourceFetcherTests::ResourceFetcherDeletedInCallbackOnRenderer,
base::Unretained(this), url));
}
Expand All @@ -388,7 +388,7 @@ IN_PROC_BROWSER_TEST_F(ResourceFetcherTests, ResourceFetcherPost) {
// Grab a page that echos the POST body.
GURL url(embedded_test_server()->GetURL("/echo"));

PostTaskToInProcessRendererAndWait(base::Bind(
PostTaskToInProcessRendererAndWait(base::BindOnce(
&ResourceFetcherTests::ResourceFetcherPost, base::Unretained(this), url));
}

Expand All @@ -402,8 +402,8 @@ IN_PROC_BROWSER_TEST_F(ResourceFetcherTests, ResourceFetcherSetHeader) {
GURL url(embedded_test_server()->GetURL("/echoheader?header"));

PostTaskToInProcessRendererAndWait(
base::Bind(&ResourceFetcherTests::ResourceFetcherSetHeader,
base::Unretained(this), url));
base::BindOnce(&ResourceFetcherTests::ResourceFetcherSetHeader,
base::Unretained(this), url));
}

} // namespace content
3 changes: 1 addition & 2 deletions content/renderer/java/gin_java_bridge_value_converter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ std::unique_ptr<base::Value> GinJavaBridgeValueConverter::FromV8Value(
bool GinJavaBridgeValueConverter::FromV8Object(
v8::Local<v8::Object> value,
std::unique_ptr<base::Value>* out,
v8::Isolate* isolate,
const FromV8ValueCallback& callback) {
v8::Isolate* isolate) {
GinJavaBridgeObject* unwrapped;
if (!gin::ConvertFromV8(isolate, value, &unwrapped)) {
return false;
Expand Down
3 changes: 1 addition & 2 deletions content/renderer/java/gin_java_bridge_value_converter.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@ class GinJavaBridgeValueConverter : public content::V8ValueConverter::Strategy {
// content::V8ValueConverter::Strategy
bool FromV8Object(v8::Local<v8::Object> value,
std::unique_ptr<base::Value>* out,
v8::Isolate* isolate,
const FromV8ValueCallback& callback) override;
v8::Isolate* isolate) override;
bool FromV8ArrayBuffer(v8::Local<v8::Object> value,
std::unique_ptr<base::Value>* out,
v8::Isolate* isolate) override;
Expand Down
2 changes: 1 addition & 1 deletion content/renderer/render_thread_impl_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ class RenderThreadImplGpuMemoryBufferBrowserTest

void SetUpOnMainThread() override {
NavigateToURL(shell(), GURL(url::kAboutBlankURL));
PostTaskToInProcessRendererAndWait(base::Bind(
PostTaskToInProcessRendererAndWait(base::BindOnce(
&RenderThreadImplGpuMemoryBufferBrowserTest::SetUpOnRenderThread,
base::Unretained(this)));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class RenderThreadImplDiscardableMemoryBrowserTest : public ContentBrowserTest {

void SetUpOnMainThread() override {
NavigateToURL(shell(), GURL(url::kAboutBlankURL));
PostTaskToInProcessRendererAndWait(base::Bind(
PostTaskToInProcessRendererAndWait(base::BindOnce(
&RenderThreadImplDiscardableMemoryBrowserTest::SetUpOnRenderThread,
base::Unretained(this)));
}
Expand Down
2 changes: 1 addition & 1 deletion content/renderer/savable_resources_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class SavableResourcesTest : public ContentBrowserTest {
// Load the test file.
NavigateToURL(shell(), file_url);

PostTaskToInProcessRendererAndWait(base::Bind(
PostTaskToInProcessRendererAndWait(base::BindOnce(
&SavableResourcesTest::CheckResources, base::Unretained(this),
page_file_path, expected_resources_matcher,
expected_subframe_urls_matcher, file_url,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,8 @@ class FakeControllerServiceWorker
base::RunLoop run_loop;
ptr->Read(
std::move(data_pipe.producer_handle),
base::BindOnce([](const base::Closure& quit_closure, int32_t status,
uint64_t size) { quit_closure.Run(); },
base::BindOnce([](base::OnceClosure quit_closure, int32_t status,
uint64_t size) { std::move(quit_closure).Run(); },
run_loop.QuitClosure()));
run_loop.Run();
// Copy the content to |out_string|.
Expand Down
Loading

0 comments on commit ab34758

Please sign in to comment.