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

Issue 814883 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Regression

Blocking:
issue 810166



Sign in to add a comment

CrOS: PDF rendering regression in v64

Project Member Reported by kbleicher@chromium.org, Feb 22 2018

Issue description

Creating a bug to track Chrome OS specific merge options as detailed in crbug/810166 #58:

Per bungeman@: 
It appears that CrOS uses its own version of FreeType, which seems to be at FreeType 2.8.1 + 107 commits at https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/master/media-libs/freetype/ . This will need cc2f3cdecff5a351e7e8961b9f2e389ab740231a cherry picked into it to pick up the fix.




 
Blocking: 810166
Labels: M-65

Comment 3 by js...@chromium.org, Feb 22 2018

Status: Started (was: Assigned)
I'll cherry-pick the above CL in the trunk and then make a merge request to 64 and 65 branches. 

After that, I'll update FT in CrOS to match other platforms in ToT. 


Comment 4 by drott@chromium.org, Feb 23 2018

When we roll FT for Chrome, we usually file a PDFium bug to keep the PDFium team in the loop. Should we CC you on those bugs, Jungshik?
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/1fc50102ad006fa0cf8d7b6ec6729d1c34e7bc21

commit 1fc50102ad006fa0cf8d7b6ec6729d1c34e7bc21
Author: Jungshik Shin <jshin@chromium.org>
Date: Fri Feb 23 11:36:41 2018

Freetype: cherry pick cc2f3cde from the upstream

Flex feature in CFF is mishandled in FreeType leading the PDF
rendering to be broken. The upstream cc2f3cde fixd it.

cc2f3cd [psaux] Correctly handle Flex features (#52846)

BUG= chromium:810166 , chromium:814883 
TEST=emerge-<target> freetype
TEST=cros deploy <test device ip> freetype
TEST=https://goo.gl/5nMa1R (PDF file) is rendered properly (bug comment 54)
Change-Id: I39092d03a6acaad9e67cf8431a7495100d65106f
Reviewed-on: https://chromium-review.googlesource.com/932754
Commit-Ready: Jungshik Shin <jshin@chromium.org>
Tested-by: Jungshik Shin <jshin@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/1fc50102ad006fa0cf8d7b6ec6729d1c34e7bc21/media-libs/freetype/freetype-2.8.1.ebuild
[add] https://crrev.com/1fc50102ad006fa0cf8d7b6ec6729d1c34e7bc21/media-libs/freetype/files/freetype-2.8.1-cc2f3cde.patch
[rename] https://crrev.com/1fc50102ad006fa0cf8d7b6ec6729d1c34e7bc21/media-libs/freetype/freetype-2.8.1-r2.ebuild

Comment 6 by js...@chromium.org, Feb 23 2018

Labels: Merge-Request-65
Requesting for merge to 65. (will be followed by request for 64). 

The CL in comment 5 must be very safe. It's been already tested and merged to 64 and 65 branches on non-CrOS platforms. 

drott@:  yeah, add me to PDFium issues. (for this issue, PDFium already has made a change,hasn't it according to thestig@ in  bug 810166 . Moreover, as he said, it'd be only for pdfium standalone.). 


Comment 7 by js...@chromium.org, Feb 23 2018

Labels: -Type-Bug Type-Bug-Regression

Comment 8 by drott@chromium.org, Feb 23 2018

> drott@:  yeah, add me to PDFium issues. 

Okay, will do.

> (for this issue, PDFium already has made a change,hasn't it according to thestig@ in  bug 810166 . Moreover, as he said, it'd be only for pdfium standalone.). 

Yes.
Project Member

Comment 9 by sheriffbot@chromium.org, Feb 23 2018

Labels: -Merge-Request-65 Merge-Review-65 Hotlist-Merge-Review
This bug requires manual review: We are only 10 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 10 by js...@chromium.org, Feb 23 2018

Labels: Merge-Request-64

Comment 11 by js...@chromium.org, Feb 23 2018

Making a merge request to 64 branch as well because we want to get this fixed asap.   

Comment 12 by js...@chromium.org, Feb 23 2018

Just realized that I didn't explicitly say that it's been tested on ToT CrOS (my own build) even though 'verified' check in the CL implied that.  

I did test and verify the fix in my own ToT build. 


Labels: -Merge-Request-64 Merge-Approved-64
Approved for M64
Project Member

Comment 14 by bugdroid1@chromium.org, Feb 23 2018

Labels: merge-merged-release-R64-10176.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/57469c78c9ceee5c0f514794ed81e3d5776fd37d

commit 57469c78c9ceee5c0f514794ed81e3d5776fd37d
Author: Jungshik Shin <jshin@chromium.org>
Date: Fri Feb 23 21:07:23 2018

[Merge to R64] Freetype: cherry pick cc2f3cde from the upstream

Flex feature in CFF is mishandled in FreeType leading the PDF
rendering to be broken. The upstream cc2f3cde fixd it.

cc2f3cd [psaux] Correctly handle Flex features (#52846)

BUG= chromium:810166 , chromium:814883 
TEST=emerge-<target> freetype
TEST=cros deploy <test device ip> freetype
TEST=https://goo.gl/5nMa1R (PDF file) is rendered properly (bug comment 54)
Change-Id: I39092d03a6acaad9e67cf8431a7495100d65106f
Previous-Reviewed-on: https://chromium-review.googlesource.com/932754
(cherry picked from commit 701e1fbcec5bfa30d08a6b9f3c6f78085e44b685)
TBR=kbleicher@chromium.org,derat@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/935142
Reviewed-by: Jungshik Shin <jshin@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Commit-Queue: Jungshik Shin <jshin@chromium.org>
Tested-by: Jungshik Shin <jshin@chromium.org>

[modify] https://crrev.com/57469c78c9ceee5c0f514794ed81e3d5776fd37d/media-libs/freetype/freetype-2.8.1.ebuild
[add] https://crrev.com/57469c78c9ceee5c0f514794ed81e3d5776fd37d/media-libs/freetype/files/freetype-2.8.1-cc2f3cde.patch
[rename] https://crrev.com/57469c78c9ceee5c0f514794ed81e3d5776fd37d/media-libs/freetype/freetype-2.8.1-r2.ebuild

The following PDF file is displayed properly on 66.0.3353.0/10433.0.0-Kip

https://services.hbsp.harvard.edu/services/proxy/content/72850847/72850892/3a487eee07f1ef3fdc12cf13ab9634f3

Will verify the fix on M64/M65 once the build is available. 
Thanks!   Assume we can remove the merge-approved-64 label now that the merge was made?

Comment 17 by js...@chromium.org, Feb 25 2018

Labels: -Merge-Approved-64
Removing merge-approved-64 label. 

Waiting for merge approval for R65. 

Thanks, Song, for verifying the fix in 66. 
Status: Fixed (was: Started)
Marking this as 'fixed to unblock M64 testing.
Labels: -Hotlist-Merge-Review -Merge-Review-65 Merge-Approved-65
Project Member

Comment 21 by bugdroid1@chromium.org, Feb 27 2018

Labels: merge-merged-release-R65-10323.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/d7412c421c0b42d0b3868d1e1c5caf95c49331da

commit d7412c421c0b42d0b3868d1e1c5caf95c49331da
Author: Jungshik Shin <jshin@chromium.org>
Date: Tue Feb 27 21:24:42 2018

[R65-branch] Freetype: cherry pick cc2f3cde from the upstream

Flex feature in CFF is mishandled in FreeType leading the PDF
rendering to be broken. The upstream cc2f3cde fixd it.

cc2f3cd [psaux] Correctly handle Flex features (#52846)

BUG= chromium:810166 , chromium:814883 
TEST=emerge-<target> freetype
TEST=cros deploy <test device ip> freetype
TEST=https://goo.gl/5nMa1R (PDF file) is rendered properly (bug comment 54)
Change-Id: I39092d03a6acaad9e67cf8431a7495100d65106f
Previous-Reviewed-on: https://chromium-review.googlesource.com/932754
(cherry picked from commit 6e31ac40e78857887b0ebd949934b6dae3c7fdf3)
Reviewed-on: https://chromium-review.googlesource.com/939992
Commit-Queue: Jungshik Shin <jshin@chromium.org>
Tested-by: Jungshik Shin <jshin@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/d7412c421c0b42d0b3868d1e1c5caf95c49331da/media-libs/freetype/freetype-2.8.1.ebuild
[add] https://crrev.com/d7412c421c0b42d0b3868d1e1c5caf95c49331da/media-libs/freetype/files/freetype-2.8.1-cc2f3cde.patch
[rename] https://crrev.com/d7412c421c0b42d0b3868d1e1c5caf95c49331da/media-libs/freetype/freetype-2.8.1-r2.ebuild

Comment 22 by js...@chromium.org, Feb 28 2018

Labels: -Merge-Approved-65
Merged to R65 as well. Thanks all. 

 bug 810791  is not related to this issue. It was reported before this fix went in to R64 (see  bug 810791  comment 52 :  crbug.com/810791#c52  ).  The root cause of that bug is  bug 813101  (Cloud Printing Proxy's mishandling of glyph or font cache). 

Sign in to add a comment