Skip to content

Commit

Permalink
Revert "Use flag "pdf-use-skia-renderer" to switch PDF viewer's rende…
Browse files Browse the repository at this point in the history
…rer"

This reverts commit ba504e8.

Reason for revert: Pixel tests failing on Mac:
https://ci.chromium.org/ui/p/chromium/builders/ci/mac12-arm64-rel-tests/3703/test-results?q=ExactID%3Aninja%3A%2F%2Fpdf%3Apdf_unittests%2FPDFiumPageThumbnailTest.GenerateThumbnail%2FAll.1+VHash%3Ae1e83050ba88b6e6

Original change's description:
> Use flag "pdf-use-skia-renderer" to switch PDF viewer's renderer
>
> Link flag "pdf-use-skia-renderer" with PDFiumEngine so that it can
> actually switch the renderer type for the PDF viewer upon
> initialization.
>
> This CL also changes PDFiumTestBase and tests derived from it into
> parameterized tests so that they can be tested with Skia renderer
> enabled:
> - For the tests which involve image rendering results comparison, add
>   the Skia expectations for them.
> - Test PDFiumPageImageDataTest.ImageData is currently skipped because
>   it crashes when Skia renderer is in use.
>
> Bug: 1379872
> Change-Id: I90d69e8a34e607dc9a049baf9c46d81569976f16
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4000168
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Reviewed-by: Lei Zhang <thestig@chromium.org>
> Commit-Queue: Nigi <nigi@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1068688}

Bug: 1379872, 1382605
Change-Id: Ic2a83ba9d13a3577e5c8f6177c7e7f8d318e93a8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4014220
Auto-Submit: Alan Cutter <alancutter@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Alan Cutter <alancutter@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1068917}
  • Loading branch information
Alan Cutter authored and Chromium LUCI CQ committed Nov 9, 2022
1 parent 9280ad0 commit c7d9b12
Show file tree
Hide file tree
Showing 33 changed files with 130 additions and 203 deletions.
4 changes: 2 additions & 2 deletions build_overrides/pdfium.gni
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ pdf_bundle_freetype_override = false
# use it as the memory allocator instead of `malloc()`.
pdf_use_partition_alloc_override = use_partition_alloc

# Allow to use Skia backend at run time.
pdf_use_skia_override = true
# Disable use of Skia backend.
pdf_use_skia_override = false

# Disable use of Skia backend, paths only (experimental)
pdf_use_skia_paths_override = false
28 changes: 13 additions & 15 deletions pdf/pdfium/accessibility_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ double GetExpectedCharWidth(bool using_test_fonts, size_t i, double expected) {
// update the GetExpected... functions above. If that becomes too much of a
// burden, consider changing the checks to just make sure the font metrics look
// sane.
TEST_P(AccessibilityTest, GetAccessibilityPage) {
TEST_F(AccessibilityTest, GetAccessibilityPage) {
static constexpr size_t kExpectedTextRunCount = 2;
struct {
uint32_t len;
Expand Down Expand Up @@ -112,7 +112,7 @@ TEST_P(AccessibilityTest, GetAccessibilityPage) {
}
}

TEST_P(AccessibilityTest, GetAccessibilityImageInfo) {
TEST_F(AccessibilityTest, GetAccessibilityImageInfo) {
static const AccessibilityImageInfo kExpectedImageInfo[] = {
{"Image 1", 0, {380, 78, 67, 68}, {}},
{"Image 2", 0, {380, 385, 27, 28}, {}},
Expand Down Expand Up @@ -144,7 +144,7 @@ TEST_P(AccessibilityTest, GetAccessibilityImageInfo) {
}
}

TEST_P(AccessibilityTest, GetUnderlyingTextRangeForRect) {
TEST_F(AccessibilityTest, GetUnderlyingTextRangeForRect) {
TestClient client;
std::unique_ptr<PDFiumEngine> engine =
InitializeEngine(&client, FILE_PATH_LITERAL("hello_world2.pdf"));
Expand Down Expand Up @@ -203,7 +203,7 @@ class ScrollEnabledTestClient : public TestClient {
gfx::Vector2d received_scroll_delta_;
};

TEST_P(AccessibilityTest, ScrollIntoViewActionHandling) {
TEST_F(AccessibilityTest, ScrollIntoViewActionHandling) {
// This test checks that accessibility scroll action is passed
// on to the ScrollEnabledTestClient implementation.
ScrollEnabledTestClient client;
Expand Down Expand Up @@ -286,7 +286,7 @@ TEST_P(AccessibilityTest, ScrollIntoViewActionHandling) {
EXPECT_EQ(gfx::Vector2d(-180, -300), client.GetScrollRequestDelta());
}

TEST_P(AccessibilityTest, ScrollToNearestEdge) {
TEST_F(AccessibilityTest, ScrollToNearestEdge) {
ScrollEnabledTestClient client;
std::unique_ptr<PDFiumEngine> engine = InitializeEngine(
&client, FILE_PATH_LITERAL("rectangles_multi_pages.pdf"));
Expand Down Expand Up @@ -325,7 +325,7 @@ TEST_P(AccessibilityTest, ScrollToNearestEdge) {
EXPECT_EQ(gfx::Vector2d(-199, -199), client.GetScrollRequestDelta());
}

TEST_P(AccessibilityTest, ScrollToGlobalPoint) {
TEST_F(AccessibilityTest, ScrollToGlobalPoint) {
ScrollEnabledTestClient client;
std::unique_ptr<PDFiumEngine> engine = InitializeEngine(
&client, FILE_PATH_LITERAL("rectangles_multi_pages.pdf"));
Expand Down Expand Up @@ -390,7 +390,7 @@ class NavigationEnabledTestClient : public TestClient {
float zoom_ = 0;
};

TEST_P(AccessibilityTest, WebLinkClickActionHandling) {
TEST_F(AccessibilityTest, WebLinkClickActionHandling) {
NavigationEnabledTestClient client;
std::unique_ptr<PDFiumEngine> engine =
InitializeEngine(&client, FILE_PATH_LITERAL("weblinks.pdf"));
Expand All @@ -406,7 +406,7 @@ TEST_P(AccessibilityTest, WebLinkClickActionHandling) {
EXPECT_EQ(WindowOpenDisposition::CURRENT_TAB, client.disposition());
}

TEST_P(AccessibilityTest, InternalLinkClickActionHandling) {
TEST_F(AccessibilityTest, InternalLinkClickActionHandling) {
NavigationEnabledTestClient client;
std::unique_ptr<PDFiumEngine> engine =
InitializeEngine(&client, FILE_PATH_LITERAL("link_annots.pdf"));
Expand All @@ -425,7 +425,7 @@ TEST_P(AccessibilityTest, InternalLinkClickActionHandling) {
EXPECT_TRUE(client.url().empty());
}

TEST_P(AccessibilityTest, GetAccessibilityLinkInfo) {
TEST_F(AccessibilityTest, GetAccessibilityLinkInfo) {
AccessibilityLinkInfo expected_link_info[] = {
{"http://yahoo.com", 0, {75, 191, 110, 16}, {1, 1}},
{"http://bing.com", 1, {131, 121, 138, 20}, {4, 1}},
Expand Down Expand Up @@ -466,7 +466,7 @@ TEST_P(AccessibilityTest, GetAccessibilityLinkInfo) {
}
}

TEST_P(AccessibilityTest, GetAccessibilityHighlightInfo) {
TEST_F(AccessibilityTest, GetAccessibilityHighlightInfo) {
constexpr uint32_t kHighlightDefaultColor = MakeARGB(255, 255, 255, 0);
constexpr uint32_t kHighlightRedColor = MakeARGB(102, 230, 0, 0);
constexpr uint32_t kHighlightNoColor = MakeARGB(0, 0, 0, 0);
Expand Down Expand Up @@ -508,7 +508,7 @@ TEST_P(AccessibilityTest, GetAccessibilityHighlightInfo) {
}
}

TEST_P(AccessibilityTest, GetAccessibilityTextFieldInfo) {
TEST_F(AccessibilityTest, GetAccessibilityTextFieldInfo) {
static const AccessibilityTextFieldInfo kExpectedTextFieldInfo[] = {
{"Text Box", "Text", false, false, false, 0, 5, {138, 230, 135, 41}},
{"ReadOnly", "Elephant", true, false, false, 1, 5, {138, 163, 135, 41}},
Expand Down Expand Up @@ -560,7 +560,7 @@ TEST_P(AccessibilityTest, GetAccessibilityTextFieldInfo) {
}
}

TEST_P(AccessibilityTest, SelectionActionHandling) {
TEST_F(AccessibilityTest, SelectionActionHandling) {
struct Selection {
uint32_t start_page_index;
uint32_t start_char_index;
Expand Down Expand Up @@ -624,7 +624,7 @@ TEST_P(AccessibilityTest, SelectionActionHandling) {

// Tests if PP_PDF_SET_SELECTION updates scroll offsets if the selection is not
// in the current visible rect.
TEST_P(AccessibilityTest, SetSelectionAndScroll) {
TEST_F(AccessibilityTest, SetSelectionAndScroll) {
struct Selection {
uint32_t start_page_index;
uint32_t start_char_index;
Expand Down Expand Up @@ -682,6 +682,4 @@ TEST_P(AccessibilityTest, SetSelectionAndScroll) {
}
}

INSTANTIATE_TEST_SUITE_P(All, AccessibilityTest, testing::Bool());

} // namespace chrome_pdf
22 changes: 10 additions & 12 deletions pdf/pdfium/findtext_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ void ExpectInitialSearchResults(FindTextTestClient& client, int count) {

using FindTextTest = PDFiumTestBase;

TEST_P(FindTextTest, FindText) {
TEST_F(FindTextTest, FindText) {
FindTextTestClient client(/*expected_case_sensitive=*/true);
std::unique_ptr<PDFiumEngine> engine =
InitializeEngine(&client, FILE_PATH_LITERAL("hello_world2.pdf"));
Expand All @@ -95,7 +95,7 @@ TEST_P(FindTextTest, FindText) {
engine->StartFind("o", /*case_sensitive=*/true);
}

TEST_P(FindTextTest, FindHyphenatedText) {
TEST_F(FindTextTest, FindHyphenatedText) {
FindTextTestClient client(/*expected_case_sensitive=*/true);
std::unique_ptr<PDFiumEngine> engine =
InitializeEngine(&client, FILE_PATH_LITERAL("spanner.pdf"));
Expand All @@ -105,7 +105,7 @@ TEST_P(FindTextTest, FindHyphenatedText) {
engine->StartFind("application", /*case_sensitive=*/true);
}

TEST_P(FindTextTest, FindLineBreakText) {
TEST_F(FindTextTest, FindLineBreakText) {
FindTextTestClient client(/*expected_case_sensitive=*/true);
std::unique_ptr<PDFiumEngine> engine =
InitializeEngine(&client, FILE_PATH_LITERAL("spanner.pdf"));
Expand All @@ -115,7 +115,7 @@ TEST_P(FindTextTest, FindLineBreakText) {
engine->StartFind("is the first system", /*case_sensitive=*/true);
}

TEST_P(FindTextTest, FindSimpleQuotationMarkText) {
TEST_F(FindTextTest, FindSimpleQuotationMarkText) {
FindTextTestClient client(/*expected_case_sensitive=*/true);
std::unique_ptr<PDFiumEngine> engine =
InitializeEngine(&client, FILE_PATH_LITERAL("bug_142627.pdf"));
Expand All @@ -125,7 +125,7 @@ TEST_P(FindTextTest, FindSimpleQuotationMarkText) {
engine->StartFind("don't", /*case_sensitive=*/true);
}

TEST_P(FindTextTest, FindFancyQuotationMarkText) {
TEST_F(FindTextTest, FindFancyQuotationMarkText) {
FindTextTestClient client(/*expected_case_sensitive=*/true);
std::unique_ptr<PDFiumEngine> engine =
InitializeEngine(&client, FILE_PATH_LITERAL("bug_142627.pdf"));
Expand All @@ -138,7 +138,7 @@ TEST_P(FindTextTest, FindFancyQuotationMarkText) {
engine->StartFind(base::UTF16ToUTF8(term), /*case_sensitive=*/true);
}

TEST_P(FindTextTest, FindHiddenCroppedText) {
TEST_F(FindTextTest, FindHiddenCroppedText) {
FindTextTestClient client(/*expected_case_sensitive=*/true);
std::unique_ptr<PDFiumEngine> engine =
InitializeEngine(&client, FILE_PATH_LITERAL("hello_world_cropped.pdf"));
Expand All @@ -149,7 +149,7 @@ TEST_P(FindTextTest, FindHiddenCroppedText) {
engine->StartFind("Hello", /*case_sensitive=*/true);
}

TEST_P(FindTextTest, FindVisibleCroppedText) {
TEST_F(FindTextTest, FindVisibleCroppedText) {
FindTextTestClient client(/*expected_case_sensitive=*/true);
std::unique_ptr<PDFiumEngine> engine =
InitializeEngine(&client, FILE_PATH_LITERAL("hello_world_cropped.pdf"));
Expand All @@ -160,7 +160,7 @@ TEST_P(FindTextTest, FindVisibleCroppedText) {
engine->StartFind("world", /*case_sensitive=*/true);
}

TEST_P(FindTextTest, FindVisibleCroppedTextRepeatedly) {
TEST_F(FindTextTest, FindVisibleCroppedTextRepeatedly) {
FindTextTestClient client(/*expected_case_sensitive=*/true);
std::unique_ptr<PDFiumEngine> engine =
InitializeEngine(&client, FILE_PATH_LITERAL("hello_world_cropped.pdf"));
Expand All @@ -174,7 +174,7 @@ TEST_P(FindTextTest, FindVisibleCroppedTextRepeatedly) {
engine->StartFind("world", /*case_sensitive=*/true);
}

TEST_P(FindTextTest, SelectFindResult) {
TEST_F(FindTextTest, SelectFindResult) {
FindTextTestClient client(/*expected_case_sensitive=*/true);
std::unique_ptr<PDFiumEngine> engine =
InitializeEngine(&client, FILE_PATH_LITERAL("hello_world2.pdf"));
Expand All @@ -199,7 +199,7 @@ TEST_P(FindTextTest, SelectFindResult) {
ASSERT_TRUE(engine->SelectFindResult(/*forward=*/false));
}

TEST_P(FindTextTest, SelectFindResultAndSwitchToTwoUpView) {
TEST_F(FindTextTest, SelectFindResultAndSwitchToTwoUpView) {
FindTextTestClient client(/*expected_case_sensitive=*/false);
std::unique_ptr<PDFiumEngine> engine =
InitializeEngine(&client, FILE_PATH_LITERAL("hello_world2.pdf"));
Expand Down Expand Up @@ -240,6 +240,4 @@ TEST_P(FindTextTest, SelectFindResultAndSwitchToTwoUpView) {
ASSERT_TRUE(engine->SelectFindResult(/*forward=*/true));
}

INSTANTIATE_TEST_SUITE_P(All, FindTextTest, testing::Bool());

} // namespace chrome_pdf
7 changes: 1 addition & 6 deletions pdf/pdfium/pdfium_engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@
#include "third_party/pdfium/public/fpdf_fwlevent.h"
#include "third_party/pdfium/public/fpdf_ppo.h"
#include "third_party/pdfium/public/fpdf_searchex.h"
#include "third_party/pdfium/public/fpdfview.h"
#include "third_party/skia/include/core/SkBitmap.h"
#include "ui/base/cursor/mojom/cursor_type.mojom-shared.h"
#include "ui/base/window_open_disposition_utils.h"
Expand Down Expand Up @@ -503,15 +502,11 @@ void ParamsTransformPageToScreen(unsigned long view_fit_type,

void InitializeSDK(bool enable_v8, FontMappingMode font_mapping_mode) {
FPDF_LIBRARY_CONFIG config;
config.version = 4;
config.version = 3;
config.m_pUserFontPaths = nullptr;
config.m_pIsolate = nullptr;
config.m_pPlatform = nullptr;
config.m_v8EmbedderSlot = gin::kEmbedderPDFium;
config.m_RendererType =
base::FeatureList::IsEnabled(features::kPdfUseSkiaRenderer)
? FPDF_RENDERERTYPE_SKIA
: FPDF_RENDERERTYPE_AGG;

#if defined(PDF_ENABLE_V8)
if (enable_v8) {
Expand Down

0 comments on commit c7d9b12

Please sign in to comment.