New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 766451 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Dec 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

PdfConverter (PDF to EMF) should be servicified

Project Member Reported by jcivelli@chromium.org, Sep 19 2017

Issue description

The PdfConverter class, used to convert PDF to EMF should be mojofied and servicified.
 
Components: Internals>Printing
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9c50a0c545b8624819a539a45144591757bc174e

commit 9c50a0c545b8624819a539a45144591757bc174e
Author: Jay Civelli <jcivelli@chromium.org>
Date: Wed Dec 06 20:39:33 2017

Moving PdfRenderSettings to its own mojom.

Moving PdfRenderSettings from pdf_to_pwg_raster_converter.mojom
to its own mojom/typemap/traits files as it will soon be used by other
interfaces.

Bug:  766451 
Change-Id: Ib50ceeefa62a56c91048d6a346323b4cad601359
Reviewed-on: https://chromium-review.googlesource.com/811448
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522181}
[modify] https://crrev.com/9c50a0c545b8624819a539a45144591757bc174e/chrome/common/printing/BUILD.gn
[add] https://crrev.com/9c50a0c545b8624819a539a45144591757bc174e/chrome/common/printing/pdf_render_settings.mojom
[add] https://crrev.com/9c50a0c545b8624819a539a45144591757bc174e/chrome/common/printing/pdf_render_settings.typemap
[add] https://crrev.com/9c50a0c545b8624819a539a45144591757bc174e/chrome/common/printing/pdf_render_settings_struct_traits.cc
[add] https://crrev.com/9c50a0c545b8624819a539a45144591757bc174e/chrome/common/printing/pdf_render_settings_struct_traits.h
[modify] https://crrev.com/9c50a0c545b8624819a539a45144591757bc174e/chrome/common/printing/pdf_to_pwg_raster_converter.mojom
[modify] https://crrev.com/9c50a0c545b8624819a539a45144591757bc174e/chrome/common/printing/pdf_to_pwg_raster_converter.typemap
[modify] https://crrev.com/9c50a0c545b8624819a539a45144591757bc174e/chrome/common/printing/pdf_to_pwg_raster_converter_struct_traits.cc
[modify] https://crrev.com/9c50a0c545b8624819a539a45144591757bc174e/chrome/common/printing/pdf_to_pwg_raster_converter_struct_traits.h
[modify] https://crrev.com/9c50a0c545b8624819a539a45144591757bc174e/chrome/common/printing/typemaps.gni

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/48bff020e422f5722ae4fef55a9da6a12a70b1c2

commit 48bff020e422f5722ae4fef55a9da6a12a70b1c2
Author: Jay Civelli <jcivelli@chromium.org>
Date: Wed Dec 06 23:16:31 2017

Harmonizing names in PDFToPWGRasterConverter related files.

Changing names in PDFToPWGRasterConverterService related files from all
caps to the style guide preferred way: PDF -> Pdf, PWG -> pwg.

Bug:  766451 
Change-Id: I6e3329d218a98917bf0a2474d46612dcf91976db
Reviewed-on: https://chromium-review.googlesource.com/811450
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522241}
[modify] https://crrev.com/48bff020e422f5722ae4fef55a9da6a12a70b1c2/chrome/browser/printing/pwg_raster_converter.cc
[modify] https://crrev.com/48bff020e422f5722ae4fef55a9da6a12a70b1c2/chrome/browser/printing/pwg_raster_converter_browsertest.cc
[modify] https://crrev.com/48bff020e422f5722ae4fef55a9da6a12a70b1c2/chrome/common/printing/pdf_render_settings.mojom
[modify] https://crrev.com/48bff020e422f5722ae4fef55a9da6a12a70b1c2/chrome/common/printing/pdf_render_settings.typemap
[modify] https://crrev.com/48bff020e422f5722ae4fef55a9da6a12a70b1c2/chrome/common/printing/pdf_render_settings_struct_traits.cc
[modify] https://crrev.com/48bff020e422f5722ae4fef55a9da6a12a70b1c2/chrome/common/printing/pdf_render_settings_struct_traits.h
[modify] https://crrev.com/48bff020e422f5722ae4fef55a9da6a12a70b1c2/chrome/common/printing/pdf_to_pwg_raster_converter.mojom
[modify] https://crrev.com/48bff020e422f5722ae4fef55a9da6a12a70b1c2/chrome/common/printing/pdf_to_pwg_raster_converter.typemap
[modify] https://crrev.com/48bff020e422f5722ae4fef55a9da6a12a70b1c2/chrome/common/printing/pdf_to_pwg_raster_converter_struct_traits.cc
[modify] https://crrev.com/48bff020e422f5722ae4fef55a9da6a12a70b1c2/chrome/common/printing/pdf_to_pwg_raster_converter_struct_traits.h
[modify] https://crrev.com/48bff020e422f5722ae4fef55a9da6a12a70b1c2/chrome/utility/chrome_content_utility_client.cc
[modify] https://crrev.com/48bff020e422f5722ae4fef55a9da6a12a70b1c2/chrome/utility/printing/pdf_to_pwg_raster_converter_impl.cc
[modify] https://crrev.com/48bff020e422f5722ae4fef55a9da6a12a70b1c2/chrome/utility/printing/pdf_to_pwg_raster_converter_impl.h
[modify] https://crrev.com/48bff020e422f5722ae4fef55a9da6a12a70b1c2/chrome/utility/printing/pdf_to_pwg_raster_converter_manifest.json
[modify] https://crrev.com/48bff020e422f5722ae4fef55a9da6a12a70b1c2/chrome/utility/printing/pdf_to_pwg_raster_converter_service.cc
[modify] https://crrev.com/48bff020e422f5722ae4fef55a9da6a12a70b1c2/chrome/utility/printing/pdf_to_pwg_raster_converter_service.h
[modify] https://crrev.com/48bff020e422f5722ae4fef55a9da6a12a70b1c2/content/public/app/mojo/content_utility_manifest.json

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/80a982a13319eb6c0853130d2c06549c7986c298

commit 80a982a13319eb6c0853130d2c06549c7986c298
Author: Jay Civelli <jcivelli@chromium.org>
Date: Thu Dec 07 19:18:08 2017

Mojofying PdfToEmfConverter.

Mojofying PdfToEmfConverter in preparation of its servicification.
The PDF engine running in the service process might query for
pre-caching of fonts, which was previously done as part of the utility
process initialization in PrintingHandler. It is now done through a
client interface that must be provided when creating an instance of the
PdfToEmfConverter interface through the PdfToEmfConverterFactory.

Bug:  766451 
Change-Id: Ie0a990121c0676c5a0a4d48ef0ea15339aefb4b9
Reviewed-on: https://chromium-review.googlesource.com/801778
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522501}
[modify] https://crrev.com/80a982a13319eb6c0853130d2c06549c7986c298/chrome/browser/printing/pdf_to_emf_converter.cc
[modify] https://crrev.com/80a982a13319eb6c0853130d2c06549c7986c298/chrome/common/printing/BUILD.gn
[add] https://crrev.com/80a982a13319eb6c0853130d2c06549c7986c298/chrome/common/printing/pdf_to_emf_converter.mojom
[modify] https://crrev.com/80a982a13319eb6c0853130d2c06549c7986c298/chrome/utility/BUILD.gn
[modify] https://crrev.com/80a982a13319eb6c0853130d2c06549c7986c298/chrome/utility/chrome_content_utility_client.cc
[add] https://crrev.com/80a982a13319eb6c0853130d2c06549c7986c298/chrome/utility/printing/pdf_to_emf_converter_factory_impl.cc
[add] https://crrev.com/80a982a13319eb6c0853130d2c06549c7986c298/chrome/utility/printing/pdf_to_emf_converter_factory_impl.h
[add] https://crrev.com/80a982a13319eb6c0853130d2c06549c7986c298/chrome/utility/printing/pdf_to_emf_converter_impl.cc
[add] https://crrev.com/80a982a13319eb6c0853130d2c06549c7986c298/chrome/utility/printing/pdf_to_emf_converter_impl.h
[modify] https://crrev.com/80a982a13319eb6c0853130d2c06549c7986c298/chrome/utility/printing_handler.cc
[modify] https://crrev.com/80a982a13319eb6c0853130d2c06549c7986c298/chrome/utility/printing_handler.h
[modify] https://crrev.com/80a982a13319eb6c0853130d2c06549c7986c298/content/public/app/mojo/content_utility_manifest.json

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0f8b6578635ded37a9ede666f64818d88dafc130

commit 0f8b6578635ded37a9ede666f64818d88dafc130
Author: Jay Civelli <jcivelli@chromium.org>
Date: Thu Dec 07 19:32:07 2017

Changing PdfToEmfConverter to use BindOnce.

Changing from base::Bind to base::BindOnce in the PdfToEmfConverter
class.

Bug:  766451 
Change-Id: Ic93ec8d23d1c610cd2f67b7fb462479a612355fe
Reviewed-on: https://chromium-review.googlesource.com/812065
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522508}
[modify] https://crrev.com/0f8b6578635ded37a9ede666f64818d88dafc130/chrome/browser/printing/pdf_to_emf_converter.cc
[modify] https://crrev.com/0f8b6578635ded37a9ede666f64818d88dafc130/chrome/browser/printing/pdf_to_emf_converter.h
[modify] https://crrev.com/0f8b6578635ded37a9ede666f64818d88dafc130/chrome/browser/printing/print_job.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Dec 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/64e646e7eef28d3feb5f51a9f8e9e58f4700917e

commit 64e646e7eef28d3feb5f51a9f8e9e58f4700917e
Author: Jay Civelli <jcivelli@chromium.org>
Date: Fri Dec 08 22:38:35 2017

Revert "Changing PdfToEmfConverter to use BindOnce."

This reverts commit 0f8b6578635ded37a9ede666f64818d88dafc130.

Reason for revert: breaks printing on Windows.

Tbr: thestig

Original change's description:
> Changing PdfToEmfConverter to use BindOnce.
> 
> Changing from base::Bind to base::BindOnce in the PdfToEmfConverter
> class.
> 
> Bug:  766451 
> Change-Id: Ic93ec8d23d1c610cd2f67b7fb462479a612355fe
> Reviewed-on: https://chromium-review.googlesource.com/812065
> Commit-Queue: Jay Civelli <jcivelli@chromium.org>
> Reviewed-by: Lei Zhang <thestig@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#522508}

TBR=thestig@chromium.org,jcivelli@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  766451 
Change-Id: I8633d220e1dd7d4a59a79326a88dcb9a247492cf
Reviewed-on: https://chromium-review.googlesource.com/818175
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522902}
[modify] https://crrev.com/64e646e7eef28d3feb5f51a9f8e9e58f4700917e/chrome/browser/printing/pdf_to_emf_converter.cc
[modify] https://crrev.com/64e646e7eef28d3feb5f51a9f8e9e58f4700917e/chrome/browser/printing/pdf_to_emf_converter.h
[modify] https://crrev.com/64e646e7eef28d3feb5f51a9f8e9e58f4700917e/chrome/browser/printing/print_job.cc

Status: Started (was: Untriaged)
Project Member

Comment 9 by bugdroid1@chromium.org, Dec 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f9c90b5c3f59e7bdba839e0a864859189f242ed9

commit f9c90b5c3f59e7bdba839e0a864859189f242ed9
Author: Jay Civelli <jcivelli@chromium.org>
Date: Fri Dec 15 00:12:12 2017

Changing PdfToEmfConverter to use BindOnce/Repeating.

Changing from base::Bind to base::BindOnce/Repeating in the
PdfToEmfConverter class.

Bug:  766451 
Change-Id: I8e319028f1610a1a0e40cc20af642afa711b25ee
Reviewed-on: https://chromium-review.googlesource.com/826319
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524248}
[modify] https://crrev.com/f9c90b5c3f59e7bdba839e0a864859189f242ed9/chrome/browser/printing/pdf_to_emf_converter.cc
[modify] https://crrev.com/f9c90b5c3f59e7bdba839e0a864859189f242ed9/chrome/browser/printing/pdf_to_emf_converter.h
[modify] https://crrev.com/f9c90b5c3f59e7bdba839e0a864859189f242ed9/chrome/browser/printing/print_job.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Dec 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6c35041e27b9bd82ac5b6ac56fa5814841d0829c

commit 6c35041e27b9bd82ac5b6ac56fa5814841d0829c
Author: Jay Civelli <jcivelli@chromium.org>
Date: Sat Dec 16 00:17:36 2017

Servicifying PdfToEmfConverter.

Moving the PdfToEmfConverter interface to the printing service.
As a result, simplified the client of that interface in
chrome/browser/printing/pdf_to_emf_converter.cc:
- merging PdfConverterUtilityProcessHostClient and PdfConverterImpl.
- calls on the service can now happen on the UI thread instead of the
  IO thread

Renamed the mojom::PdfToEmfConverter implementation from
PdfToEmfConverterImpl to PdfToEmfConverter as is now preferred naming
style for Mojo interface implementation.

Bug:  766451 
Change-Id: Ic748547f56bb193a558210f97558640ef8d6e44b
Reviewed-on: https://chromium-review.googlesource.com/826308
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524534}
[modify] https://crrev.com/6c35041e27b9bd82ac5b6ac56fa5814841d0829c/chrome/browser/BUILD.gn
[modify] https://crrev.com/6c35041e27b9bd82ac5b6ac56fa5814841d0829c/chrome/browser/printing/pdf_to_emf_converter.cc
[modify] https://crrev.com/6c35041e27b9bd82ac5b6ac56fa5814841d0829c/chrome/browser/printing/pdf_to_emf_converter.h
[modify] https://crrev.com/6c35041e27b9bd82ac5b6ac56fa5814841d0829c/chrome/browser/printing/print_job.cc
[modify] https://crrev.com/6c35041e27b9bd82ac5b6ac56fa5814841d0829c/chrome/common/BUILD.gn
[delete] https://crrev.com/a834c680bf3c367c4b788aec6e2f5ec6b73ce795/chrome/common/printing/BUILD.gn
[delete] https://crrev.com/a834c680bf3c367c4b788aec6e2f5ec6b73ce795/chrome/common/printing/OWNERS
[modify] https://crrev.com/6c35041e27b9bd82ac5b6ac56fa5814841d0829c/chrome/services/printing/BUILD.gn
[modify] https://crrev.com/6c35041e27b9bd82ac5b6ac56fa5814841d0829c/chrome/services/printing/manifest.json
[rename] https://crrev.com/6c35041e27b9bd82ac5b6ac56fa5814841d0829c/chrome/services/printing/pdf_to_emf_converter.cc
[rename] https://crrev.com/6c35041e27b9bd82ac5b6ac56fa5814841d0829c/chrome/services/printing/pdf_to_emf_converter.h
[add] https://crrev.com/6c35041e27b9bd82ac5b6ac56fa5814841d0829c/chrome/services/printing/pdf_to_emf_converter_factory.cc
[add] https://crrev.com/6c35041e27b9bd82ac5b6ac56fa5814841d0829c/chrome/services/printing/pdf_to_emf_converter_factory.h
[modify] https://crrev.com/6c35041e27b9bd82ac5b6ac56fa5814841d0829c/chrome/services/printing/pdf_to_pwg_raster_converter.cc
[modify] https://crrev.com/6c35041e27b9bd82ac5b6ac56fa5814841d0829c/chrome/services/printing/pdf_to_pwg_raster_converter.h
[modify] https://crrev.com/6c35041e27b9bd82ac5b6ac56fa5814841d0829c/chrome/services/printing/printing_service.cc
[modify] https://crrev.com/6c35041e27b9bd82ac5b6ac56fa5814841d0829c/chrome/services/printing/public/interfaces/BUILD.gn
[rename] https://crrev.com/6c35041e27b9bd82ac5b6ac56fa5814841d0829c/chrome/services/printing/public/interfaces/pdf_to_emf_converter.mojom
[modify] https://crrev.com/6c35041e27b9bd82ac5b6ac56fa5814841d0829c/chrome/utility/BUILD.gn
[modify] https://crrev.com/6c35041e27b9bd82ac5b6ac56fa5814841d0829c/chrome/utility/chrome_content_utility_client.cc
[delete] https://crrev.com/a834c680bf3c367c4b788aec6e2f5ec6b73ce795/chrome/utility/printing/pdf_to_emf_converter_factory_impl.cc
[delete] https://crrev.com/a834c680bf3c367c4b788aec6e2f5ec6b73ce795/chrome/utility/printing/pdf_to_emf_converter_factory_impl.h

Status: Fixed (was: Started)

Sign in to add a comment