Closed Bug 1382509 Opened 8 years ago Closed 8 years ago

Build PDFium as a separate library instead of linking it into libxul

Categories

(Core :: Printing: Output, enhancement, P3)

Product:
Core
Component:
Printing: Output
Platform:
Unspecified
Windows
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla57
Tracking Flags:
Tracking Status
firefox57 --- fixed

People

(Reporter: fatseng, Assigned: fatseng)

References

Details

Attachments

(4 files, 4 obsolete files)

Bug 1382509 - Part1. Align the calling convention with PDFium in PDFiumEngineShim.
59 bytes, text/x-review-board-request
fatseng
: review+
Details
Bug 1382509 - Part3. Build PDFium as a separate library instead of linking it into libxul
59 bytes, text/x-review-board-request
glandium
: review+
Details
Bug 1382509 - Part2. Remove the code from PDFiumEngineShim that allows for linking PDFium.dll
59 bytes, text/x-review-board-request
jwatt
: review+
Details
Bug 1382509 - Part4. Install the pdfium.dll
59 bytes, text/x-review-board-request
jwatt
: review+
Details
No description provided.
Building PDFium into xul caused that startup time increase 3%. The file size of xul increases around 3MB. It spends the time to loading xul while starting up. The PDFium is only used in printing now. I would like to pull out the PDFium form xul and build it as a dynamic library.
Assignee: nobody → fatseng
Status: NEW → ASSIGNED
Priority: -- → P3
See Also: → 1379124
Attachment #8888224 - Flags: feedback?(brsun)
Attachment #8888225 - Flags: feedback?(brsun)
See Also: → 1378797
Comment on attachment 8888224 [details] Bug 1382509 - Part1. Align the calling convention with PDFium in PDFiumEngineShim. https://reviewboard.mozilla.org/r/159176/#review164594 ::: widget/windows/PDFiumEngineShim.h:12 (Diff revision 1) > #define PDFIUMENGINESHIM_H > > #include "prlink.h" > #include "fpdfview.h" > > -//#define USE_EXTERNAL_PDFIUM > +#define USE_PDFIUM_DYNAMIC_LIB Suggest to remove this definition if this will always be defined without any exceptions.
Comment on attachment 8888225 [details] Bug 1382509 - Part3. Build PDFium as a separate library instead of linking it into libxul https://reviewboard.mozilla.org/r/159178/#review164596 ::: modules/pdfium/moz.build:522 (Diff revision 1) > ] > > # We allow warnings for third-party code that can be updated from upstream. > ALLOW_COMPILER_WARNINGS = True > > -FINAL_LIBRARY = 'xul' > +GeckoSharedLibrary('pdfiumlib', linkage=None) Suggest to use 'pdfium' directly. ::: modules/pdfium/moz.build:525 (Diff revision 1) > ALLOW_COMPILER_WARNINGS = True > > -FINAL_LIBRARY = 'xul' > +GeckoSharedLibrary('pdfiumlib', linkage=None) > + > +if CONFIG['OS_TARGET'] == 'WINNT': > + DEFFILE = SRCDIR + '/pdfiumlib.def' ditto
Comment on attachment 8888224 [details] Bug 1382509 - Part1. Align the calling convention with PDFium in PDFiumEngineShim. f+ if comments would be addressed.
Attachment #8888224 - Flags: feedback?(brsun) → feedback+
Comment on attachment 8888225 [details] Bug 1382509 - Part3. Build PDFium as a separate library instead of linking it into libxul f+ if comments would be addressed.
Comment on attachment 8888224 [details] Bug 1382509 - Part1. Align the calling convention with PDFium in PDFiumEngineShim. Building PDFium into xul caused that startup time increase 3.57%(Bug 1379124). It spends the time to loading xul while starting up. The PDFium is only used in printing now, so the managers decide to pull out the PDFium from xul and build it as a dynamic library.
Attachment #8888224 - Flags: review?(jwatt)
Attachment #8888225 - Flags: review?(jwatt)
By using a DEF file to explicitly export these eight symbols would result some inconsistencies. These eight exported symbols on pdfium.dll would not have any decorations, but all others have _ and @ decorations. This inconsistency between calling conventions and decorated names would confuse future maintainers. In order to mitigate the confusion, I would suggest either to remove all decorations of all symbols at once, or to use __cdecl to export symbols from pdfium.dll instead of __stdcall. For the simplicity, I would prefer to use __cdecl directly to save the maintaining efforts of the DEF file (which might contain a complete list of all exported PDFium symbols.)
Farmer, do you plan to update the patches to address Bruce's comments in comment 11?
Flags: needinfo?(fatseng)
Flags: needinfo?(fatseng)
Attachment #8888224 - Flags: review?(jwatt)
Attachment #8888225 - Flags: review?(jwatt)
(In reply to Jonathan Watt [:jwatt] from comment #12) > Farmer, do you plan to update the patches to address Bruce's comments in > comment 11? Yes, I am working on this. I am suffering some problem by pulling out pdfium.
One additional option could be: keeping using the original calling conventions (i.e. __stdcall) and using decorated symbols (i.e. function name with _ and @ decorations) to load functions. The only drawbacks of this approach is that, it would require more efforts to switch between load-time dynamic linking and run-time dynamic linking (if we would like to do so somehow in the future). But since we don't (and probably won't) use load-time dynamic linking for PDFium.dll, I guess this option might be acceptable as well.
(In reply to Bruce Sun [:brsun] from comment #14) > One additional option could be: keeping using the original calling > conventions (i.e. __stdcall) and using decorated symbols (i.e. function name > with _ and @ decorations) to load functions. The only drawbacks of this > approach is that, it would require more efforts to switch between load-time > dynamic linking and run-time dynamic linking (if we would like to do so > somehow in the future). Because we want to load the dynamic library at run-time, not only "__stdcall", but also "__cdcel" needs to spend the time to load the dynamic library. Is it?
Flags: needinfo?(brsun)
I am suffering a problem that how to pack the dll into the installer. Checking on this.
> it would require more efforts to switch between load-time dynamic linking and run-time dynamic linking Well...I forgot what would be the extra efforts in my original thoughts. But anyway, using actual symbol names to load function pointers seems rational to me.
Flags: needinfo?(brsun)
(In reply to Farmer Tseng[:fatseng] from comment #16) > I am suffering a problem that how to pack the dll into the installer. > Checking on this. We have to modify browser\installer\Makefile.in and package-manifest.in to pack pdfium.dll into the installer.
After pulling out pdfium from xul, it passed on my NB. But gtest failed on try server. I am debugging it.
(In reply to Farmer Tseng[:fatseng] from comment #19) > After pulling out pdfium from xul, it passed on my NB. But gtest failed on > try server. I am debugging it. I fixed it. We have to align calling conversion with pdfium.dll. Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1687cb199ade093b9903666936f7810284eef43c&selectedJob=118833865
(In reply to Bruce Sun [:brsun] from comment #11) > By using a DEF file to explicitly export these eight symbols would result > some inconsistencies. These eight exported symbols on pdfium.dll would not > have any decorations, but all others have _ and @ decorations. This > inconsistency between calling conventions and decorated names would confuse > future maintainers. In order to mitigate the confusion, I would suggest > either to remove all decorations of all symbols at once, or to use __cdecl > to export symbols from pdfium.dll instead of __stdcall. For the simplicity, > I would prefer to use __cdecl directly to save the maintaining efforts of > the DEF file (which might contain a complete list of all exported PDFium > symbols.) Finally, we decide to use __cdecl to export the symbols from pdfium.dll.
Attachment #8888224 - Flags: feedback?(brsun)
Attachment #8891239 - Flags: feedback?(brsun)
Attachment #8891240 - Flags: feedback?(brsun)
Attachment #8891241 - Flags: feedback?(brsun)
Attachment #8891242 - Flags: feedback?(brsun)
Attachment #8888225 - Flags: feedback?(brsun)
Comment on attachment 8891239 [details] Bug 1382509 - Part2. use __cdecl to export symbols from pdfium.dll https://reviewboard.mozilla.org/r/162434/#review167698 ::: commit-message-d06e5:1 (Diff revision 1) > +Bug1382509 - Part2. Add a patch file to patch PDFium to export symbols by __cdecl from pdfium.dll The order of part 1 and part 2 seems reversed to me. Suppose your part 1 is exactly just a reflection after applying your part 2, right? Please double check your codes still work as expected after executing update.sh. ::: modules/pdfium/patches/bug1382509_use_cdecl_to_export_symbols.patch:7 (Diff revision 1) > +# User Farmer Tseng <fatseng@mozilla.com> > +# Date 1501214170 -28800 > +# Fri Jul 28 11:56:10 2017 +0800 > +# Node ID 22fd9dbd7e5c3fc466ec6b4ceb57dbf8ad0f8f0a > +# Parent a48f587f835e99daee9fd071801ed436deb412d1 > +Bug 1382509 - Part1. use __cdecl to export symbols from pdfium.dll Probably you don't need 'Part1.' in this .patch file.
Comment on attachment 8891240 [details] Bug 1382509 - part3. Align the calling convention with PDFium. https://reviewboard.mozilla.org/r/162436/#review167708 ::: widget/windows/PDFiumEngineShim.h:20 (Diff revision 1) > #include <windows.h> > > namespace mozilla { > namespace widget { > > -typedef void (*FPDF_InitLibrary_Pfn)(); > +typedef void (STDCALL *FPDF_InitLibrary_Pfn)(); Don't use STDCALL here for the following reasons: - This macro is really misleading especially we don't use `__stdcall` calling conventions at all for pdfium.dll. - Whether the calling convertion is used solely depends on the compiler options in our cases in practice. Adding STDCALL here doesn't help future maintainers to be awared of this behavior. I would suggest either to simply drop this modification from your patch queues, or to have a better hints for the actual calling convention.
Comment on attachment 8888225 [details] Bug 1382509 - Part3. Build PDFium as a separate library instead of linking it into libxul https://reviewboard.mozilla.org/r/159178/#review167720 ::: widget/windows/moz.build:102 (Diff revision 3) > 'nsDeviceContextSpecWin.cpp', > 'nsPrintOptionsWin.cpp', > 'nsPrintSettingsWin.cpp', > ] > if CONFIG['MOZ_ENABLE_SKIA_PDF']: > + DEFINES['FPDFSDK_EXPORTS'] = True By adding 'FPDFSDK_EXPORTS' definition here, all public pdfium functions would be declared as `__declspec(dllexport)` at the caller side as well. I am not sure whether it would make any harm to us or not, but I guess `__declspec(dllexport)` is not intended to be used at the caller side by its design. I would suggest not to add this definition in this moz.build file.
Comment on attachment 8888224 [details] Bug 1382509 - Part1. Align the calling convention with PDFium in PDFiumEngineShim. It looks good to me except for the patch order.
Attachment #8888224 - Flags: feedback?(brsun) → feedback+
Comment on attachment 8891239 [details] Bug 1382509 - Part2. use __cdecl to export symbols from pdfium.dll Please reverse the patch order and make sure it works as expected after executing update.sh.
Attachment #8891239 - Flags: feedback?(brsun)
Attachment #8891240 - Flags: feedback?(brsun)
Comment on attachment 8891241 [details] Bug 1382509 - Part2. Remove the code from PDFiumEngineShim that allows for linking PDFium.dll LGTM
Attachment #8891241 - Flags: feedback?(brsun) → feedback+
Attachment #8888225 - Flags: feedback?(brsun)
Comment on attachment 8891242 [details] Bug 1382509 - Part4. Install the pdfium.dll I am sorry but probably you need to find someone else for feedback. I am not familiar with packaging stuffs.
Attachment #8891242 - Flags: feedback?(brsun)
Comment on attachment 8891240 [details] Bug 1382509 - part3. Align the calling convention with PDFium. https://reviewboard.mozilla.org/r/162436/#review167708 > Don't use STDCALL here for the following reasons: > - This macro is really misleading especially we don't use `__stdcall` calling conventions at all for pdfium.dll. > - Whether the calling convertion is used solely depends on the compiler options in our cases in practice. Adding STDCALL here doesn't help future maintainers to be awared of this behavior. > > I would suggest either to simply drop this modification from your patch queues, or to have a better hints for the actual calling convention. Exactly, we don't need to use STDCALL because we decided to use __cdecl calling conventions for pdfium.dll. I will remove this patch.
Comment on attachment 8888225 [details] Bug 1382509 - Part3. Build PDFium as a separate library instead of linking it into libxul https://reviewboard.mozilla.org/r/159178/#review167720 > By adding 'FPDFSDK_EXPORTS' definition here, all public pdfium functions would be declared as `__declspec(dllexport)` at the caller side as well. I am not sure whether it would make any harm to us or not, but I guess `__declspec(dllexport)` is not intended to be used at the caller side by its design. > > I would suggest not to add this definition in this moz.build file. Since I will remove attachment 8891240 [details], we don't need to add 'FPDFSDK_EXPORTS' in this moz.build file.
Attachment #8891240 - Attachment is obsolete: true
Attachment #8888224 - Flags: feedback?(brsun)
Attachment #8888225 - Flags: feedback?(brsun)
Comment on attachment 8891241 [details] Bug 1382509 - Part2. Remove the code from PDFiumEngineShim that allows for linking PDFium.dll carry over f+ from Comment 33
Attachment #8891241 - Flags: feedback+
Comment on attachment 8891239 [details] Bug 1382509 - Part2. use __cdecl to export symbols from pdfium.dll I switch the patch order for "Bug1382509 - Add a patch file to patch PDFium to export symbols by __cdecl from pdfium.dll" and "Bug 1382509 - Part1. use __cdecl to export symbols from pdfium.dll" Carry over f+ from Comment 31
Attachment #8891239 - Flags: feedback+
Attachment #8888224 - Flags: feedback?(brsun)
Attachment #8888225 - Flags: feedback?(brsun)
Comment on attachment 8888224 [details] Bug 1382509 - Part1. Align the calling convention with PDFium in PDFiumEngineShim. https://reviewboard.mozilla.org/r/159176/#review168552 ::: modules/pdfium/patches/bug1382509_use_cdecl_to_export_symbols.patch:26 (Diff revision 5) > + // On Windows system, functions are exported in a DLL > + #define DLLEXPORT __declspec(dllexport) > +-#define STDCALL __stdcall > ++// Remove __stdcall, use default convention to export symbols from pdfium.dll. > ++// __cdecl is the default calling convention for C and C++ programs. > ++// https://msdn.microsoft.com/en-us/library/zkwh89ks.aspx I will flag f+ to this patch, but please explain WHY we have to do this modification. The intention is not so clear by simply glancing at comments or the patch itself.
Attachment #8888224 - Flags: feedback?(brsun) → feedback+
Attachment #8888225 - Flags: feedback?(brsun) → feedback+
Building PDFium into xul caused that startup time increase 3.57% (bug 1379124). To fix this issue, we decide to pull out the pdfium from xul. After pulling out the pdfium, the startup time is back to the original situation[1]. Pdfium uses some third party libraries which are shared with xul. Pull out the pdfium. The installed size would increase to 259 KB[2]. The file size would increase 0.99 MB[3]. [1] Windows 2012 opt. (32 bits) xpref ----------------------------------------------------------------- Not use pdfium 62066104 [4] call pdfium functions via xul 64491157 [5] call pdfium functions via pdfium.dll 62066324 [6] [2] Windows 2012 opt. (32 bits) installer size ------------------------------------------------------------------ call pdfium functions via xul 51541944 [7] call pdfium functions via pdfium.dll 51801550 [8] [3] Windows 2012 opt. (32 bits) library size ------------------------------------------------------------------- Build pdfium into xul 58.3 MB (xul.dll) Pull out the pdfium from xul 56.0 MB (xul.dll) 3.29 MB (pdfium.dll) [4] https://treeherder.mozilla.org/#/jobs?repo=try&revision=29ab4c02bb8d1244af99b4d0f7c84edf1fdb4e16&selectedJob=116864546 [5] https://treeherder.mozilla.org/#/jobs?repo=try&revision=27dafa40a593f82b10880301f604d7a2e363d8b8&selectedJob=116841580 [6] https://treeherder.mozilla.org/#/jobs?repo=try&revision=1687cb199ade093b9903666936f7810284eef43c&selectedJob=118840070 [7] https://treeherder.mozilla.org/#/jobs?repo=try&revision=27dafa40a593f82b10880301f604d7a2e363d8b8&selectedJob=116833162 [8] https://treeherder.mozilla.org/#/jobs?repo=try&revision=1687cb199ade093b9903666936f7810284eef43c&selectedJob=118833865
Attachment #8888224 - Flags: review?(jwatt)
Attachment #8891239 - Flags: review?(jwatt)
Attachment #8891241 - Flags: review?(jwatt)
Attachment #8888225 - Flags: review?(jwatt)
Attachment #8891242 - Flags: review?(jwatt)
Update the result of installer size for Windows 2012 - 64bits opt. The installed size increases to 298 KB[1]. The file size would increase 1.12 MB[2]. [1] Windows 2012 opt. (64 bits) installer size ------------------------------------------------------------------ call pdfium functions via xul 56921023 [3] call pdfium functions via pdfium.dll 57219893 [4] [2] Windows 2012 opt. (64 bits) library size ------------------------------------------------------------------- Build pdfium into xul 71.4 MB (xul.dll) Pull out the pdfium from xul 68.8 MB (xul.dll) 3.72 MB (pdfium.dll) [3] https://treeherder.mozilla.org/#/jobs?repo=try&revision=2d0c9d774c5d75cd35811939e06208fd0972f684&selectedJob=121334639 [4] https://treeherder.mozilla.org/#/jobs?repo=try&revision=1687cb199ade093b9903666936f7810284eef43c&selectedJob=118833791
Comment on attachment 8891239 [details] Bug 1382509 - Part2. use __cdecl to export symbols from pdfium.dll https://reviewboard.mozilla.org/r/162434/#review170668 ::: commit-message-d06e5:1 (Diff revision 4) > +Bug 1382509 - Part2. use __cdecl to export symbols from pdfium.dll I think you should give this the message "Apply the PDFium patch modules/pdfium/patches/bug1382509_use_cdecl_to_export_symbols.patch" Even if you change part 1, you can carry this r+ (you don't need to ask for it again) since you're just applying that patch.
Attachment #8891239 - Flags: review?(jwatt) → review+
Comment on attachment 8891241 [details] Bug 1382509 - Part2. Remove the code from PDFiumEngineShim that allows for linking PDFium.dll https://reviewboard.mozilla.org/r/162438/#review170670
Attachment #8891241 - Flags: review?(jwatt) → review+
Comment on attachment 8891241 [details] Bug 1382509 - Part2. Remove the code from PDFiumEngineShim that allows for linking PDFium.dll https://reviewboard.mozilla.org/r/162438/#review170672 ::: commit-message-4aefa:1 (Diff revision 4) > +Bug 1382509 - Part3. Use PDFium dynamtic library Please make the commit message: "Remove the code from PDFiumEngineShim that allows for linking PDFium into libxul"
Comment on attachment 8888225 [details] Bug 1382509 - Part3. Build PDFium as a separate library instead of linking it into libxul https://reviewboard.mozilla.org/r/159178/#review170676 ::: commit-message-730dc:1 (Diff revision 6) > +Bug 1382509 - Part4. Pull out PDFium from xul.dll Make the commit message "Build PDFium as a separate library instead of linking it into libxul"
Attachment #8888225 - Flags: review?(jwatt) → review+
Comment on attachment 8891242 [details] Bug 1382509 - Part4. Install the pdfium.dll https://reviewboard.mozilla.org/r/162440/#review170866
Attachment #8891242 - Flags: review?(jwatt) → review+
Comment on attachment 8888224 [details] Bug 1382509 - Part1. Align the calling convention with PDFium in PDFiumEngineShim. Mike, could you take a look at this or suggest a reviewer? I'm not familiar with linking on windows. This patch seems simple enough, but maybe there are issues I'm unaware of.
Attachment #8888224 - Flags: review?(mh+mozilla)
Attachment #8888224 - Flags: review?(jwatt)
Attachment #8888224 - Flags: feedback+
Comment on attachment 8888224 [details] Bug 1382509 - Part1. Align the calling convention with PDFium in PDFiumEngineShim. https://reviewboard.mozilla.org/r/159176/#review171630 Can you investigate using a .def instead, that should allow to skip the mangling while keeping the stdcall convention, and as a bonus, could allow to limit the symbols you export from the dll (although that might require to remove the dllexports)
Attachment #8888224 - Flags: review?(mh+mozilla)
Blocks: 1388951
Attachment #8891239 - Attachment is obsolete: true
Comment on attachment 8888224 [details] Bug 1382509 - Part1. Align the calling convention with PDFium in PDFiumEngineShim. Hi Glandium, Please help review it. Since we would use .def to build pdfium.dll and keep stdcall convention in the library, the PDFiumEngineShim calling convention should align with pdfium.
Attachment #8888224 - Flags: review?(mh+mozilla)
Attachment #8888225 - Flags: review?(mh+mozilla)
Summary: Pull out PDFium from xul → Build PDFium as a separate library instead of linking it into libxul
Comment on attachment 8888224 [details] Bug 1382509 - Part1. Align the calling convention with PDFium in PDFiumEngineShim. https://reviewboard.mozilla.org/r/159176/#review173884
Attachment #8888224 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8888225 [details] Bug 1382509 - Part3. Build PDFium as a separate library instead of linking it into libxul https://reviewboard.mozilla.org/r/159178/#review173890 ::: modules/pdfium/moz.build:19 (Diff revision 7) > # pdfium_common_config > DEFINES['OPJ_STATIC'] = True > DEFINES['PNG_PREFIX'] = True > DEFINES['PNG_USE_READ_MACROS'] = True > if CONFIG['OS_TARGET'] == 'WINNT': > + DEFINES['FPDFSDK_EXPORTS'] = True I /think/ this will end up exporting more than the symbols in the .def file. Do you have a try build with these changes?
Attachment #8888225 - Flags: review?(mh+mozilla)
Comment on attachment 8888224 [details] Bug 1382509 - Part1. Align the calling convention with PDFium in PDFiumEngineShim. Carry over r+ by Comment 69
Attachment #8888224 - Flags: review+
Attachment #8897724 - Flags: review?(mh+mozilla)
Attachment #8897725 - Flags: review?(mh+mozilla)
Attachment #8888225 - Flags: review?(mh+mozilla)
Comment on attachment 8897724 [details] Bug 1382509 - Part3. Add a patch file to patch PDFium setting DLLEXPORT to nothing https://reviewboard.mozilla.org/r/169028/#review175246 I'm confused. Why do you need this? Setting DLLEXPORT to nothing, combined with the .def should export those symbols.
Attachment #8897724 - Flags: review?(mh+mozilla)
(In reply to Mike Hommey [:glandium] from comment #84) > Comment on attachment 8897724 [details] > Bug 1382509 - Part3. Add a patch file to patch PDFium to limit exporting > symbols from pdfium.dll > > https://reviewboard.mozilla.org/r/169028/#review175246 > > I'm confused. Why do you need this? Setting DLLEXPORT to nothing, combined > with the .def should export those symbols. Thanks, I will try it.
Attachment #8897724 - Flags: review?(mh+mozilla)
Attachment #8897725 - Flags: review?(mh+mozilla)
Attachment #8888225 - Flags: review?(mh+mozilla)
Comment on attachment 8888225 [details] Bug 1382509 - Part3. Build PDFium as a separate library instead of linking it into libxul https://reviewboard.mozilla.org/r/159178/#review176100 ::: modules/pdfium/moz.build:19 (Diff revision 11) > # pdfium_common_config > DEFINES['OPJ_STATIC'] = True > DEFINES['PNG_PREFIX'] = True > DEFINES['PNG_USE_READ_MACROS'] = True > if CONFIG['OS_TARGET'] == 'WINNT': > + DEFINES['FPDFSDK_EXPORTS'] = True Wouldn't this work equally without setting FPDFSDK_EXPORTS, and without the patch?
Attachment #8888225 - Flags: review?(mh+mozilla)
Attachment #8897724 - Attachment is obsolete: true
Attachment #8897724 - Flags: review?(mh+mozilla)
Attachment #8897725 - Attachment is obsolete: true
Attachment #8897725 - Flags: review?(mh+mozilla)
Attachment #8888225 - Flags: review?(mh+mozilla)
Comment on attachment 8888225 [details] Bug 1382509 - Part3. Build PDFium as a separate library instead of linking it into libxul https://reviewboard.mozilla.org/r/159178/#review176186 Much simpler :)
Attachment #8888225 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8888224 [details] Bug 1382509 - Part1. Align the calling convention with PDFium in PDFiumEngineShim. Carry over r+ by Comment 69
Attachment #8888224 - Flags: review+
Pushed by fatseng@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3360dfa8c756 Part1. Align the calling convention with PDFium in PDFiumEngineShim. r=glandium https://hg.mozilla.org/integration/autoland/rev/2d7a01b9ac51 Part2. Remove the code from PDFiumEngineShim that allows for linking PDFium.dll r=jwatt https://hg.mozilla.org/integration/autoland/rev/806ba87bbf3c Part3. Build PDFium as a separate library instead of linking it into libxul r=glandium,jwatt https://hg.mozilla.org/integration/autoland/rev/68db42ac8c23 Part4. Install the pdfium.dll r=jwatt
No longer blocks: 1388951
Noticed these Talos improvements! == Change summary for alert #8917 (as of August 22 2017 12:07 UTC) == Improvements: 4% tp5n main_startup_fileio windows7-32 pgo e10s 69,103,744.08 -> 66,554,226.08 3% tp5n main_startup_fileio windows7-32 opt e10s 68,798,907.92 -> 66,497,617.17 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=8917
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: