0

Remove PDF ScheduleTaskOnMainThread() code.

This abstraction layer for PPAPI is no longer useful. Just use //base
APIs directly.

Bug: 1302059
Change-Id: I3342f5654ab2aaeea1778ebfca6196f1dd939c52
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3498884
Reviewed-by: Nigi <nigi@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#977239}
This commit is contained in:
Lei Zhang
2022-03-03 19:07:46 +00:00
committed by Chromium LUCI CQ
parent 8005e84f02
commit 9bf9d3cf1c
16 changed files with 49 additions and 140 deletions

@ -14,6 +14,7 @@
#include "base/callback.h"
#include "base/check.h"
#include "base/location.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/time.h"
#include "pdf/paint_ready_rect.h"
#include "pdf/ppapi_migration/callback.h"
@ -160,11 +161,9 @@ void PaintManager::EnsureCallbackPending() {
if (manual_callback_pending_)
return;
client_->ScheduleTaskOnMainThread(
FROM_HERE,
base::BindOnce(&PaintManager::OnManualCallbackComplete,
weak_factory_.GetWeakPtr()),
/*result=*/0, base::TimeDelta());
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(&PaintManager::OnManualCallbackComplete,
weak_factory_.GetWeakPtr()));
manual_callback_pending_ = true;
}
@ -290,7 +289,7 @@ void PaintManager::OnFlushComplete(int32_t) {
}
}
void PaintManager::OnManualCallbackComplete(int32_t) {
void PaintManager::OnManualCallbackComplete() {
DCHECK(manual_callback_pending_);
manual_callback_pending_ = false;

@ -14,13 +14,8 @@
#include "base/memory/weak_ptr.h"
#include "base/time/time.h"
#include "pdf/paint_aggregator.h"
#include "pdf/ppapi_migration/callback.h"
#include "ui/gfx/geometry/size.h"
namespace base {
class Location;
} // namespace base
namespace gfx {
class Point;
class Rect;
@ -73,17 +68,6 @@ class PaintManager {
std::vector<PaintReadyRect>& ready,
std::vector<gfx::Rect>& pending) = 0;
// Schedules work to be executed on a main thread after a specific delay.
// The `result` parameter will be passed as the argument to the `callback`.
// `result` is needed sometimes to emulate calls of some callbacks, but it's
// not always needed. `delay` should be no longer than `INT32_MAX`
// milliseconds for the Pepper plugin implementation to prevent integer
// overflow.
virtual void ScheduleTaskOnMainThread(const base::Location& from_here,
ResultCallback callback,
int32_t result,
base::TimeDelta delay) = 0;
protected:
// You shouldn't be doing deleting through this interface.
~Client() = default;
@ -164,7 +148,7 @@ class PaintManager {
// Callback for manual scheduling of paints when there is no flush callback
// pending.
void OnManualCallbackComplete(int32_t);
void OnManualCallbackComplete();
// Non-owning pointer. See the constructor.
const raw_ptr<Client> client_;

@ -17,7 +17,6 @@
#include "base/values.h"
#include "build/build_config.h"
#include "pdf/document_layout.h"
#include "pdf/ppapi_migration/callback.h"
#include "printing/mojom/print.mojom-forward.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "third_party/skia/include/core/SkColor.h"
@ -33,10 +32,6 @@
class SkBitmap;
namespace base {
class Location;
} // namespace base
namespace blink {
class WebInputEvent;
struct WebPrintParams;
@ -289,17 +284,6 @@ class PDFEngine {
// viewers.
// See https://crbug.com/312882 for an example.
virtual bool IsValidLink(const std::string& url) = 0;
// Schedules work to be executed on a main thread after a specific delay.
// The `result` parameter will be passed as the argument to the `callback`.
// `result` is needed sometimes to emulate calls of some callbacks, but it's
// not always needed. `delay` should be no longer than `INT32_MAX`
// milliseconds for the Pepper plugin implementation to prevent integer
// overflow.
virtual void ScheduleTaskOnMainThread(const base::Location& from_here,
ResultCallback callback,
int32_t result,
base::TimeDelta delay) = 0;
};
virtual ~PDFEngine() = default;

@ -35,6 +35,7 @@
#include "base/strings/string_piece.h"
#include "base/strings/string_split.h"
#include "base/strings/string_util.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/time.h"
#include "base/values.h"
#include "net/base/escape.h"
@ -324,11 +325,11 @@ void PdfViewPluginBase::NotifyNumberOfFindResultsChanged(int total,
return;
recently_sent_find_update_ = true;
ScheduleTaskOnMainThread(
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
base::BindOnce(&PdfViewPluginBase::ResetRecentlySentFindUpdate,
GetWeakPtr()),
/*result=*/0, kFindResultCooldown);
kFindResultCooldown);
}
void PdfViewPluginBase::NotifyTouchSelectionOccurred() {
@ -816,11 +817,9 @@ void PdfViewPluginBase::InvalidateAfterPaintDone() {
if (deferred_invalidates_.empty())
return;
ScheduleTaskOnMainThread(
FROM_HERE,
base::BindOnce(&PdfViewPluginBase::ClearDeferredInvalidates,
GetWeakPtr()),
/*result=*/0, base::TimeDelta());
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(&PdfViewPluginBase::ClearDeferredInvalidates,
GetWeakPtr()));
}
void PdfViewPluginBase::OnGeometryChanged(double old_zoom,
@ -1050,11 +1049,11 @@ void PdfViewPluginBase::PrepareAndSetAccessibilityPageInfo(int32_t page_index) {
std::move(chars), std::move(page_objects));
// Schedule loading the next page.
ScheduleTaskOnMainThread(
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
base::BindOnce(&PdfViewPluginBase::PrepareAndSetAccessibilityPageInfo,
GetWeakPtr()),
/*result=*/page_index + 1, kAccessibilityPageDelay);
GetWeakPtr(), page_index + 1),
kAccessibilityPageDelay);
}
void PdfViewPluginBase::PrepareAndSetAccessibilityViewportInfo() {
@ -1539,8 +1538,7 @@ void PdfViewPluginBase::PrepareForFirstPaint(
PaintReadyRect(rect, GetPluginImageData(), /*flush_now=*/true));
}
void PdfViewPluginBase::ClearDeferredInvalidates(
int32_t /*unused_but_required*/) {
void PdfViewPluginBase::ClearDeferredInvalidates() {
DCHECK(!in_paint_);
for (const gfx::Rect& rect : deferred_invalidates_)
Invalidate(rect);
@ -1666,15 +1664,14 @@ void PdfViewPluginBase::LoadAccessibility() {
PrepareAndSetAccessibilityViewportInfo();
// Schedule loading the first page.
ScheduleTaskOnMainThread(
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
base::BindOnce(&PdfViewPluginBase::PrepareAndSetAccessibilityPageInfo,
GetWeakPtr()),
/*result=*/0, kAccessibilityPageDelay);
GetWeakPtr(), /*page_index=*/0),
kAccessibilityPageDelay);
}
void PdfViewPluginBase::ResetRecentlySentFindUpdate(
int32_t /*unused_but_required*/) {
void PdfViewPluginBase::ResetRecentlySentFindUpdate() {
recently_sent_find_update_ = false;
}

@ -62,8 +62,6 @@ class PdfViewPluginBase : public PDFEngine::Client,
public PaintManager::Client,
public PreviewModeClient::Client {
public:
using PDFEngine::Client::ScheduleTaskOnMainThread;
// Do not save files with over 100 MB. This cap should be kept in sync with
// and is also enforced in chrome/browser/resources/pdf/pdf_viewer.js.
static constexpr size_t kMaximumSavedFileSize = 100 * 1000 * 1000;
@ -480,7 +478,7 @@ class PdfViewPluginBase : public PDFEngine::Client,
void PrepareForFirstPaint(std::vector<PaintReadyRect>& ready);
// Callback to clear deferred invalidates after painting finishes.
void ClearDeferredInvalidates(int32_t /*unused_but_required*/);
void ClearDeferredInvalidates();
// Sends the attachments data.
void SendAttachments();
@ -497,7 +495,7 @@ class PdfViewPluginBase : public PDFEngine::Client,
// Starts loading accessibility information.
void LoadAccessibility();
void ResetRecentlySentFindUpdate(int32_t /*unused_but_required*/);
void ResetRecentlySentFindUpdate();
// Records metrics about the attachment types.
void RecordAttachmentTypes();

@ -201,11 +201,6 @@ class FakePdfViewPluginBase : public PdfViewPluginBase {
MOCK_METHOD(bool, BindPaintGraphics, (Graphics&), (override));
MOCK_METHOD(void,
ScheduleTaskOnMainThread,
(const base::Location&, ResultCallback, int32_t, base::TimeDelta),
(override));
MOCK_METHOD(std::unique_ptr<PDFiumEngine>,
CreateEngine,
(PDFEngine::Client*, PDFiumFormFiller::ScriptOption),

@ -775,14 +775,6 @@ bool PdfViewWebPlugin::BindPaintGraphics(Graphics& graphics) {
return false;
}
void PdfViewWebPlugin::ScheduleTaskOnMainThread(const base::Location& from_here,
ResultCallback callback,
int32_t result,
base::TimeDelta delay) {
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
from_here, base::BindOnce(std::move(callback), result), delay);
}
void PdfViewWebPlugin::SetCaretPosition(const gfx::PointF& position) {
PdfViewPluginBase::SetCaretPosition(position);
}
@ -981,11 +973,9 @@ void PdfViewWebPlugin::PluginDidStopLoading() {
}
void PdfViewWebPlugin::InvokePrintDialog() {
ScheduleTaskOnMainThread(
FROM_HERE,
base::BindOnce(&PdfViewWebPlugin::OnInvokePrintDialog,
weak_factory_.GetWeakPtr()),
/*result=*/0, base::TimeDelta());
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(&PdfViewWebPlugin::OnInvokePrintDialog,
weak_factory_.GetWeakPtr()));
}
void PdfViewWebPlugin::NotifySelectionChanged(const gfx::PointF& left,
@ -1096,7 +1086,7 @@ void PdfViewWebPlugin::HandleImeCommit(const blink::WebString& text) {
}
}
void PdfViewWebPlugin::OnInvokePrintDialog(int32_t /*result*/) {
void PdfViewWebPlugin::OnInvokePrintDialog() {
client_->Print(Container()->GetElement());
}

@ -250,10 +250,6 @@ class PdfViewWebPlugin final : public PdfViewPluginBase,
bool IsValidLink(const std::string& url) override;
std::unique_ptr<Graphics> CreatePaintGraphics(const gfx::Size& size) override;
bool BindPaintGraphics(Graphics& graphics) override;
void ScheduleTaskOnMainThread(const base::Location& from_here,
ResultCallback callback,
int32_t result,
base::TimeDelta delay) override;
// pdf::mojom::PdfListener:
void SetCaretPosition(const gfx::PointF& position) override;
@ -359,7 +355,7 @@ class PdfViewWebPlugin final : public PdfViewPluginBase,
// see crbug.com/66334.
// TODO(crbug.com/1217012): Re-evaluate the need for a callback when parts of
// the plugin are moved off the main thread.
void OnInvokePrintDialog(int32_t /*result*/);
void OnInvokePrintDialog();
// Callback to set the document information in the accessibility tree
// asynchronously.

@ -346,11 +346,8 @@ class PdfViewWebPluginTest : public PdfViewWebPluginWithoutInitializeTest {
// Waits for main thread callback scheduled by `PaintManager`.
base::RunLoop run_loop;
plugin_->ScheduleTaskOnMainThread(
FROM_HERE, base::BindLambdaForTesting([&run_loop](int32_t /*result*/) {
run_loop.Quit();
}),
/*result=*/0, base::TimeDelta());
base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
run_loop.QuitClosure());
run_loop.Run();
}

@ -592,11 +592,9 @@ void PDFiumEngine::PluginSizeUpdated(const gfx::Size& size) {
// asynchronously to avoid observable differences between this path and the
// normal loading path.
document_pending_ = false;
client_->ScheduleTaskOnMainThread(
FROM_HERE,
base::BindOnce(&PDFiumEngine::FinishLoadingDocument,
weak_factory_.GetWeakPtr()),
/*result=*/0, base::TimeDelta());
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(&PDFiumEngine::FinishLoadingDocument,
weak_factory_.GetWeakPtr()));
}
}
@ -823,7 +821,7 @@ void PDFiumEngine::OnNewDataReceived() {
void PDFiumEngine::OnDocumentComplete() {
if (doc())
return FinishLoadingDocument(0);
return FinishLoadingDocument();
document_->file_access().m_FileLen = doc_loader_->GetDocumentSize();
if (!fpdf_availability()) {
@ -840,7 +838,7 @@ void PDFiumEngine::OnDocumentCanceled() {
OnDocumentComplete();
}
void PDFiumEngine::FinishLoadingDocument(int32_t /*unused_but_required*/) {
void PDFiumEngine::FinishLoadingDocument() {
// Note that doc_loader_->IsDocumentComplete() may not be true here if
// called via `OnDocumentCanceled()`.
DCHECK(doc());
@ -1819,11 +1817,10 @@ void PDFiumEngine::StartFind(const std::string& text, bool case_sensitive) {
if (doc_loader_set_for_testing_) {
ContinueFind(case_sensitive ? 1 : 0);
} else {
client_->ScheduleTaskOnMainThread(
FROM_HERE,
base::BindOnce(&PDFiumEngine::ContinueFind,
find_weak_factory_.GetWeakPtr()),
case_sensitive ? 1 : 0, base::TimeDelta());
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(&PDFiumEngine::ContinueFind,
find_weak_factory_.GetWeakPtr(),
case_sensitive ? 1 : 0));
}
}
@ -2793,7 +2790,7 @@ void PDFiumEngine::ContinueLoadingDocument(const std::string& password) {
LoadBody();
if (doc_loader_->IsDocumentComplete())
FinishLoadingDocument(0);
FinishLoadingDocument();
}
void PDFiumEngine::LoadPageInfo() {

@ -294,7 +294,7 @@ class PDFiumEngine : public PDFEngine,
// This should only be called after `doc_` has been loaded and the document is
// fully downloaded.
// If this has been run once, it will not notify the client again.
void FinishLoadingDocument(int32_t /*unused_but_required*/);
void FinishLoadingDocument();
// Loads information about the pages in the document and performs layout.
void LoadPageInfo();

@ -10,11 +10,13 @@
#include "base/callback.h"
#include "base/hash/md5.h"
#include "base/run_loop.h"
#include "base/strings/utf_string_conversions.h"
#include "base/test/gmock_move_support.h"
#include "base/test/gtest_util.h"
#include "base/test/mock_callback.h"
#include "base/test/scoped_feature_list.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/time.h"
#include "build/build_config.h"
#include "pdf/document_attachment_info.h"
@ -73,13 +75,6 @@ class MockTestClient : public TestClient {
(const DocumentLayout& layout),
(override));
MOCK_METHOD(void, ScrollToPage, (int page), (override));
MOCK_METHOD(void,
ScheduleTaskOnMainThread,
(const base::Location& from_here,
ResultCallback callback,
int32_t result,
base::TimeDelta delay),
(override));
MOCK_METHOD(void, DocumentFocusChanged, (bool), (override));
MOCK_METHOD(void, SetLinkUnderCursor, (const std::string&), (override));
};
@ -134,15 +129,13 @@ class PDFiumEngineTest : public PDFiumTestBase {
return loaded_incrementally;
}
void FinishWithPluginSizeUpdated(MockTestClient& client,
PDFiumEngine& engine) {
ResultCallback callback;
EXPECT_CALL(client, ScheduleTaskOnMainThread)
.WillOnce(MoveArg<1>(&callback));
void FinishWithPluginSizeUpdated(PDFiumEngine& engine) {
engine.PluginSizeUpdated({});
ASSERT_TRUE(callback);
std::move(callback).Run(0);
base::RunLoop run_loop;
base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
run_loop.QuitClosure());
run_loop.Run();
}
// Counts the number of available pages. Returns `int` instead of `size_t` for
@ -279,7 +272,7 @@ TEST_F(PDFiumEngineTest, ApplyDocumentLayoutBeforePluginSizeUpdated) {
EXPECT_EQ(gfx::Size(343, 1664), engine.ApplyDocumentLayout(options));
EXPECT_CALL(client, ScrollToPage(-1)).Times(1);
ASSERT_NO_FATAL_FAILURE(FinishWithPluginSizeUpdated(client, engine));
FinishWithPluginSizeUpdated(engine);
}
TEST_F(PDFiumEngineTest, ApplyDocumentLayoutAvoidsInfiniteLoop) {
@ -511,7 +504,7 @@ TEST_F(PDFiumEngineTest, PluginSizeUpdatedAfterLoad) {
PDFiumEngine& engine = *initialize_result.engine;
initialize_result.FinishLoading();
ASSERT_NO_FATAL_FAILURE(FinishWithPluginSizeUpdated(client, engine));
FinishWithPluginSizeUpdated(engine);
EXPECT_EQ(engine.GetNumberOfPages(), CountAvailablePages(engine));
}

@ -173,12 +173,4 @@ bool PreviewModeClient::IsValidLink(const std::string& url) {
return false;
}
void PreviewModeClient::ScheduleTaskOnMainThread(
const base::Location& from_here,
ResultCallback callback,
int32_t result,
base::TimeDelta delay) {
NOTREACHED();
}
} // namespace chrome_pdf

@ -71,10 +71,6 @@ class PreviewModeClient : public PDFEngine::Client {
void SetSelectedText(const std::string& selected_text) override;
void SetLinkUnderCursor(const std::string& link_under_cursor) override;
bool IsValidLink(const std::string& url) override;
void ScheduleTaskOnMainThread(const base::Location& from_here,
ResultCallback callback,
int32_t result,
base::TimeDelta delay) override;
private:
const raw_ptr<Client> client_;

@ -65,9 +65,4 @@ bool TestClient::IsValidLink(const std::string& url) {
return !url.empty();
}
void TestClient::ScheduleTaskOnMainThread(const base::Location& from_here,
ResultCallback callback,
int32_t result,
base::TimeDelta delay) {}
} // namespace chrome_pdf

@ -40,10 +40,6 @@ class TestClient : public PDFEngine::Client {
void SetSelectedText(const std::string& selected_text) override;
void SetLinkUnderCursor(const std::string& link_under_cursor) override;
bool IsValidLink(const std::string& url) override;
void ScheduleTaskOnMainThread(const base::Location& from_here,
ResultCallback callback,
int32_t result,
base::TimeDelta delay) override;
private:
// Not owned. Expected to dangle briefly, as the engine usually is destroyed