[PDF Ink Signatures] Fix existing Ink paths not updating
V2 Ink paths are loaded in PDFiumEngine when the user first enters annotation mode in the PDF viewer. However, if the user scrolls away, the page may unload and reload, and the references to the V2 Ink paths may no longer match the PDF object. Any updates would no longer be visible on these Ink paths. Fix existing Ink paths not erasing nor undoing/redoing by adding a ScopedUnloadPreventer to every page with a loaded V2 Ink path to prevent them from unloading. Fixed: 402364794, 402454523 Change-Id: I777f9a5eaa760dccb0a6e7d4c4da450d20b0a6df Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6348662 Commit-Queue: Andy Phan <andyphan@chromium.org> Reviewed-by: Alan Screen <awscreen@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Cr-Commit-Position: refs/heads/main@{#1431783}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
123a3ba2d2
commit
f8b346847b
@ -4656,14 +4656,28 @@ PDFiumEngine::LoadV2InkPathsForPage(int page_index) {
|
||||
|
||||
std::map<InkModeledShapeId, ink::PartitionedMesh> page_shape_map;
|
||||
|
||||
PDFiumPage* page = pages_[page_index].get();
|
||||
std::vector<ReadV2InkPathResult> read_results =
|
||||
ReadV2InkPathsFromPageAsModeledShapes(pages_[page_index]->GetPage());
|
||||
ReadV2InkPathsFromPageAsModeledShapes(page->GetPage());
|
||||
for (auto& read_result : read_results) {
|
||||
InkModeledShapeId id(next_ink_modeled_shape_id_++);
|
||||
page_shape_map[id] = std::move(read_result.shape);
|
||||
ink_modeled_shape_map_[id] = read_result.page_object;
|
||||
}
|
||||
|
||||
// Should be unique due to the caller's responsibility to call
|
||||
// `LoadV2InkPathsForPage()` at most once per page.
|
||||
CHECK(!stroked_pages_unload_preventers_.contains(page_index));
|
||||
|
||||
// Prevent pages with existing Ink paths from unloading. Otherwise, if the
|
||||
// page unloads and reloads, then the loaded V2 Ink path will no longer match
|
||||
// the PDF object, and any updates to the Ink path will not be visible in the
|
||||
// PDF.
|
||||
if (!page_shape_map.empty()) {
|
||||
stroked_pages_unload_preventers_.insert(
|
||||
{page_index, PDFiumPage::ScopedUnloadPreventer(page)});
|
||||
}
|
||||
|
||||
return page_shape_map;
|
||||
}
|
||||
|
||||
|
@ -2175,8 +2175,9 @@ TEST_P(PDFiumEngineInkTest, LoadV2InkPathsForPage) {
|
||||
ASSERT_EQ(1, engine->GetNumberOfPages());
|
||||
EXPECT_TRUE(engine->ink_modeled_shape_map_for_testing().empty());
|
||||
|
||||
constexpr int kPageIndex = 0;
|
||||
std::map<InkModeledShapeId, ink::PartitionedMesh> ink_shapes =
|
||||
engine->LoadV2InkPathsForPage(/*page_index=*/0);
|
||||
engine->LoadV2InkPathsForPage(kPageIndex);
|
||||
ASSERT_EQ(1u, ink_shapes.size());
|
||||
const auto ink_shapes_it = ink_shapes.begin();
|
||||
|
||||
@ -2188,6 +2189,9 @@ TEST_P(PDFiumEngineInkTest, LoadV2InkPathsForPage) {
|
||||
EXPECT_EQ(ink_shapes_it->first, pdf_shapes_it->first);
|
||||
EXPECT_EQ(1u, ink_shapes_it->second.Meshes().size());
|
||||
EXPECT_TRUE(pdf_shapes_it->second);
|
||||
|
||||
EXPECT_TRUE(engine->stroked_pages_unload_preventers_for_testing().contains(
|
||||
kPageIndex));
|
||||
}
|
||||
|
||||
INSTANTIATE_TEST_SUITE_P(All, PDFiumEngineInkTest, testing::Bool());
|
||||
@ -2418,6 +2422,12 @@ TEST_P(PDFiumEngineInkDrawTest, LoadedV2InkPathsAndUpdateShapeActive) {
|
||||
kInkAnnotationIdentifierKeyV2),
|
||||
1);
|
||||
|
||||
// Attempt to unload the page before erasing. This would have caught
|
||||
// https://crbug.com/402364794.
|
||||
EXPECT_TRUE(engine->stroked_pages_unload_preventers_for_testing().contains(
|
||||
kPageIndex));
|
||||
page.Unload();
|
||||
|
||||
// Erase the shape and check the rendering. Also check the save version.
|
||||
const auto ink_shapes_it = ink_shapes.begin();
|
||||
const InkModeledShapeId& shape_id = ink_shapes_it->first;
|
||||
@ -2432,6 +2442,12 @@ TEST_P(PDFiumEngineInkDrawTest, LoadedV2InkPathsAndUpdateShapeActive) {
|
||||
kInkAnnotationIdentifierKeyV2),
|
||||
0);
|
||||
|
||||
// Attempt to unload the page before undoing. This would have caught
|
||||
// https://crbug.com/402454523.
|
||||
EXPECT_TRUE(engine->stroked_pages_unload_preventers_for_testing().contains(
|
||||
kPageIndex));
|
||||
page.Unload();
|
||||
|
||||
// Undo the erasure and check the rendering.
|
||||
engine->UpdateShapeActive(kPageIndex, shape_id, /*active=*/true);
|
||||
CheckPdfRendering(page.GetPage(), kPageSizeInPoints, kInkV2PngPath);
|
||||
|
Reference in New Issue
Block a user