Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
694213002:
Remove calls to deprecated SkPDFDevice and SkPDFDocuemnt. (Closed)
|
Created:
6 years, 1 month ago by hal.canary Modified:
5 years, 11 months ago CC:
chromium-reviews, djsollen Base URL:
https://chromium.googlesource.com/chromium/src.git@master Project:
chromium Visibility:
Public. |
DescriptionRemove calls to deprecated SkPDFDevice and SkPDFDocuemnt.
In the PdfMetafileSkia class, Instead of storing a
SkPDFDocument, store a vector of pages as
SkPictures. This allows access to individual at any
time. When FinishDocument() is called, use the
SkDocument API to print all pages to PDF.
BUG=278148
Committed: https://crrev.com/816b7105b078f7300f825cd81f379e312efdb821
Cr-Commit-Position: refs/heads/master@{#304379}
Patch Set 1 : . #
Total comments: 37
Patch Set 2 : code duplication #
Total comments: 6
Patch Set 3 : comments from Vitaly #
Total comments: 14
Patch Set 4 : rebase again, respond Vitaly, remove build/gyp changes, git cl format #Messages
Total messages: 47 (23 generated)
Patchset #1 (id:1) has been deleted
The CQ bit was checked by halcanary@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/694213002/20001
The CQ bit was unchecked by commit-bot@chromium.org
No LGTM from a valid reviewer yet. Only full committers are accepted. Even if an LGTM may have been provided, it was from a non-committer, _not_ a full super star committer. See http://www.chromium.org/getting-involved/become-a-committer Note that this has nothing to do with OWNERS files.
halcanary@google.com changed reviewers: + reed@google.com, robertphillips@google.com, vitalybuka@chromium.org
please take a look.
https://codereview.chromium.org/694213002/diff/100001/chrome/renderer/printin... File chrome/renderer/printing/print_web_view_helper_linux.cc (right): https://codereview.chromium.org/694213002/diff/100001/chrome/renderer/printin... chrome/renderer/printing/print_web_view_helper_linux.cc:159: GetPageSizeAndContentAreaFromPageLayout(page_layout_in_points, &page_size, Similar code in and _win.cc, _mac.mm and android_webview/renderer/print_web_view_helper_linux.cc
Is any bug for this? https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... File printing/pdf_metafile_skia.cc (right): https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:100: const float& scale_factor) { Why not just kill this code. https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:106: return SkSize::Make(SkIntToScalar(size.width()), to_sk_size -> ToSkSize https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:107: SkIntToScalar(size.height())); please put in namespace {} in front pf namespace printing { and remove inline, usually we don't use them https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:110: inline SkRect to_sk_rect(const gfx::Rect& rect) { fix name https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:113: SkIntToScalar(rect.bottom())); same https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:119: if (data_->PageOutstanding()) { no need {} https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:130: if (NULL == recordingCanvas) { if (!recordingCanvas) { https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:138: if (!data_->PageOutstanding()) return false; please move returns on separate line also please run "git cl format" https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:139: DCHECK(data_->pages_.back().content_.get() == NULL); DCHECK_EQ https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:148: if (data_->HasData()) data_->pdf_data_.clear(); // free us RAM first/ put if body on the separate line https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:152: // std::vector<skia::RefPtr<SkPicture>> pages_; remove comment https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:164: pdf_doc.clear(); why pdf_doc.clear(); is needed? https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:171: if (data_->pdf_data_.get() == NULL) return 0; RefPtr has bool operator, so please just put if (!data_->pdf_data_) https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:175: inline bool copy_asset(SkStreamAsset* asset, void* dst, size_t size) { copy_asset -> CopyAsset https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:176: DCHECK(asset); move all utilities into namespace {} and remove inlines https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:297: static const Page* get_page(PdfMetafileSkiaData* input_data, remove static and move into namespace {} https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... File printing/pdf_metafile_skia.h (right): https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.h:42: // Deprecated. Please use GetVectorCanvasForNewPage(). I guess we are going to remove that soon after rolling skia. Could you please put TODO here with bug number?
https://codereview.chromium.org/694213002/diff/120001/chrome/renderer/printin... File chrome/renderer/printing/print_web_view_helper_linux.cc (right): https://codereview.chromium.org/694213002/diff/120001/chrome/renderer/printin... chrome/renderer/printing/print_web_view_helper_linux.cc:185: canvas.clear(); can you make "clear" use consistent? also webview version has neither clear nor {} https://codereview.chromium.org/694213002/diff/120001/chrome/renderer/printin... File chrome/renderer/printing/print_web_view_helper_pdf_win.cc (right): https://codereview.chromium.org/694213002/diff/120001/chrome/renderer/printin... chrome/renderer/printing/print_web_view_helper_pdf_win.cc:183: if (!canvas.get()) { Please try to keep style of the file consistent, don't put {} for one liners here
BTW this patch created with @google.com account not chromium.org one.
+1 for removing the extra (unneeded) class https://codereview.chromium.org/694213002/diff/120001/chrome/renderer/printin... File chrome/renderer/printing/print_web_view_helper_linux.cc (right): https://codereview.chromium.org/694213002/diff/120001/chrome/renderer/printin... chrome/renderer/printing/print_web_view_helper_linux.cc:185: canvas.clear(); On 2014/11/03 20:03:29, Vitaly Buka wrote: > can you make "clear" use consistent? What is meant by this?
https://codereview.chromium.org/694213002/diff/120001/chrome/renderer/printin... File chrome/renderer/printing/print_web_view_helper_linux.cc (right): https://codereview.chromium.org/694213002/diff/120001/chrome/renderer/printin... chrome/renderer/printing/print_web_view_helper_linux.cc:185: canvas.clear(); On 2014/11/03 20:29:33, reed1 wrote: > On 2014/11/03 20:03:29, Vitaly Buka wrote: > > can you make "clear" use consistent? > > What is meant by this? here canvas.clear(); is used to reset canvas before metafile->FinishPage() android_webview/renderer/print_web_view_helper_linux.cc does not reset canvas at all mac and win use {} to reset canvas before metafile->FinishPage() I'd prefer to use {} everywhere.
On 2014/11/03 20:50:11, Vitaly Buka wrote: > https://codereview.chromium.org/694213002/diff/120001/chrome/renderer/printin... > File chrome/renderer/printing/print_web_view_helper_linux.cc (right): > > https://codereview.chromium.org/694213002/diff/120001/chrome/renderer/printin... > chrome/renderer/printing/print_web_view_helper_linux.cc:185: canvas.clear(); > On 2014/11/03 20:29:33, reed1 wrote: > > On 2014/11/03 20:03:29, Vitaly Buka wrote: > > > can you make "clear" use consistent? > > > > What is meant by this? > > here canvas.clear(); is used to reset canvas before metafile->FinishPage() > android_webview/renderer/print_web_view_helper_linux.cc does not reset canvas at > all > mac and win use {} to reset canvas before metafile->FinishPage() > > I'd prefer to use {} everywhere. {} meaning have canvas go out of scope?
On 2014/11/03 20:56:48, reed1 wrote: > On 2014/11/03 20:50:11, Vitaly Buka wrote: > > > https://codereview.chromium.org/694213002/diff/120001/chrome/renderer/printin... > > File chrome/renderer/printing/print_web_view_helper_linux.cc (right): > > > > > https://codereview.chromium.org/694213002/diff/120001/chrome/renderer/printin... > > chrome/renderer/printing/print_web_view_helper_linux.cc:185: canvas.clear(); > > On 2014/11/03 20:29:33, reed1 wrote: > > > On 2014/11/03 20:03:29, Vitaly Buka wrote: > > > > can you make "clear" use consistent? > > > > > > What is meant by this? > > > > here canvas.clear(); is used to reset canvas before metafile->FinishPage() > > android_webview/renderer/print_web_view_helper_linux.cc does not reset canvas > at > > all > > mac and win use {} to reset canvas before metafile->FinishPage() > > > > I'd prefer to use {} everywhere. > > {} meaning have canvas go out of scope? Yes, I guess we need to delete VectorCanvas before calling metafile->FinishPage()
https://codereview.chromium.org/694213002/diff/100001/chrome/renderer/printin... File chrome/renderer/printing/print_web_view_helper_linux.cc (right): https://codereview.chromium.org/694213002/diff/100001/chrome/renderer/printin... chrome/renderer/printing/print_web_view_helper_linux.cc:159: GetPageSizeAndContentAreaFromPageLayout(page_layout_in_points, &page_size, On 2014/11/03 17:57:24, Vitaly Buka wrote: > Similar code in and _win.cc, _mac.mm and > android_webview/renderer/print_web_view_helper_linux.cc Done. Maybe a future CL can factor out the similarities. https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... File printing/pdf_metafile_skia.cc (right): https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:100: const float& scale_factor) { On 2014/11/03 19:57:02, Vitaly Buka wrote: > Why not just kill this code. The Metafile interface requires it. https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:106: return SkSize::Make(SkIntToScalar(size.width()), On 2014/11/03 19:57:02, Vitaly Buka wrote: > to_sk_size -> ToSkSize Done. https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:107: SkIntToScalar(size.height())); On 2014/11/03 19:57:02, Vitaly Buka wrote: > please put in namespace {} in front pf namespace printing { > and remove inline, usually we don't use them Done. https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:110: inline SkRect to_sk_rect(const gfx::Rect& rect) { On 2014/11/03 19:57:02, Vitaly Buka wrote: > fix name Done. https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:113: SkIntToScalar(rect.bottom())); On 2014/11/03 19:57:02, Vitaly Buka wrote: > same Done. https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:119: if (data_->PageOutstanding()) { On 2014/11/03 19:57:02, Vitaly Buka wrote: > no need {} Done. https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:130: if (NULL == recordingCanvas) { On 2014/11/03 19:57:02, Vitaly Buka wrote: > if (!recordingCanvas) { Done. https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:138: if (!data_->PageOutstanding()) return false; On 2014/11/03 19:57:02, Vitaly Buka wrote: > please move returns on separate line > > also please run "git cl format" Done. https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:139: DCHECK(data_->pages_.back().content_.get() == NULL); On 2014/11/03 19:57:02, Vitaly Buka wrote: > DCHECK_EQ Done. https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:148: if (data_->HasData()) data_->pdf_data_.clear(); // free us RAM first/ On 2014/11/03 19:57:02, Vitaly Buka wrote: > put if body on the separate line Done. https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:152: // std::vector<skia::RefPtr<SkPicture>> pages_; On 2014/11/03 19:57:02, Vitaly Buka wrote: > remove comment Done. https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:164: pdf_doc.clear(); On 2014/11/03 19:57:02, Vitaly Buka wrote: > why pdf_doc.clear(); is needed? removed https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:171: if (data_->pdf_data_.get() == NULL) return 0; On 2014/11/03 19:57:02, Vitaly Buka wrote: > RefPtr has bool operator, so please just put if (!data_->pdf_data_) Done. https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:175: inline bool copy_asset(SkStreamAsset* asset, void* dst, size_t size) { On 2014/11/03 19:57:02, Vitaly Buka wrote: > copy_asset -> CopyAsset Done. https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:176: DCHECK(asset); On 2014/11/03 19:57:02, Vitaly Buka wrote: > move all utilities into namespace {} and remove inlines Done. https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:297: static const Page* get_page(PdfMetafileSkiaData* input_data, On 2014/11/03 19:57:02, Vitaly Buka wrote: > remove static and move into namespace {} I made it a method of PdfMetafileSkiaData. https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... File printing/pdf_metafile_skia.h (right): https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.h:42: // Deprecated. Please use GetVectorCanvasForNewPage(). On 2014/11/03 19:57:02, Vitaly Buka wrote: > I guess we are going to remove that soon after rolling skia. > Could you please put TODO here with bug number? Done. https://codereview.chromium.org/694213002/diff/120001/chrome/renderer/printin... File chrome/renderer/printing/print_web_view_helper_linux.cc (right): https://codereview.chromium.org/694213002/diff/120001/chrome/renderer/printin... chrome/renderer/printing/print_web_view_helper_linux.cc:185: canvas.clear(); On 2014/11/03 20:03:29, Vitaly Buka wrote: > can you make "clear" use consistent? > also webview version has neither clear nor {} Done. https://codereview.chromium.org/694213002/diff/120001/chrome/renderer/printin... File chrome/renderer/printing/print_web_view_helper_pdf_win.cc (right): https://codereview.chromium.org/694213002/diff/120001/chrome/renderer/printin... chrome/renderer/printing/print_web_view_helper_pdf_win.cc:183: if (!canvas.get()) { On 2014/11/03 20:03:29, Vitaly Buka wrote: > Please try to keep style of the file consistent, don't put {} for one liners > here Done.
Vitaly, It sounds like you have no problem with the major changes of this CL. This will allow us to refactor SkPDFDevice and SkPDFDocument for some much-needed improvements. I'm sorry about the formatting issues, most of them are my fault since I don't do many big Chromium CLs , some of them were inserted by clang-format. Thanks for getting back to me so quickly.
On 2014/11/04 15:33:49, Hal Canary wrote: > Vitaly, > > It sounds like you have no problem with the major changes of this CL. This will I don't have problems if it works. Please let me know if you expect any issues from that. Could you delay landing till next week, after branch m40? > allow us to refactor SkPDFDevice and SkPDFDocument for some much-needed > improvements. I'm sorry about the formatting issues, most of them are my fault > since I don't do many big Chromium CLs , some of them were inserted by > clang-format. Thanks for getting back to me so quickly. Probably you have used wrong clang format profile. Please just run clang format as "git cl format" and feel free to keep all changes it made.
please run "git cl format" and lgtm with few other comments https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... File printing/pdf_metafile_skia.cc (right): https://codereview.chromium.org/694213002/diff/100001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:100: const float& scale_factor) { On 2014/11/04 15:30:35, Hal Canary wrote: > On 2014/11/03 19:57:02, Vitaly Buka wrote: > > Why not just kill this code. > > The Metafile interface requires it. https://codereview.chromium.org/697373004/ https://codereview.chromium.org/694213002/diff/160001/printing/metafile.cc File printing/metafile.cc (right): https://codereview.chromium.org/694213002/diff/160001/printing/metafile.cc#ne... printing/metafile.cc:30: this->StartPageForVectorCanvas(page_size, content_area, scale_factor); don't need this-> https://codereview.chromium.org/694213002/diff/160001/printing/metafile.h File printing/metafile.h (right): https://codereview.chromium.org/694213002/diff/160001/printing/metafile.h#new... printing/metafile.h:118: // StartPage or NULL on error. The default implementation calls Let's remove them from Metafile. https://codereview.chromium.org/697373004/ https://codereview.chromium.org/694213002/diff/160001/printing/pdf_metafile_s... File printing/pdf_metafile_skia.cc (right): https://codereview.chromium.org/694213002/diff/160001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:88: skia::RefPtr<SkStreamAsset> pdf_data_; I guess order shouldbe be: 1. Methods 2. Members https://codereview.chromium.org/694213002/diff/160001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:206: &(page.content_area_)); just &page.content_area_ https://codereview.chromium.org/694213002/diff/160001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:273: scoped_ptr<uint8_t[]> buffer(new uint8_t[length]); just std::vector<uint8_t> buffer(length)? https://codereview.chromium.org/694213002/diff/160001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:290: char buffer[4096]; maybe std::vector for 1Mb? https://codereview.chromium.org/694213002/diff/160001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:292: size_t read_size = asset->read(buffer, sizeof(buffer)); read_size == 0 ? https://codereview.chromium.org/694213002/diff/160001/printing/pdf_metafile_s... File printing/pdf_metafile_skia.h (right): https://codereview.chromium.org/694213002/diff/160001/printing/pdf_metafile_s... printing/pdf_metafile_skia.h:44: SkBaseDevice* StartPageForVectorCanvas(const gfx::Size& page_size, Please just remove StartPageForVectorCanvas. There is no point to keep deprecated unused method.
https://codereview.chromium.org/694213002/diff/160001/printing/metafile.h File printing/metafile.h (right): https://codereview.chromium.org/694213002/diff/160001/printing/metafile.h#new... printing/metafile.h:118: // StartPage or NULL on error. The default implementation calls On 2014/11/04 19:23:55, Vitaly Buka wrote: > Let's remove them from Metafile. > https://codereview.chromium.org/697373004/ Sounds good. I'll rebase this CL on top of yours. I might even break this CL into two CLs: 1. PdfMetafileSkia returns a SkCanvas* ptr instead of a SkBaseDevice*. All of the call sites are modified to accept that instead. 2. Switch from SkPDFDocument/SkPDFDevice to SkDocument.
On 2014/11/05 14:31:33, Hal Canary wrote: > https://codereview.chromium.org/694213002/diff/160001/printing/metafile.h > File printing/metafile.h (right): > > https://codereview.chromium.org/694213002/diff/160001/printing/metafile.h#new... > printing/metafile.h:118: // StartPage or NULL on error. The default > implementation calls > On 2014/11/04 19:23:55, Vitaly Buka wrote: > > Let's remove them from Metafile. > > https://codereview.chromium.org/697373004/ > > Sounds good. I'll rebase this CL on top of yours. I might even break this CL > into two CLs: > > 1. PdfMetafileSkia returns a SkCanvas* ptr instead of a SkBaseDevice*. All of > the call sites are modified to accept that instead. > > 2. Switch from SkPDFDocument/SkPDFDevice to SkDocument. sgtm
Patchset #13 (id:260001) has been deleted
Patchset #10 (id:200001) has been deleted
Patchset #11 (id:240001) has been deleted
Patchset #10 (id:220001) has been deleted
Patchset #9 (id:180001) has been deleted
Patchset #7 (id:140001) has been deleted
Patchset #4 (id:80001) has been deleted
Patchset #3 (id:60001) has been deleted
Patchset #2 (id:40001) has been deleted
Patchset #1 (id:20001) has been deleted
Patchset #5 (id:290001) has been deleted
Patchset #5 (id:310001) has been deleted
Patchset #5 (id:330001) has been deleted
https://codereview.chromium.org/694213002/diff/160001/printing/pdf_metafile_s... File printing/pdf_metafile_skia.cc (right): https://codereview.chromium.org/694213002/diff/160001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:88: skia::RefPtr<SkStreamAsset> pdf_data_; On 2014/11/04 19:23:55, Vitaly Buka wrote: > I guess order shouldbe be: > 1. Methods > 2. Members Acknowledged. https://codereview.chromium.org/694213002/diff/160001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:206: &(page.content_area_)); On 2014/11/04 19:23:55, Vitaly Buka wrote: > just &page.content_area_ Done. https://codereview.chromium.org/694213002/diff/160001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:273: scoped_ptr<uint8_t[]> buffer(new uint8_t[length]); On 2014/11/04 19:23:55, Vitaly Buka wrote: > just std::vector<uint8_t> buffer(length)? Done. https://codereview.chromium.org/694213002/diff/160001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:290: char buffer[4096]; On 2014/11/04 19:23:55, Vitaly Buka wrote: > maybe std::vector for 1Mb? Done. https://codereview.chromium.org/694213002/diff/160001/printing/pdf_metafile_s... printing/pdf_metafile_skia.cc:292: size_t read_size = asset->read(buffer, sizeof(buffer)); On 2014/11/04 19:23:55, Vitaly Buka wrote: > read_size == 0 ? Done.
The CQ bit was checked by halcanary@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/694213002/350001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_compile_dbg on tryserver.chromium.mac (http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
The CQ bit was checked by halcanary@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/694213002/370001
Patchset #5 (id:350001) has been deleted
Patchset #4 (id:270001) has been deleted
Message was sent while issue was closed.
Committed patchset #6 (id:370001)
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/816b7105b078f7300f825cd81f379e312efdb821 Cr-Commit-Position: refs/heads/master@{#304379}
Message was sent while issue was closed.
A revert of this CL (patchset #4 id:370001) has been created in https://codereview.chromium.org/731143004/ by vitalybuka@chromium.org. The reason for reverting is: Breaks printing. BUG=434079.
halcanary@google.com changed reviewers: - robertphillips@google.com |
