Allow reproduction of the issue in 1156170 with pdfium_test. Pump the message loop more frequently to expose GC concurency issues. Does not fix the linked bug but rather makes it reproducible. To avoid a number of new #ifdefs at various layers, encapsulate the new logic in a std::function<> callback that can be passed down to called functions. Allows removal of a longstanding TODO() in foreground_task.in as this test now completes as designed. Bug: chromium:1156170 Change-Id: I82d95d71f31f9c427f330dbf08ab62ba94078079 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/76890 Reviewed-by: Hui Yingst <nigi@chromium.org> Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/samples/pdfium_test.cc b/samples/pdfium_test.cc index 91b4f90..26251fa 100644 --- a/samples/pdfium_test.cc +++ b/samples/pdfium_test.cc
@@ -6,6 +6,7 @@ #include <stdlib.h> #include <string.h> +#include <functional> #include <iterator> #include <map> #include <memory> @@ -730,12 +731,13 @@ FPDF_FORMFILLINFO_PDFiumTest* form_fill_info, const int page_index, const Options& options, - const std::string& events) { + const std::string& events, + const std::function<void()>& idler) { FPDF_PAGE page = GetPageForIndex(form_fill_info, doc, page_index); if (!page) return false; if (options.send_events) - SendPageEvents(form, page, events); + SendPageEvents(form, page, events, idler); if (options.save_images) WriteImages(page, name.c_str(), page_index); if (options.save_rendered_images) @@ -795,9 +797,12 @@ } FPDF_FFLDraw(form, bitmap.get(), page, 0, 0, width, height, 0, flags); + idler(); - if (!options.render_oneshot) + if (!options.render_oneshot) { FPDF_RenderPage_Close(page); + idler(); + } int stride = FPDFBitmap_GetStride(bitmap.get()); void* buffer = FPDFBitmap_GetBuffer(bitmap.get()); @@ -861,7 +866,11 @@ } FORM_DoPageAAction(page, form, FPDFPAGE_AACTION_CLOSE); + idler(); + FORM_OnBeforeClosePage(page, form); + idler(); + return !!bitmap; } @@ -869,7 +878,8 @@ const char* buf, size_t len, const Options& options, - const std::string& events) { + const std::string& events, + const std::function<void()>& idler) { TestLoader loader({buf, len}); FPDF_FILEACCESS file_access = {}; @@ -1013,14 +1023,17 @@ } } if (ProcessPage(name, doc.get(), form.get(), &form_callbacks, i, options, - events)) { + events, idler)) { ++processed_pages; } else { ++bad_pages; } + idler(); } FORM_DoDocumentAAction(form.get(), FPDFDOC_AACTION_WC); + idler(); + fprintf(stderr, "Processed %d pages.\n", processed_pages); if (bad_pages) fprintf(stderr, "Skipped %d bad pages.\n", bad_pages); @@ -1163,6 +1176,7 @@ config.m_v8EmbedderSlot = 0; config.m_pPlatform = nullptr; + std::function<void()> idler = []() {}; #ifdef PDF_ENABLE_V8 #ifdef V8_USE_EXTERNAL_STARTUP_DATA v8::StartupData snapshot; @@ -1187,6 +1201,14 @@ FPDF_GetArrayBufferAllocatorSharedInstance()); isolate.reset(v8::Isolate::New(params)); config.m_pIsolate = isolate.get(); + + idler = [&platform, &isolate]() { + int task_count = 0; + while (v8::platform::PumpMessageLoop(platform.get(), isolate.get())) + ++task_count; + if (task_count) + fprintf(stderr, "Pumped %d tasks\n", task_count); + }; } #endif // PDF_ENABLE_V8 @@ -1245,18 +1267,10 @@ } } } - ProcessPdf(filename, file_contents.get(), file_length, options, events); -#ifdef PDF_ENABLE_V8 - if (!options.disable_javascript) { - int task_count = 0; - while (v8::platform::PumpMessageLoop(platform.get(), isolate.get())) - ++task_count; - - if (task_count) - fprintf(stderr, "Pumped %d tasks\n", task_count); - } -#endif // PDF_ENABLE_V8 + ProcessPdf(filename, file_contents.get(), file_length, options, events, + idler); + idler(); #ifdef ENABLE_CALLGRIND if (options.callgrind_delimiters)
diff --git a/samples/pdfium_test_event_helper.cc b/samples/pdfium_test_event_helper.cc index 1fe30c3..292d5a1 100644 --- a/samples/pdfium_test_event_helper.cc +++ b/samples/pdfium_test_event_helper.cc
@@ -156,7 +156,8 @@ void SendPageEvents(FPDF_FORMHANDLE form, FPDF_PAGE page, - const std::string& events) { + const std::string& events, + const std::function<void()>& idler) { auto lines = StringSplit(events, '\n'); for (const auto& line : lines) { auto command = StringSplit(line, '#'); @@ -182,5 +183,6 @@ } else { fprintf(stderr, "Unrecognized event: %s\n", tokens[0].c_str()); } + idler(); } }
diff --git a/samples/pdfium_test_event_helper.h b/samples/pdfium_test_event_helper.h index e823369..f03ddb9 100644 --- a/samples/pdfium_test_event_helper.h +++ b/samples/pdfium_test_event_helper.h
@@ -5,6 +5,7 @@ #ifndef SAMPLES_PDFIUM_TEST_EVENT_HELPER_H_ #define SAMPLES_PDFIUM_TEST_EVENT_HELPER_H_ +#include <functional> #include <string> #include "public/fpdf_formfill.h" @@ -12,6 +13,7 @@ void SendPageEvents(FPDF_FORMHANDLE form, FPDF_PAGE page, - const std::string& events); + const std::string& events, + const std::function<void()>& idler); #endif // SAMPLES_PDFIUM_TEST_EVENT_HELPER_H_
diff --git a/testing/resources/javascript/foreground_task.in b/testing/resources/javascript/foreground_task.in index b96917c..dd5444a 100644 --- a/testing/resources/javascript/foreground_task.in +++ b/testing/resources/javascript/foreground_task.in
@@ -30,8 +30,7 @@ >> stream try { - // TODO(tsepez): figure out why .then() doesn't fire. - gc({type: 'major', execution: 'async'}).then(() => { app.alert('resolved') }); + gc({type: 'major', execution: 'async'}).then(() => { app.alert('Resolved') }); app.alert('Posted'); } catch (e) { app.alert('Error: ' + e);
diff --git a/testing/resources/javascript/foreground_task_expected.txt b/testing/resources/javascript/foreground_task_expected.txt index a561e88..d40e100 100644 --- a/testing/resources/javascript/foreground_task_expected.txt +++ b/testing/resources/javascript/foreground_task_expected.txt
@@ -1 +1,2 @@ Alert: Posted +Alert: Resolved