Remove pp::Var dependency from PDFiumPage
This change removes pp::Var dependency from PDFiumPage. All usage of pp::Var are now scoped to OutOfProcessInstance or pdf/ppapi_migration. Move IsValidLink() to PDFEngine::Client. Pepper and Non-pepper implementations differ for IsValidLink() as one requires the string to convert to pp::Var and other to base::Value. Bug: 1113523 Change-Id: Ic2f95bf5fbd4d41e26d9de472148d600747e6d8b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2504751 Reviewed-by: K. Moon <kmoon@chromium.org> Commit-Queue: Ankit Kumar 🌪️ <ankk@microsoft.com> Cr-Commit-Position: refs/heads/master@{#822293}
This commit is contained in:

committed by
Commit Bot

parent
0a5c418eee
commit
02fcb696e5
@ -2199,6 +2199,10 @@ void OutOfProcessInstance::SetLinkUnderCursor(
|
||||
pp::PDF::SetLinkUnderCursor(this, link_under_cursor.c_str());
|
||||
}
|
||||
|
||||
bool OutOfProcessInstance::IsValidLink(const std::string& url) {
|
||||
return pp::Var(url).is_string();
|
||||
}
|
||||
|
||||
void OutOfProcessInstance::ProcessPreviewPageInfo(const std::string& url,
|
||||
int dest_page_index) {
|
||||
DCHECK(IsPrintPreview());
|
||||
|
@ -165,6 +165,7 @@ class OutOfProcessInstance : public PdfViewPluginBase,
|
||||
void DocumentFocusChanged(bool document_has_focus) override;
|
||||
void SetSelectedText(const std::string& selected_text) override;
|
||||
void SetLinkUnderCursor(const std::string& link_under_cursor) override;
|
||||
bool IsValidLink(const std::string& url) override;
|
||||
|
||||
// PreviewModeClient::Client:
|
||||
void PreviewDocumentLoadComplete() override;
|
||||
|
@ -279,6 +279,12 @@ class PDFEngine {
|
||||
|
||||
// Sets the link under cursor.
|
||||
virtual void SetLinkUnderCursor(const std::string& link_under_cursor) = 0;
|
||||
|
||||
// If the link cannot be converted to JS payload struct, then it is not
|
||||
// possible to pass it to JS. In this case, ignore the link like other PDF
|
||||
// viewers.
|
||||
// See https://crbug.com/312882 for an example.
|
||||
virtual bool IsValidLink(const std::string& url) = 0;
|
||||
};
|
||||
|
||||
struct AccessibilityLinkInfo {
|
||||
|
@ -293,6 +293,10 @@ void PdfViewWebPlugin::SetLinkUnderCursor(
|
||||
NOTIMPLEMENTED();
|
||||
}
|
||||
|
||||
bool PdfViewWebPlugin::IsValidLink(const std::string& url) {
|
||||
return base::Value(url).is_string();
|
||||
}
|
||||
|
||||
bool PdfViewWebPlugin::IsValid() const {
|
||||
return container_ && container_->GetDocument().GetFrame();
|
||||
}
|
||||
|
@ -102,6 +102,7 @@ class PdfViewWebPlugin final : public PdfViewPluginBase,
|
||||
void DocumentFocusChanged(bool document_has_focus) override;
|
||||
void SetSelectedText(const std::string& selected_text) override;
|
||||
void SetLinkUnderCursor(const std::string& link_under_cursor) override;
|
||||
bool IsValidLink(const std::string& url) override;
|
||||
|
||||
// BlinkUrlLoader::Client:
|
||||
bool IsValid() const override;
|
||||
|
@ -811,6 +811,10 @@ FPDF_FORMHANDLE PDFiumEngine::form() const {
|
||||
return document_ ? document_->form() : nullptr;
|
||||
}
|
||||
|
||||
bool PDFiumEngine::IsValidLink(const std::string& url) {
|
||||
return client_->IsValidLink(url);
|
||||
}
|
||||
|
||||
void PDFiumEngine::ContinueFind(int32_t result) {
|
||||
StartFind(current_find_text_, result != 0);
|
||||
}
|
||||
|
@ -186,6 +186,8 @@ class PDFiumEngine : public PDFEngine,
|
||||
FPDF_DOCUMENT doc() const;
|
||||
FPDF_FORMHANDLE form() const;
|
||||
|
||||
bool IsValidLink(const std::string& url);
|
||||
|
||||
private:
|
||||
// This helper class is used to detect the difference in selection between
|
||||
// construction and destruction. At destruction, it invalidates all the
|
||||
|
@ -55,17 +55,6 @@ constexpr float k180DegreesInRadians = base::kPiFloat;
|
||||
constexpr float k270DegreesInRadians = 3 * base::kPiFloat / 2;
|
||||
constexpr float k360DegreesInRadians = 2 * base::kPiFloat;
|
||||
|
||||
PDFiumPage::IsValidLinkFunction g_is_valid_link_func_for_testing = nullptr;
|
||||
|
||||
// If the link cannot be converted to a pp::Var, then it is not possible to
|
||||
// pass it to JS. In this case, ignore the link like other PDF viewers.
|
||||
// See https://crbug.com/312882 for an example.
|
||||
// TODO(crbug.com/702993): Get rid of the PPAPI usage here, as well as
|
||||
// SetIsValidLinkFunctionForTesting() and related code.
|
||||
bool IsValidLink(const std::string& url) {
|
||||
return pp::Var(url).is_string();
|
||||
}
|
||||
|
||||
gfx::RectF FloatPageRectToPixelRect(FPDF_PAGE page, const gfx::RectF& input) {
|
||||
int output_width = FPDF_GetPageWidthF(page);
|
||||
int output_height = FPDF_GetPageHeightF(page);
|
||||
@ -283,12 +272,6 @@ PDFiumPage::~PDFiumPage() {
|
||||
DCHECK_EQ(0, preventing_unload_count_);
|
||||
}
|
||||
|
||||
// static
|
||||
void PDFiumPage::SetIsValidLinkFunctionForTesting(
|
||||
IsValidLinkFunction function) {
|
||||
g_is_valid_link_func_for_testing = function;
|
||||
}
|
||||
|
||||
void PDFiumPage::Unload() {
|
||||
// Do not unload while in the middle of a load.
|
||||
if (preventing_unload_count_)
|
||||
@ -985,10 +968,7 @@ void PDFiumPage::PopulateWebLinks() {
|
||||
Link link;
|
||||
link.target.url = base::UTF16ToUTF8(url);
|
||||
|
||||
IsValidLinkFunction is_valid_link_func =
|
||||
g_is_valid_link_func_for_testing ? g_is_valid_link_func_for_testing
|
||||
: &IsValidLink;
|
||||
if (!is_valid_link_func(link.target.url))
|
||||
if (!engine_->IsValidLink(link.target.url))
|
||||
continue;
|
||||
|
||||
// Make sure all the characters in the URL are valid per RFC 1738.
|
||||
|
@ -44,9 +44,6 @@ class PDFiumPage {
|
||||
PDFiumPage(PDFiumPage&& that);
|
||||
~PDFiumPage();
|
||||
|
||||
using IsValidLinkFunction = bool (*)(const std::string& url);
|
||||
static void SetIsValidLinkFunctionForTesting(IsValidLinkFunction function);
|
||||
|
||||
// Unloads the PDFium data for this page from memory.
|
||||
void Unload();
|
||||
// Gets the FPDF_PAGE for this page, loading and parsing it if necessary.
|
||||
|
@ -35,10 +35,6 @@ base::FilePath GetTestFontsDir() {
|
||||
}
|
||||
#endif // defined(OS_LINUX) || defined(OS_CHROMEOS)
|
||||
|
||||
bool IsValidLinkForTesting(const std::string& url) {
|
||||
return !url.empty();
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
PDFiumTestBase::PDFiumTestBase() = default;
|
||||
@ -56,11 +52,9 @@ bool PDFiumTestBase::UsingTestFonts() {
|
||||
|
||||
void PDFiumTestBase::SetUp() {
|
||||
InitializePDFium();
|
||||
PDFiumPage::SetIsValidLinkFunctionForTesting(&IsValidLinkForTesting);
|
||||
}
|
||||
|
||||
void PDFiumTestBase::TearDown() {
|
||||
PDFiumPage::SetIsValidLinkFunctionForTesting(nullptr);
|
||||
FPDF_DestroyLibrary();
|
||||
}
|
||||
|
||||
|
@ -173,4 +173,9 @@ void PreviewModeClient::SetLinkUnderCursor(
|
||||
NOTREACHED();
|
||||
}
|
||||
|
||||
bool PreviewModeClient::IsValidLink(const std::string& url) {
|
||||
NOTREACHED();
|
||||
return false;
|
||||
}
|
||||
|
||||
} // namespace chrome_pdf
|
||||
|
@ -77,6 +77,7 @@ class PreviewModeClient : public PDFEngine::Client {
|
||||
uint32_t GetBackgroundColor() override;
|
||||
void SetSelectedText(const std::string& selected_text) override;
|
||||
void SetLinkUnderCursor(const std::string& link_under_cursor) override;
|
||||
bool IsValidLink(const std::string& url) override;
|
||||
|
||||
private:
|
||||
Client* const client_;
|
||||
|
@ -67,4 +67,8 @@ void TestClient::SetSelectedText(const std::string& selected_text) {}
|
||||
|
||||
void TestClient::SetLinkUnderCursor(const std::string& link_under_cursor) {}
|
||||
|
||||
bool TestClient::IsValidLink(const std::string& url) {
|
||||
return !url.empty();
|
||||
}
|
||||
|
||||
} // namespace chrome_pdf
|
||||
|
@ -40,6 +40,7 @@ class TestClient : public PDFEngine::Client {
|
||||
float GetToolbarHeightInScreenCoords() override;
|
||||
void SetSelectedText(const std::string& selected_text) override;
|
||||
void SetLinkUnderCursor(const std::string& link_under_cursor) override;
|
||||
bool IsValidLink(const std::string& url) override;
|
||||
|
||||
private:
|
||||
// Not owned. Expected to dangle briefly, as the engine usually is destroyed
|
||||
|
Reference in New Issue
Block a user