Skip to content

Commit

Permalink
Add font map user metrics for Linux.
Browse files Browse the repository at this point in the history
The method MapFont, defined in pdfium_engine.cc, is used by Chrome on
Linux to find a substitute font whenever a pdf file uses a font which is
not embedded (or could not be loaded). When this method is called, the
method UserMetricsRecordAction will now be called to record that this
document has requested a substitute font.

Adding similar functionality for other OS's will take more effort just
because the MapFont definitions are completely inside pdfium core.

BUG=chromium:448584

Review-Url: https://codereview.chromium.org/2398763002
Cr-Commit-Position: refs/heads/master@{#424877}
  • Loading branch information
npm1 authored and Commit bot committed Oct 12, 2016
1 parent c227350 commit d24ca04
Show file tree
Hide file tree
Showing 9 changed files with 79 additions and 0 deletions.
16 changes: 16 additions & 0 deletions pdf/out_of_process_instance.cc
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,9 @@ OutOfProcessInstance::OutOfProcessInstance(PP_Instance instance)
preview_document_load_state_(LOAD_STATE_COMPLETE),
uma_(this),
told_browser_about_unsupported_feature_(false),
#if defined(OS_LINUX)
font_substitution_reported_(false),
#endif
print_preview_page_count_(0),
last_progress_sent_(0),
recently_sent_find_update_(false),
Expand Down Expand Up @@ -1206,6 +1209,10 @@ void OutOfProcessInstance::DocumentLoadComplete(int page_count) {
UserMetricsRecordAction("PDF.LoadSuccess");
uma_.HistogramEnumeration("PDF.DocumentFeature", LOADED_DOCUMENT,
FEATURES_COUNT);
#if defined(OS_LINUX)
if (!font_substitution_reported_)
uma_.HistogramEnumeration("PDF.IsFontSubstituted", 0, 2);
#endif

// Note: If we are in print preview mode the scroll location is retained
// across document loads so we don't want to scroll again and override it.
Expand Down Expand Up @@ -1312,6 +1319,15 @@ void OutOfProcessInstance::DocumentLoadFailed() {
PostMessage(message);
}

void OutOfProcessInstance::FontSubstituted() {
#if defined(OS_LINUX)
if (font_substitution_reported_)
return;
font_substitution_reported_ = true;
uma_.HistogramEnumeration("PDF.IsFontSubstituted", 1, 2);
#endif
}

void OutOfProcessInstance::PreviewDocumentLoadFailed() {
UserMetricsRecordAction("PDF.PreviewDocumentLoadFailure");
if (preview_document_load_state_ != LOAD_STATE_LOADING ||
Expand Down
7 changes: 7 additions & 0 deletions pdf/out_of_process_instance.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ class OutOfProcessInstance : public pp::Instance,
void DocumentPaintOccurred() override;
void DocumentLoadComplete(int page_count) override;
void DocumentLoadFailed() override;
void FontSubstituted() override;
pp::Instance* GetPluginInstance() override;
void DocumentHasUnsupportedFeature(const std::string& feature) override;
void DocumentLoadProgress(uint32_t available, uint32_t doc_size) override;
Expand Down Expand Up @@ -309,6 +310,12 @@ class OutOfProcessInstance : public pp::Instance,
// the stats if a feature shows up many times per document.
std::set<std::string> unsupported_features_reported_;

#if defined(OS_LINUX)
// Keeps track of whether font substitution has been reported, so we avoid
// spamming the stats if a document requested multiple substitutes.
bool font_substitution_reported_;
#endif

// Number of pages in print preview mode, 0 if not in print preview mode.
int print_preview_page_count_;
std::vector<int> print_preview_page_numbers_;
Expand Down
3 changes: 3 additions & 0 deletions pdf/pdf_engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,9 @@ class PDFEngine {
// Notifies the client that the document has failed to load.
virtual void DocumentLoadFailed() = 0;

// Notifies the client that the document has requested substitute fonts.
virtual void FontSubstituted() = 0;

virtual pp::Instance* GetPluginInstance() = 0;

// Notifies that an unsupported feature in the PDF was encountered.
Expand Down
22 changes: 22 additions & 0 deletions pdf/pdfium/pdfium_engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ const int32_t kMaxProgressivePaintTimeMs = 300;
// process.
const int32_t kMaxInitialProgressivePaintTimeMs = 250;

PDFiumEngine* g_engine_for_fontmapper = nullptr;

std::vector<uint32_t> GetPageNumbersFromPrintPageNumberRange(
const PP_PrintPageNumberRange_Dev* page_ranges,
uint32_t page_range_count) {
Expand Down Expand Up @@ -259,6 +261,9 @@ void* MapFont(struct _FPDF_SYSFONTINFO*, int weight, int italic,
return nullptr;
}

if (g_engine_for_fontmapper)
g_engine_for_fontmapper->FontSubstituted();

PP_Resource font_resource = pp::PDF::GetFontFileWithFallback(
pp::InstanceHandle(g_last_instance_id),
&description.pp_font_description(),
Expand Down Expand Up @@ -1171,6 +1176,10 @@ void PDFiumEngine::UnsupportedFeature(int type) {
client_->DocumentHasUnsupportedFeature(feature);
}

void PDFiumEngine::FontSubstituted() {
client_->FontSubstituted();
}

void PDFiumEngine::ContinueFind(int32_t result) {
StartFind(current_find_text_, result != 0);
}
Expand Down Expand Up @@ -1223,6 +1232,7 @@ void PDFiumEngine::PrintBegin() {
pp::Resource PDFiumEngine::PrintPages(
const PP_PrintPageNumberRange_Dev* page_ranges, uint32_t page_range_count,
const PP_PrintSettings_Dev& print_settings) {
ScopedSubstFont scoped_subst_font(this);
if (HasPermission(PDFEngine::PERMISSION_PRINT_HIGH_QUALITY))
return PrintPagesAsPDF(page_ranges, page_range_count, print_settings);
else if (HasPermission(PDFEngine::PERMISSION_PRINT_LOW_QUALITY))
Expand Down Expand Up @@ -1375,6 +1385,7 @@ pp::Buffer_Dev PDFiumEngine::PrintPagesAsRasterPDF(

pp::Buffer_Dev PDFiumEngine::GetFlattenedPrintData(const FPDF_DOCUMENT& doc) {
pp::Buffer_Dev buffer;
ScopedSubstFont scoped_subst_font(this);
int page_count = FPDF_GetPageCount(doc);
for (int i = 0; i < page_count; ++i) {
FPDF_PAGE page = FPDF_LoadPage(doc, i);
Expand Down Expand Up @@ -2453,6 +2464,7 @@ void PDFiumEngine::LoadDocument() {
return;

ScopedUnsupportedFeature scoped_unsupported_feature(this);
ScopedSubstFont scoped_subst_font(this);
bool needs_password = false;
if (TryLoadingDoc(std::string(), &needs_password)) {
ContinueLoadingDocument(std::string());
Expand Down Expand Up @@ -2517,6 +2529,7 @@ void PDFiumEngine::OnGetPasswordComplete(int32_t result,

void PDFiumEngine::ContinueLoadingDocument(const std::string& password) {
ScopedUnsupportedFeature scoped_unsupported_feature(this);
ScopedSubstFont scoped_subst_font(this);

bool needs_password = false;
bool loaded = TryLoadingDoc(password, &needs_password);
Expand Down Expand Up @@ -3733,6 +3746,15 @@ ScopedUnsupportedFeature::~ScopedUnsupportedFeature() {
g_engine_for_unsupported = old_engine_;
}

ScopedSubstFont::ScopedSubstFont(PDFiumEngine* engine)
: old_engine_(g_engine_for_fontmapper) {
g_engine_for_fontmapper = engine;
}

ScopedSubstFont::~ScopedSubstFont() {
g_engine_for_fontmapper = old_engine_;
}

namespace {

base::LazyInstance<PDFiumEngineExports>::Leaky g_pdf_engine_exports =
Expand Down
16 changes: 16 additions & 0 deletions pdf/pdfium/pdfium_engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ class PDFiumEngine : public PDFEngine,
void OnDocumentComplete() override;

void UnsupportedFeature(int type);
void FontSubstituted();

std::string current_find_text() const { return current_find_text_; }

Expand Down Expand Up @@ -742,6 +743,21 @@ class ScopedUnsupportedFeature {

private:
PDFiumEngine* const old_engine_;

DISALLOW_COPY_AND_ASSIGN(ScopedUnsupportedFeature);
};

// Create a local variable of this when calling PDFium functions which can call
// our global callback when a substitute font is mapped.
class ScopedSubstFont {
public:
explicit ScopedSubstFont(PDFiumEngine* engine);
~ScopedSubstFont();

private:
PDFiumEngine* const old_engine_;

DISALLOW_COPY_AND_ASSIGN(ScopedSubstFont);
};

class PDFiumEngineExports : public PDFEngineExports {
Expand Down
2 changes: 2 additions & 0 deletions pdf/pdfium/pdfium_page.cc
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ void PDFiumPage::Unload() {

FPDF_PAGE PDFiumPage::GetPage() {
ScopedUnsupportedFeature scoped_unsupported_feature(engine_);
ScopedSubstFont scoped_subst_font(engine_);
if (!available_)
return nullptr;
if (!page_) {
Expand All @@ -132,6 +133,7 @@ FPDF_PAGE PDFiumPage::GetPage() {

FPDF_PAGE PDFiumPage::GetPrintPage() {
ScopedUnsupportedFeature scoped_unsupported_feature(engine_);
ScopedSubstFont scoped_subst_font(engine_);
if (!available_)
return nullptr;
if (!page_) {
Expand Down
4 changes: 4 additions & 0 deletions pdf/preview_mode_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,10 @@ void PreviewModeClient::DocumentHasUnsupportedFeature(
NOTREACHED();
}

void PreviewModeClient::FontSubstituted() {
NOTREACHED();
}

void PreviewModeClient::DocumentLoadProgress(uint32_t available,
uint32_t doc_size) {}

Expand Down
1 change: 1 addition & 0 deletions pdf/preview_mode_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class PreviewModeClient : public PDFEngine::Client {
void DocumentPaintOccurred() override;
void DocumentLoadComplete(int page_count) override;
void DocumentLoadFailed() override;
void FontSubstituted() override;
pp::Instance* GetPluginInstance() override;
void DocumentHasUnsupportedFeature(const std::string& feature) override;
void DocumentLoadProgress(uint32_t available, uint32_t doc_size) override;
Expand Down
8 changes: 8 additions & 0 deletions tools/metrics/histograms/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -42021,6 +42021,14 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
</summary>
</histogram>

<histogram name="PDF.IsFontSubstituted" enum="Boolean">
<owner>npm@chromium.org</owner>
<summary>
Tracks documents opened in the PDF viewer where substitute fonts need to be
used.
</summary>
</histogram>

<histogram name="Pepper.InterfaceUsed" enum="PepperInterface">
<owner>sehr@chromium.org</owner>
<owner>bradnelson@chromium.org</owner>
Expand Down

0 comments on commit d24ca04

Please sign in to comment.