0

Add backend functionality to set two-up view status.

1. Add a two-up view flag inside DocumentLayout::Options. Modify
   layout_test.js accordingly and add matching unit tests.
2. Add backend support for handling message type "setTwoUpView",
   which will update viewport's layout options and apply the changes.
3. Add unit tests for PDFiumEngine::SetTwoUpView().

Bug: 51472
Change-Id: I212418b439e860feb9e6ed8761a5f722f2a14a9d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1976156
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Hui Yingst <nigi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732517}
This commit is contained in:
Hui Yingst
2020-01-16 19:52:56 +00:00
committed by Commit Bot
parent 35f0fb58c2
commit 28f9f5c917
10 changed files with 181 additions and 40 deletions

@ -10,6 +10,7 @@ const tests = [
chrome.test.assertEq(
{
defaultPageOrientation: 0,
twoUpViewEnabled: false,
},
viewer.viewport.getLayoutOptions());
chrome.test.succeed();

@ -17,6 +17,7 @@ namespace chrome_pdf {
namespace {
constexpr char kDefaultPageOrientation[] = "defaultPageOrientation";
constexpr char kTwoUpViewEnabled[] = "twoUpViewEnabled";
int GetWidestPageWidth(const std::vector<pp::Size>& page_sizes) {
int widest_page_width = 0;
@ -51,6 +52,7 @@ pp::Var DocumentLayout::Options::ToVar() const {
pp::VarDictionary dictionary;
dictionary.Set(kDefaultPageOrientation,
static_cast<int32_t>(default_page_orientation_));
dictionary.Set(kTwoUpViewEnabled, two_up_view_enabled_);
return dictionary;
}
@ -65,6 +67,8 @@ void DocumentLayout::Options::FromVar(const pp::Var& var) {
static_cast<int32_t>(PageOrientation::kLast));
default_page_orientation_ =
static_cast<PageOrientation>(default_page_orientation);
two_up_view_enabled_ = dictionary.Get(kTwoUpViewEnabled).AsBool();
}
void DocumentLayout::Options::RotatePagesClockwise() {
@ -80,14 +84,14 @@ DocumentLayout::DocumentLayout() = default;
DocumentLayout::~DocumentLayout() = default;
void DocumentLayout::SetOptions(const Options& options) {
// TODO(kmoon): This is pessimistic, but we still want to consider the layout
// dirty for orientation changes, even if the page rects don't change.
// To be conservative, we want to consider the layout dirty for any layout
// option changes, even if the page rects don't necessarily change when
// layout options change.
//
// We also probably don't want orientation changes to actually kick in until
// We also probably don't want layout changes to actually kick in until
// the next call to ComputeLayout(). (In practice, we'll call ComputeLayout()
// shortly after calling SetOptions().)
if (options_.default_page_orientation() !=
options.default_page_orientation()) {
if (options_ != options) {
dirty_ = true;
}
options_ = options;

@ -39,6 +39,15 @@ class DocumentLayout final {
~Options();
friend bool operator==(const Options& lhs, const Options& rhs) {
return lhs.two_up_view_enabled() == rhs.two_up_view_enabled() &&
lhs.default_page_orientation() == rhs.default_page_orientation();
}
friend bool operator!=(const Options& lhs, const Options& rhs) {
return !(lhs == rhs);
}
// Serializes layout options to a pp::Var.
pp::Var ToVar() const;
@ -55,8 +64,14 @@ class DocumentLayout final {
// Rotates default page orientation 90 degrees counterclockwise.
void RotatePagesCounterclockwise();
bool two_up_view_enabled() const { return two_up_view_enabled_; }
// Changes two-up view status.
void set_two_up_view_enabled(bool enable) { two_up_view_enabled_ = enable; }
private:
PageOrientation default_page_orientation_ = PageOrientation::kOriginal;
bool two_up_view_enabled_ = false;
};
static const draw_utils::PageInsetSizes kSingleViewInsets;

@ -33,28 +33,81 @@ inline bool PpRectEq(const pp::Rect& lhs, const pp::Rect& rhs) {
TEST_F(DocumentLayoutOptionsTest, DefaultConstructor) {
EXPECT_EQ(options_.default_page_orientation(), PageOrientation::kOriginal);
EXPECT_FALSE(options_.two_up_view_enabled());
}
TEST_F(DocumentLayoutOptionsTest, CopyConstructor) {
options_.RotatePagesClockwise();
options_.set_two_up_view_enabled(true);
DocumentLayout::Options copy(options_);
EXPECT_EQ(copy.default_page_orientation(), PageOrientation::kClockwise90);
EXPECT_TRUE(copy.two_up_view_enabled());
options_.RotatePagesClockwise();
options_.set_two_up_view_enabled(false);
EXPECT_EQ(copy.default_page_orientation(), PageOrientation::kClockwise90);
EXPECT_TRUE(copy.two_up_view_enabled());
}
TEST_F(DocumentLayoutOptionsTest, CopyAssignment) {
options_.RotatePagesClockwise();
options_.set_two_up_view_enabled(true);
DocumentLayout::Options copy;
EXPECT_EQ(copy.default_page_orientation(), PageOrientation::kOriginal);
copy = options_;
DocumentLayout::Options copy = options_;
EXPECT_EQ(copy.default_page_orientation(), PageOrientation::kClockwise90);
EXPECT_TRUE(copy.two_up_view_enabled());
options_.RotatePagesClockwise();
options_.set_two_up_view_enabled(false);
EXPECT_EQ(copy.default_page_orientation(), PageOrientation::kClockwise90);
EXPECT_TRUE(copy.two_up_view_enabled());
}
TEST_F(DocumentLayoutOptionsTest, Equals) {
EXPECT_TRUE(options_ == options_);
DocumentLayout::Options copy;
EXPECT_TRUE(copy == options_);
options_.RotatePagesClockwise();
EXPECT_FALSE(copy == options_);
copy.RotatePagesClockwise();
EXPECT_TRUE(copy == options_);
options_.RotatePagesCounterclockwise();
EXPECT_FALSE(copy == options_);
copy.RotatePagesCounterclockwise();
EXPECT_TRUE(copy == options_);
options_.set_two_up_view_enabled(true);
EXPECT_FALSE(copy == options_);
copy.set_two_up_view_enabled(true);
EXPECT_TRUE(copy == options_);
options_.set_two_up_view_enabled(false);
EXPECT_FALSE(copy == options_);
copy.set_two_up_view_enabled(false);
EXPECT_TRUE(copy == options_);
}
TEST_F(DocumentLayoutOptionsTest, NotEquals) {
// Given that "!=" is defined as "!(==)", minimal tests should be sufficient
// here.
EXPECT_FALSE(options_ != options_);
DocumentLayout::Options copy;
EXPECT_FALSE(copy != options_);
options_.RotatePagesClockwise();
EXPECT_TRUE(copy != options_);
copy.RotatePagesClockwise();
EXPECT_FALSE(copy != options_);
}
TEST_F(DocumentLayoutOptionsTest, RotatePagesClockwise) {
@ -92,6 +145,7 @@ TEST_F(DocumentLayoutOptionsTest, RotatePagesCounterclockwise) {
TEST_F(DocumentLayoutTest, DefaultConstructor) {
EXPECT_EQ(layout_.options().default_page_orientation(),
PageOrientation::kOriginal);
EXPECT_FALSE(layout_.options().two_up_view_enabled());
EXPECT_FALSE(layout_.dirty());
EXPECT_PRED2(PpSizeEq, layout_.size(), pp::Size(0, 0));
EXPECT_EQ(layout_.page_count(), 0u);
@ -109,7 +163,7 @@ TEST_F(DocumentLayoutTest, SetOptionsDoesNotRecomputeLayout) {
EXPECT_PRED2(PpSizeEq, layout_.size(), pp::Size(100, 200));
}
TEST_F(DocumentLayoutTest, DirtySetOnOrientationChange) {
TEST_F(DocumentLayoutTest, DirtySetOnOptionsChange) {
DocumentLayout::Options options;
layout_.SetOptions(options);
EXPECT_FALSE(layout_.dirty());
@ -117,6 +171,25 @@ TEST_F(DocumentLayoutTest, DirtySetOnOrientationChange) {
options.RotatePagesClockwise();
layout_.SetOptions(options);
EXPECT_TRUE(layout_.dirty());
layout_.clear_dirty();
options.set_two_up_view_enabled(true);
layout_.SetOptions(options);
EXPECT_TRUE(layout_.dirty());
}
TEST_F(DocumentLayoutTest, DirtyNotSetOnSameOptions) {
DocumentLayout::Options options;
options.set_two_up_view_enabled(true);
layout_.SetOptions(options);
EXPECT_TRUE(layout_.dirty());
layout_.clear_dirty();
options.set_two_up_view_enabled(true);
layout_.SetOptions(options);
EXPECT_FALSE(layout_.dirty());
}
TEST_F(DocumentLayoutTest, ComputeSingleViewLayout) {

@ -155,6 +155,9 @@ constexpr char kJSEmailBody[] = "body";
// Rotation (Page -> Plugin)
constexpr char kJSRotateClockwiseType[] = "rotateClockwise";
constexpr char kJSRotateCounterclockwiseType[] = "rotateCounterclockwise";
// Toggle two-up view (Page -> Plugin)
constexpr char kJSSetTwoUpViewType[] = "setTwoUpView";
constexpr char kJSEnableTwoUpView[] = "enableTwoUpView";
// Select all text in the document (Page -> Plugin)
constexpr char kJSSelectAllType[] = "selectAll";
// Get the selected text in the document (Page -> Plugin)
@ -716,6 +719,8 @@ void OutOfProcessInstance::HandleMessage(const pp::Var& message) {
RotateClockwise();
} else if (type == kJSRotateCounterclockwiseType) {
RotateCounterclockwise();
} else if (type == kJSSetTwoUpViewType) {
SetTwoUpView(dict.Get(pp::Var(kJSEnableTwoUpView)).AsBool());
} else if (type == kJSSelectAllType) {
engine_->SelectAll();
} else if (type == kJSBackgroundColorChangedType) {
@ -1785,6 +1790,10 @@ void OutOfProcessInstance::RotateCounterclockwise() {
engine_->RotateCounterclockwise();
}
void OutOfProcessInstance::SetTwoUpView(bool enable_two_up_view) {
engine_->SetTwoUpView(enable_two_up_view);
}
std::string OutOfProcessInstance::GetFileNameFromUrl(const std::string& url) {
// Generate a file name. Unfortunately, MIME type can't be provided, since it
// requires IO.

@ -158,6 +158,7 @@ class OutOfProcessInstance : public pp::Instance,
// Helper functions for implementing PPP_PDF.
void RotateClockwise();
void RotateCounterclockwise();
void SetTwoUpView(bool enable_two_up_view);
// Creates a file name for saving a PDF file, given the source URL. Exposed
// for testing.

@ -339,6 +339,7 @@ class PDFEngine {
virtual void ZoomUpdated(double new_zoom_level) = 0;
virtual void RotateClockwise() = 0;
virtual void RotateCounterclockwise() = 0;
virtual void SetTwoUpView(bool enable) = 0;
// Applies the document layout options proposed by a call to
// PDFEngine::Client::ProposeDocumentLayout(), returning the overall size of

@ -500,10 +500,10 @@ void PDFiumEngine::Paint(const pp::Rect& rect,
// Compute the leftover dirty region. The first page may have blank space
// above it, in which case we also need to subtract that space from the
// dirty region.
// If |two_up_view_|, we don't need to recompute |leftover| since
// If two-up view is enabled, we don't need to recompute |leftover| since
// subtracting |leftover| with a two-up view page won't result in a
// rectangle.
if (!two_up_view_) {
if (!layout_.options().two_up_view_enabled()) {
if (i == 0) {
pp::Rect blank_space_in_screen = dirty_in_screen;
blank_space_in_screen.set_y(0);
@ -1925,6 +1925,11 @@ void PDFiumEngine::RotateCounterclockwise() {
ProposeNextDocumentLayout();
}
void PDFiumEngine::SetTwoUpView(bool enable) {
desired_layout_options_.set_two_up_view_enabled(enable);
ProposeNextDocumentLayout();
}
void PDFiumEngine::InvalidateAllPages() {
CancelPaints();
StopFind();
@ -2535,7 +2540,7 @@ void PDFiumEngine::UpdateDocumentLayout(DocumentLayout* layout) {
if (page_sizes.empty())
return;
if (two_up_view_) {
if (layout->options().two_up_view_enabled()) {
layout->ComputeTwoUpViewLayout(page_sizes);
} else {
layout->ComputeSingleViewLayout(page_sizes);
@ -2577,7 +2582,7 @@ std::vector<pp::Size> PDFiumEngine::LoadPageSizes(
// layout options, and handled in the layout code.
pp::Size size = page_available ? GetPageSizeForLayout(i, layout_options)
: default_page_size_;
EnlargePage(i, new_page_count, &size);
EnlargePage(layout_options, i, new_page_count, &size);
page_sizes.push_back(size);
}
@ -2788,11 +2793,12 @@ pp::Size PDFiumEngine::GetPageSizeForLayout(
}
draw_utils::PageInsetSizes PDFiumEngine::GetInsetSizes(
const DocumentLayout::Options& layout_options,
size_t page_index,
size_t num_of_pages) const {
DCHECK_LT(page_index, num_of_pages);
if (two_up_view_) {
if (layout_options.two_up_view_enabled()) {
return draw_utils::GetPageInsetsForTwoUpView(
page_index, num_of_pages, DocumentLayout::kSingleViewInsets,
DocumentLayout::kHorizontalSeparator);
@ -2801,21 +2807,23 @@ draw_utils::PageInsetSizes PDFiumEngine::GetInsetSizes(
return DocumentLayout::kSingleViewInsets;
}
void PDFiumEngine::EnlargePage(size_t page_index,
void PDFiumEngine::EnlargePage(const DocumentLayout::Options& layout_options,
size_t page_index,
size_t num_of_pages,
pp::Size* page_size) const {
draw_utils::PageInsetSizes inset_sizes =
GetInsetSizes(page_index, num_of_pages);
GetInsetSizes(layout_options, page_index, num_of_pages);
page_size->Enlarge(inset_sizes.left + inset_sizes.right,
inset_sizes.top + inset_sizes.bottom);
}
void PDFiumEngine::InsetPage(size_t page_index,
void PDFiumEngine::InsetPage(const DocumentLayout::Options& layout_options,
size_t page_index,
size_t num_of_pages,
double multiplier,
pp::Rect* rect) const {
draw_utils::PageInsetSizes inset_sizes =
GetInsetSizes(page_index, num_of_pages);
GetInsetSizes(layout_options, page_index, num_of_pages);
rect->Inset(static_cast<int>(ceil(inset_sizes.left * multiplier)),
static_cast<int>(ceil(inset_sizes.top * multiplier)),
static_cast<int>(ceil(inset_sizes.right * multiplier)),
@ -2827,7 +2835,7 @@ base::Optional<size_t> PDFiumEngine::GetAdjacentPageIndexForTwoUpView(
size_t num_of_pages) const {
DCHECK_LT(page_index, num_of_pages);
if (!two_up_view_)
if (!layout_.options().two_up_view_enabled())
return base::nullopt;
int adjacent_page_offset = page_index % 2 ? -1 : 1;
@ -2937,10 +2945,11 @@ void PDFiumEngine::FillPageSides(int progressive_index) {
progressive_paints_[progressive_index].rect();
FPDF_BITMAP bitmap = progressive_paints_[progressive_index].bitmap();
draw_utils::PageInsetSizes inset_sizes =
GetInsetSizes(page_index, pages_.size());
GetInsetSizes(layout_.options(), page_index, pages_.size());
pp::Rect page_rect = pages_[page_index]->rect();
if (page_rect.x() > 0 && (!two_up_view_ || page_index % 2 == 0)) {
const bool is_two_up_view = layout_.options().two_up_view_enabled();
if (page_rect.x() > 0 && (!is_two_up_view || page_index % 2 == 0)) {
// If in two-up view, only need to draw the left empty space for left pages
// since the gap between the left and right page will be drawn by the left
// page.
@ -2967,7 +2976,7 @@ void PDFiumEngine::FillPageSides(int progressive_index) {
}
pp::Rect bottom_in_screen;
if (two_up_view_) {
if (is_two_up_view) {
pp::Rect page_in_screen = GetScreenRect(page_rect);
bottom_in_screen = draw_utils::GetBottomGapBetweenRects(
page_in_screen.bottom(), dirty_in_screen);
@ -3001,7 +3010,8 @@ void PDFiumEngine::PaintPageShadow(int progressive_index,
progressive_paints_[progressive_index].rect();
pp::Rect page_rect = pages_[page_index]->rect();
pp::Rect shadow_rect(page_rect);
InsetPage(page_index, pages_.size(), /*multiplier=*/-1, &shadow_rect);
InsetPage(layout_.options(), page_index, pages_.size(), /*multiplier=*/-1,
&shadow_rect);
// Due to the rounding errors of the GetScreenRect it is possible to get
// different size shadows on the left and right sides even they are defined
@ -3009,8 +3019,8 @@ void PDFiumEngine::PaintPageShadow(int progressive_index,
// it by the size of the shadows.
shadow_rect = GetScreenRect(shadow_rect);
page_rect = shadow_rect;
InsetPage(page_index, pages_.size(), /*multiplier=*/current_zoom_,
&page_rect);
InsetPage(layout_.options(), page_index, pages_.size(),
/*multiplier=*/current_zoom_, &page_rect);
DrawPageShadow(page_rect, shadow_rect, dirty_in_screen, image_data);
}
@ -3137,7 +3147,7 @@ pp::Rect PDFiumEngine::GetVisibleRect() const {
pp::Rect PDFiumEngine::GetPageScreenRect(int page_index) const {
const pp::Rect& page_rect = pages_[page_index]->rect();
draw_utils::PageInsetSizes inset_sizes =
GetInsetSizes(page_index, pages_.size());
GetInsetSizes(layout_.options(), page_index, pages_.size());
int max_page_height = page_rect.height();
base::Optional<size_t> adjacent_page_index =

@ -87,6 +87,7 @@ class PDFiumEngine : public PDFEngine,
void ZoomUpdated(double new_zoom_level) override;
void RotateClockwise() override;
void RotateCounterclockwise() override;
void SetTwoUpView(bool enable) override;
pp::Size ApplyDocumentLayout(const DocumentLayout::Options& options) override;
std::string GetSelectedText() override;
bool CanEditText() override;
@ -148,7 +149,6 @@ class PDFiumEngine : public PDFEngine,
void KillFormFocus() override;
uint32_t GetLoadedByteSize() override;
bool ReadLoadedBytes(uint32_t length, void* buffer) override;
#if defined(PDF_ENABLE_XFA)
void UpdatePageCount();
#endif // defined(PDF_ENABLE_XFA)
@ -292,29 +292,33 @@ class PDFiumEngine : public PDFEngine,
const DocumentLayout::Options& layout_options);
// Helper function for getting the inset sizes for the current layout. If
// |two_up_view_| is true, the configuration of inset sizes depends on
// two-up view is enabled, the configuration of inset sizes depends on
// the position of the page, specified by |page_index| and |num_of_pages|.
draw_utils::PageInsetSizes GetInsetSizes(size_t page_index,
size_t num_of_pages) const;
draw_utils::PageInsetSizes GetInsetSizes(
const DocumentLayout::Options& layout_options,
size_t page_index,
size_t num_of_pages) const;
// If |two_up_view_| is false, enlarges |page_size| with inset sizes for
// single-view. If |two_up_view_| is true, calls GetInsetSizes() with
// If two-up view is disabled, enlarges |page_size| with inset sizes for
// single-view. If two-up view is enabled, calls GetInsetSizes() with
// |page_index| and |num_of_pages|, and uses the returned inset sizes to
// enlarge |page_size|.
void EnlargePage(size_t page_index,
void EnlargePage(const DocumentLayout::Options& layout_options,
size_t page_index,
size_t num_of_pages,
pp::Size* page_size) const;
// Similar to EnlargePage(), but insets a |rect|. Also multiplies the inset
// sizes by |multiplier|, using the ceiling of the result.
void InsetPage(size_t page_index,
void InsetPage(const DocumentLayout::Options& layout_options,
size_t page_index,
size_t num_of_pages,
double multiplier,
pp::Rect* rect) const;
// If |two_up_view_| is true, returns the index of the page beside
// If two-up view is enabled, returns the index of the page beside
// |page_index| page. Returns base::nullopt if there is no adjacent page or
// if |two_up_view_| is false.
// if two-up view is disabled.
base::Optional<size_t> GetAdjacentPageIndexForTwoUpView(
size_t page_index,
size_t num_of_pages) const;
@ -603,10 +607,6 @@ class PDFiumEngine : public PDFEngine,
// The indexes of the pages pending download.
std::vector<int> pending_pages_;
// True if loading pages in two-up view layout. False if loading pages in
// single view layout. Has to be in sync with |twoUpView_| in ViewportImpl.
bool two_up_view_ = false;
// During handling of input events we don't want to unload any pages in
// callbacks to us from PDFium, since the current page can change while PDFium
// code still has a pointer to it.

@ -24,6 +24,10 @@ MATCHER_P2(LayoutWithSize, width, height, "") {
return arg.size() == pp::Size(width, height);
}
MATCHER_P(LayoutWithOptions, options, "") {
return arg.options() == options;
}
class MockTestClient : public TestClient {
public:
MockTestClient() {
@ -73,6 +77,29 @@ TEST_F(PDFiumEngineTest, InitializeWithRectanglesMultiPagesPdf) {
ExpectPageRect(engine.get(), 4, {38, 1324, 266, 333});
}
TEST_F(PDFiumEngineTest, InitializeWithRectanglesMultiPagesPdfInTwoUpView) {
NiceMock<MockTestClient> client;
std::unique_ptr<PDFiumEngine> engine = InitializeEngine(
&client, FILE_PATH_LITERAL("rectangles_multi_pages.pdf"));
ASSERT_TRUE(engine);
DocumentLayout::Options options;
options.set_two_up_view_enabled(true);
EXPECT_CALL(client, ProposeDocumentLayout(LayoutWithOptions(options)))
.WillOnce(Return());
engine->SetTwoUpView(true);
engine->ApplyDocumentLayout(options);
ASSERT_EQ(5, engine->GetNumberOfPages());
ExpectPageRect(engine.get(), 0, {72, 3, 266, 333});
ExpectPageRect(engine.get(), 1, {340, 3, 333, 266});
ExpectPageRect(engine.get(), 2, {72, 346, 266, 333});
ExpectPageRect(engine.get(), 3, {340, 346, 266, 333});
ExpectPageRect(engine.get(), 4, {68, 689, 266, 333});
}
TEST_F(PDFiumEngineTest, AppendBlankPagesWithFewerPages) {
NiceMock<MockTestClient> client;
{