0

Move PDFiumFormFiller away from using blink::MainThreadIsolate()

Instead, have PDFiumFormFiller talk to PDFiumEngine, whose client
delegates to PdfViewWebPluginClient. PdfViewWebPluginClient can then
talk to the WebFrame to get the per-frame isolate.

While this removes blink::MainThreadIsolate() usage in production code,
the newly added GetIsolate() method needs to be implemented for unit
tests to pass. This CL adds two new test-only blink::MainThreadIsolate()
calls for now.

Change-Id: I304eda857803616351056dfada21e1d869d0754b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5091351
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1243013}
This commit is contained in:
Lei Zhang
2024-01-04 18:49:03 +00:00
committed by Chromium LUCI CQ
parent 932113c257
commit 90784b92f4
11 changed files with 37 additions and 5 deletions

@ -77,6 +77,10 @@ blink::WebPluginContainer* PdfViewWebPluginClient::PluginContainer() {
return plugin_container_;
}
v8::Isolate* PdfViewWebPluginClient::GetIsolate() {
return GetFrame()->GetAgentGroupScheduler()->Isolate();
}
net::SiteForCookies PdfViewWebPluginClient::SiteForCookies() const {
return plugin_container_->GetDocument().SiteForCookies();
}

@ -41,6 +41,7 @@ class PdfViewWebPluginClient : public chrome_pdf::PdfViewWebPlugin::Client {
base::WeakPtr<chrome_pdf::PdfViewWebPlugin::Client> GetWeakPtr() override;
void SetPluginContainer(blink::WebPluginContainer* container) override;
blink::WebPluginContainer* PluginContainer() override;
v8::Isolate* GetIsolate() override;
net::SiteForCookies SiteForCookies() const override;
blink::WebURL CompleteURL(const blink::WebString& partial_url) const override;
void PostMessage(base::Value::Dict message) override;

@ -25,6 +25,7 @@
#include "ui/gfx/geometry/point_f.h"
#include "ui/gfx/geometry/rect.h"
#include "ui/gfx/geometry/rect_f.h"
#include "v8/include/v8-forward.h"
#if BUILDFLAG(IS_WIN)
#include <windows.h>
@ -229,6 +230,9 @@ class PDFEngine {
// Creates and returns new URL loader for partial document requests.
virtual std::unique_ptr<UrlLoader> CreateUrlLoader() = 0;
// Returns the current V8 isolate, if any.
virtual v8::Isolate* GetIsolate() = 0;
// Searches the given string for "term" and returns the results. Unicode-
// aware.
struct SearchStringResult {

@ -87,7 +87,6 @@
#include "third_party/blink/public/web/web_associated_url_loader.h"
#include "third_party/blink/public/web/web_associated_url_loader_options.h"
#include "third_party/blink/public/web/web_document.h"
#include "third_party/blink/public/web/web_frame.h"
#include "third_party/blink/public/web/web_plugin_container.h"
#include "third_party/blink/public/web/web_plugin_params.h"
#include "third_party/blink/public/web/web_print_params.h"
@ -1071,6 +1070,10 @@ std::unique_ptr<UrlLoader> PdfViewWebPlugin::CreateUrlLoader() {
return std::make_unique<UrlLoader>(weak_factory_.GetWeakPtr());
}
v8::Isolate* PdfViewWebPlugin::GetIsolate() {
return client_->GetIsolate();
}
std::vector<PDFEngine::Client::SearchStringResult>
PdfViewWebPlugin::SearchString(const char16_t* string,
const char16_t* term,

@ -123,7 +123,10 @@ class PdfViewWebPlugin final : public PDFEngine::Client,
// Returns the plugin container set by `SetPluginContainer()`.
virtual blink::WebPluginContainer* PluginContainer() = 0;
// Returrns the document's site for cookies.
// Returns the current V8 isolate, if any.
virtual v8::Isolate* GetIsolate() = 0;
// Returns the document's site for cookies.
virtual net::SiteForCookies SiteForCookies() const = 0;
// Resolves `partial_url` relative to the document's base URL.
@ -331,6 +334,7 @@ class PdfViewWebPlugin final : public PDFEngine::Client,
const void* data,
int length) override;
std::unique_ptr<UrlLoader> CreateUrlLoader() override;
v8::Isolate* GetIsolate() override;
std::vector<SearchStringResult> SearchString(const char16_t* string,
const char16_t* term,
bool case_sensitive) override;

@ -60,6 +60,7 @@
#include "third_party/blink/public/platform/web_url.h"
#include "third_party/blink/public/platform/web_url_request.h"
#include "third_party/blink/public/platform/web_url_response.h"
#include "third_party/blink/public/web/blink.h"
#include "third_party/blink/public/web/web_associated_url_loader.h"
#include "third_party/blink/public/web/web_associated_url_loader_client.h"
#include "third_party/blink/public/web/web_plugin_container.h"
@ -214,6 +215,8 @@ class FakePdfViewWebPluginClient : public PdfViewWebPlugin::Client {
});
return associated_loader;
});
ON_CALL(*this, GetIsolate)
.WillByDefault(Return(blink::MainThreadIsolate()));
ON_CALL(*this, GetEmbedderOriginString)
.WillByDefault(
Return("chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/"));
@ -239,6 +242,8 @@ class FakePdfViewWebPluginClient : public PdfViewWebPlugin::Client {
(override));
MOCK_METHOD(blink::WebPluginContainer*, PluginContainer, (), (override));
MOCK_METHOD(v8::Isolate*, GetIsolate, (), (override));
MOCK_METHOD(net::SiteForCookies, SiteForCookies, (), (const override));
MOCK_METHOD(blink::WebURL,

@ -20,7 +20,6 @@
#include "pdf/pdf_features.h"
#include "pdf/pdfium/pdfium_engine.h"
#include "third_party/blink/public/common/input/web_input_event.h"
#include "third_party/blink/public/web/blink.h"
#include "third_party/pdfium/public/fpdf_annot.h"
#include "ui/base/window_open_disposition_utils.h"
#include "ui/gfx/geometry/rect.h"
@ -748,8 +747,9 @@ PDFiumFormFiller::EngineInIsolateScopeFactory::EngineInIsolateScopeFactory(
PDFiumFormFiller::ScriptOption::kNoJavaScript
? v8::Isolate::TryGetCurrent()
: nullptr) {
if (callback_isolate_)
CHECK_EQ(blink::MainThreadIsolate(), callback_isolate_);
if (callback_isolate_) {
CHECK_EQ(engine_->client_->GetIsolate(), callback_isolate_);
}
}
PDFiumFormFiller::EngineInIsolateScopeFactory::~EngineInIsolateScopeFactory() =

@ -125,6 +125,10 @@ std::unique_ptr<UrlLoader> PreviewModeClient::CreateUrlLoader() {
return nullptr;
}
v8::Isolate* PreviewModeClient::GetIsolate() {
NOTREACHED_NORETURN();
}
std::vector<PDFEngine::Client::SearchStringResult>
PreviewModeClient::SearchString(const char16_t* string,
const char16_t* term,

@ -60,6 +60,7 @@ class PreviewModeClient : public PDFEngine::Client {
const void* data,
int length) override;
std::unique_ptr<UrlLoader> CreateUrlLoader() override;
v8::Isolate* GetIsolate() override;
std::vector<SearchStringResult> SearchString(const char16_t* string,
const char16_t* term,
bool case_sensitive) override;

@ -9,6 +9,7 @@
#include "base/time/time.h"
#include "pdf/document_layout.h"
#include "pdf/loader/url_loader.h"
#include "third_party/blink/public/web/blink.h"
#include "third_party/skia/include/core/SkColor.h"
namespace chrome_pdf {
@ -42,6 +43,10 @@ std::unique_ptr<UrlLoader> TestClient::CreateUrlLoader() {
return nullptr;
}
v8::Isolate* TestClient::GetIsolate() {
return blink::MainThreadIsolate();
}
std::vector<PDFEngine::Client::SearchStringResult> TestClient::SearchString(
const char16_t* string,
const char16_t* term,

@ -32,6 +32,7 @@ class TestClient : public PDFEngine::Client {
const std::string& default_answer) override;
std::string GetURL() override;
std::unique_ptr<UrlLoader> CreateUrlLoader() override;
v8::Isolate* GetIsolate() override;
std::vector<SearchStringResult> SearchString(const char16_t* string,
const char16_t* term,
bool case_sensitive) override;