Skip to content

Commit

Permalink
Combine PDF plugin into the Chromium binary.
Browse files Browse the repository at this point in the history
On Windows, this moves it to chrome_child.dll. Overall binary sizes is 4.5 MB smaller (chrome_child.dll gets 3.6 MB larger while we drop the 8.1 MB pdf.dll). On Mac, the binary is 6.6 MB smaller. On Linux, it's 7MB smaller. This is from official release builds, after stripping on Linux.

The size savings are because we don't ship duplicate versions of V8, and also the PDF plugin uses some of base and net.

This depends on OOP PDF, since otherwise the V8 isolates for the plugin and Blink interact badly. That got turned on a few weeks ago.

BUG=453844

Review URL: https://codereview.chromium.org/799643004

Cr-Commit-Position: refs/heads/master@{#314575}
  • Loading branch information
jam authored and Commit bot committed Feb 4, 2015
1 parent d771a78 commit dc0e439
Show file tree
Hide file tree
Showing 32 changed files with 263 additions and 819 deletions.
1 change: 0 additions & 1 deletion build/all.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -750,7 +750,6 @@
'dependencies': [
'../chrome/chrome_syzygy.gyp:chrome_dll_syzygy',
'../content/content_shell_and_tests.gyp:content_shell_syzyasan',
'../pdf/pdf.gyp:pdf_syzyasan',
],
'conditions': [
['chrome_multiple_dll==1', {
Expand Down
10 changes: 6 additions & 4 deletions chrome/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,8 @@ if (!is_android && (!is_win || link_chrome_on_windows)) {
# TODO(GYP) some stuff from GYP including chrome_multiple_dll.
}

if (!is_mac) {
# On Mac this is done in chrome_dll.gypi.
datadeps += [ "//pdf" ]
# TODO(GYP) pdf linux symbols
if (enable_plugins) {
deps += [ "//pdf" ]
}
}
} # !is_android
Expand Down Expand Up @@ -241,6 +239,10 @@ if (!is_win || link_chrome_on_windows) {
#}],
# TODO(GYP) Lots of other stuff in the OS=="mac" block.
}

if (enable_plugins) {
deps += [ "//pdf" ]
}
}
}

Expand Down
1 change: 1 addition & 0 deletions chrome/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ include_rules = [
"+crypto",
"+gpu",
"+net",
"+pdf",
"+printing",
"+sql",
# Browser, renderer, common and tests access V8 for various purposes.
Expand Down
11 changes: 11 additions & 0 deletions chrome/app/chrome_main_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,11 @@
#include "remoting/client/plugin/pepper_entrypoints.h"
#endif

#if defined(ENABLE_PLUGINS) && (defined(CHROME_MULTIPLE_DLL_CHILD) || \
!defined(CHROME_MULTIPLE_DLL_BROWSER))
#include "pdf/pdf.h"
#endif

#if !defined(CHROME_MULTIPLE_DLL_BROWSER)
#include "chrome/child/pdf_child_init.h"

Expand Down Expand Up @@ -826,6 +831,12 @@ void ChromeMainDelegate::SandboxInitialized(const std::string& process_type) {
nacl_plugin::PPP_InitializeModule,
nacl_plugin::PPP_ShutdownModule);
#endif
#if defined(ENABLE_PLUGINS)
ChromeContentClient::SetPDFEntryFunctions(
chrome_pdf::PPP_GetInterface,
chrome_pdf::PPP_InitializeModule,
chrome_pdf::PPP_ShutdownModule);
#endif
#endif
}

Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/chromeos/file_manager/open_with_browser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "chrome/browser/ui/browser_tabstrip.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/scoped_tabbed_browser_displayer.h"
#include "chrome/common/chrome_content_client.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/chrome_switches.h"
#include "content/public/browser/browser_thread.h"
Expand Down Expand Up @@ -86,8 +87,8 @@ bool IsPepperPluginEnabled(Profile* profile,
bool IsPdfPluginEnabled(Profile* profile) {
DCHECK(profile);

base::FilePath plugin_path;
PathService::Get(chrome::FILE_PDF_PLUGIN, &plugin_path);
base::FilePath plugin_path = base::FilePath::FromUTF8Unsafe(
ChromeContentClient::kPDFPluginPath);
return IsPepperPluginEnabled(profile, plugin_path);
}

Expand Down
7 changes: 1 addition & 6 deletions chrome/browser/extensions/component_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -626,12 +626,7 @@ void ComponentLoader::AddDefaultComponentExtensionsWithBackgroundPages(
#endif // defined(GOOGLE_CHROME_BUILD)

#if defined(ENABLE_PLUGINS)
base::FilePath pdf_path;
content::PluginService* plugin_service =
content::PluginService::GetInstance();
if (switches::OutOfProcessPdfEnabled() &&
PathService::Get(chrome::FILE_PDF_PLUGIN, &pdf_path) &&
plugin_service->GetRegisteredPpapiPluginInfo(pdf_path)) {
if (switches::OutOfProcessPdfEnabled()) {
if (switches::PdfMaterialUIEnabled())
Add(IDR_PDF_MANIFEST_MATERIAL, base::FilePath(FILE_PATH_LITERAL("pdf")));
else
Expand Down
5 changes: 2 additions & 3 deletions chrome/browser/printing/print_preview_dialog_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,8 @@ namespace {

void EnableInternalPDFPluginForContents(WebContents* preview_dialog) {
// Always enable the internal PDF plugin for the print preview page.
base::FilePath pdf_plugin_path;
if (!PathService::Get(chrome::FILE_PDF_PLUGIN, &pdf_plugin_path))
return;
base::FilePath pdf_plugin_path = base::FilePath::FromUTF8Unsafe(
ChromeContentClient::kPDFPluginPath);

content::WebPluginInfo pdf_plugin;
if (!content::PluginService::GetInstance()->GetPluginInfoByPath(
Expand Down
103 changes: 20 additions & 83 deletions chrome/browser/printing/print_preview_pdf_generated_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#include "base/memory/scoped_ptr.h"
#include "base/path_service.h"
#include "base/run_loop.h"
#include "base/scoped_native_library.h"
#include "base/strings/string_split.h"
#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/printing/print_preview_dialog_controller.h"
Expand All @@ -39,6 +38,7 @@
#include "content/public/test/browser_test_utils.h"
#include "ipc/ipc_message_macros.h"
#include "net/base/filename_util.h"
#include "pdf/pdf.h"
#include "printing/pdf_render_settings.h"
#include "printing/units.h"
#include "ui/gfx/codec/png_codec.h"
Expand Down Expand Up @@ -323,33 +323,6 @@ class PrintPreviewPdfGeneratedBrowserTest : public InProcessBrowserTest {
ASSERT_TRUE(pdf_file.IsValid());
}

// Initializes function pointers from the PDF library. Called once when the
// test starts. The library is closed when the browser test ends.
void InitPdfFunctions() {
base::FilePath pdf_module_path;

ASSERT_TRUE(PathService::Get(chrome::FILE_PDF_PLUGIN, &pdf_module_path));
ASSERT_TRUE(base::PathExists(pdf_module_path));
pdf_lib_.Reset(base::LoadNativeLibrary(pdf_module_path, NULL));

ASSERT_TRUE(pdf_lib_.is_valid());
pdf_to_bitmap_func_ =
reinterpret_cast<PDFPageToBitmapProc>(
pdf_lib_.GetFunctionPointer("RenderPDFPageToBitmap"));

pdf_doc_info_func_ =
reinterpret_cast<GetPDFDocInfoProc>(
pdf_lib_.GetFunctionPointer("GetPDFDocInfo"));

pdf_page_size_func_ =
reinterpret_cast<GetPDFPageSizeByIndexProc>(
pdf_lib_.GetFunctionPointer("GetPDFPageSizeByIndex"));

ASSERT_TRUE(pdf_to_bitmap_func_);
ASSERT_TRUE(pdf_doc_info_func_);
ASSERT_TRUE(pdf_page_size_func_);
}

// Converts the PDF to a PNG file so that the layout test can do an image
// diff on this image and a reference image.
void PdfToPng() {
Expand All @@ -360,22 +333,22 @@ class PrintPreviewPdfGeneratedBrowserTest : public InProcessBrowserTest {
std::string pdf_data;

ASSERT_TRUE(base::ReadFileToString(pdf_file_save_path_, &pdf_data));
ASSERT_TRUE(pdf_doc_info_func_(pdf_data.data(),
pdf_data.size(),
&num_pages,
&max_width_in_points));
ASSERT_TRUE(chrome_pdf::GetPDFDocInfo(pdf_data.data(),
pdf_data.size(),
&num_pages,
&max_width_in_points));

ASSERT_GT(num_pages, 0);
double max_width_in_pixels =
ConvertUnitDouble(max_width_in_points, kPointsPerInch, kDpi);

for (int i = 0; i < num_pages; ++i) {
double width_in_points, height_in_points;
ASSERT_TRUE(pdf_page_size_func_(pdf_data.data(),
pdf_data.size(),
i,
&width_in_points,
&height_in_points));
ASSERT_TRUE(chrome_pdf::GetPDFPageSizeByIndex(pdf_data.data(),
pdf_data.size(),
i,
&width_in_points,
&height_in_points));

double width_in_pixels = ConvertUnitDouble(
width_in_points, kPointsPerInch, kDpi);
Expand Down Expand Up @@ -405,15 +378,15 @@ class PrintPreviewPdfGeneratedBrowserTest : public InProcessBrowserTest {
std::vector<uint8_t> page_bitmap_data(
kColorChannels * settings.area().size().GetArea());

ASSERT_TRUE(pdf_to_bitmap_func_(pdf_data.data(),
pdf_data.size(),
i,
page_bitmap_data.data(),
settings.area().size().width(),
settings.area().size().height(),
settings.dpi(),
settings.dpi(),
true));
ASSERT_TRUE(chrome_pdf::RenderPDFPageToBitmap(
pdf_data.data(),
pdf_data.size(),
i,
page_bitmap_data.data(),
settings.area().size().width(),
settings.area().size().height(),
settings.dpi(),
true));
FillPng(&page_bitmap_data,
width_in_pixels,
max_width_in_pixels,
Expand Down Expand Up @@ -572,41 +545,6 @@ class PrintPreviewPdfGeneratedBrowserTest : public InProcessBrowserTest {
scoped_ptr<PrintPreviewObserver> print_preview_observer_;
base::FilePath pdf_file_save_path_;

// These typedefs are function pointers to pdflib functions that give
// information about the PDF as a whole and about specific pages.

// Converts the PDF to a bitmap.
typedef bool (*PDFPageToBitmapProc)(const void* pdf_buffer,
int pdf_buffer_size,
int page_number,
void* bitmap_buffer,
int bitmap_width,
int bitmap_height,
int dpi_x,
int dpi_y,
bool autorotate);

// Gets the page count and maximum page width of the PDF in points.
typedef bool (*GetPDFDocInfoProc)(const void* pdf_buffer,
int buffer_size,
int* pages_count,
double* max_page_width);

// Gets the dimensions of a specific page within a PDF.
typedef bool (*GetPDFPageSizeByIndexProc)(const void* pdf_buffer,
int buffer_size,
int index,
double* width,
double* height);

// Instantiations of the function pointers described above.
PDFPageToBitmapProc pdf_to_bitmap_func_;
GetPDFDocInfoProc pdf_doc_info_func_;
GetPDFPageSizeByIndexProc pdf_page_size_func_;

// Used to open up the pdf plugin, which contains the functions above.
base::ScopedNativeLibrary pdf_lib_;

// Vector for storing the PNG to be sent to the layout test framework.
// TODO(ivandavid): Eventually change this to uint32_t and make everything
// work with that. It might be a bit tricky to fix everything to work with
Expand Down Expand Up @@ -641,8 +579,7 @@ IN_PROC_BROWSER_TEST_F(PrintPreviewPdfGeneratedBrowserTest,
// to send data to the browser test. Writing "EOF\n" to |std::cout| indicates
// that whatever block of data that the test was expecting has been completely
// sent. Sometimes EOF is printed to stderr because the test will expect it
// from stderr in addition to stdout for certain blocks of data.
InitPdfFunctions();
// from stderr in addition to stdout for certain blocks of data.=
SetupStdinAndSavePath();

while (true) {
Expand Down
5 changes: 0 additions & 5 deletions chrome/browser/resources/pdf/pdf_extension_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,6 @@ class PDFExtensionTest : public ExtensionApiTest {
}

void RunTestsInFile(std::string filename, std::string pdf_filename) {
base::FilePath pdf_path;
ASSERT_TRUE(PathService::Get(chrome::FILE_PDF_PLUGIN, &pdf_path));
ASSERT_TRUE(
content::PluginService::GetInstance()->GetRegisteredPpapiPluginInfo(
pdf_path));
ExtensionService* service = extensions::ExtensionSystem::Get(
profile())->extension_service();
service->component_loader()->Add(IDR_PDF_MANIFEST,
Expand Down
23 changes: 14 additions & 9 deletions chrome/child/pdf_child_init.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,20 @@ void InitializePDF() {
#if defined(OS_WIN)
// Need to patch a few functions for font loading to work correctly. This can
// be removed once we switch PDF to use Skia.
base::FilePath pdf;
if (PathService::Get(chrome::FILE_PDF_PLUGIN, &pdf) &&
base::PathExists(pdf)) {
g_iat_patch_createdca.Patch(pdf.value().c_str(), "gdi32.dll", "CreateDCA",
CreateDCAPatch);
g_iat_patch_get_font_data.Patch(pdf.value().c_str(), "gdi32.dll",
"GetFontData", GetFontDataPatch);
}
#endif
HMODULE current_module = NULL;
wchar_t current_module_name[MAX_PATH];
CHECK(GetModuleHandleEx(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS,
reinterpret_cast<LPCWSTR>(InitializePDF),
&current_module));
DWORD result = GetModuleFileNameW(current_module, current_module_name,
MAX_PATH);
if (!result || result == MAX_PATH)
return;
g_iat_patch_createdca.Patch(current_module_name, "gdi32.dll", "CreateDCA",
CreateDCAPatch);
g_iat_patch_get_font_data.Patch(current_module_name, "gdi32.dll",
"GetFontData", GetFontDataPatch);
#endif // OS_WIN
}

} // namespace chrome
2 changes: 0 additions & 2 deletions chrome/chrome.isolate
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
'files': [
'<(PRODUCT_DIR)/libffmpegsumo.so',
'<(PRODUCT_DIR)/libosmesa.so',
'<(PRODUCT_DIR)/libpdf.so',
],
},
}],
Expand Down Expand Up @@ -88,7 +87,6 @@
'<(PRODUCT_DIR)/ffmpegsumo.dll',
'<(PRODUCT_DIR)/libexif.dll',
'<(PRODUCT_DIR)/osmesa.dll',
'<(PRODUCT_DIR)/pdf.dll',
],
},
}],
Expand Down
13 changes: 10 additions & 3 deletions chrome/chrome_dll.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,11 @@
'../content/content.gyp:content_app_browser',
],
}],
['chrome_multiple_dll==0 and enable_plugins==1', {
'dependencies': [
'../pdf/pdf.gyp:pdf',
],
}],
['cld_version==1', {
'dependencies': [
'<(DEPTH)/third_party/cld/cld.gyp:cld',
Expand All @@ -278,9 +283,6 @@
# sets -order_file.
'ORDER_FILE': 'app/framework.order',
},
'dependencies': [
'../pdf/pdf.gyp:pdf',
],
'include_dirs': [
'<(grit_out_dir)',
],
Expand Down Expand Up @@ -372,6 +374,11 @@
}],
]
}],
['enable_plugins==1', {
'dependencies': [
'../pdf/pdf.gyp:pdf',
],
}],
],
}, # target chrome_child_dll
],
Expand Down
4 changes: 0 additions & 4 deletions chrome/chrome_dll_bundle.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@
# Bring in pdfsqueeze and run it on all pdfs
'../build/temp_gyp/pdfsqueeze.gyp:pdfsqueeze',
'../crypto/crypto.gyp:crypto',
'../pdf/pdf.gyp:pdf',
# On Mac, Flash gets put into the framework, so we need this
# dependency here. flash_player.gyp will copy the Flash bundle
# into PRODUCT_DIR.
Expand Down Expand Up @@ -147,9 +146,6 @@
},
{
'destination': '<(PRODUCT_DIR)/$(CONTENTS_FOLDER_PATH)/Internet Plug-Ins',
'files': [
'<(PRODUCT_DIR)/PDF.plugin',
],
'conditions': [
['disable_nacl!=1', {
'conditions': [
Expand Down
Loading

0 comments on commit dc0e439

Please sign in to comment.