Bug 149898 - Revert r187626 (and r188025) as it caused a PLT regression
Summary: Revert r187626 (and r188025) as it caused a PLT regression
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on: 149937
Blocks: 147457
  Show dependency tree / graph
 
Reported: 2015-10-07 14:44 PDT by Chris Dumez
Modified: 2015-10-08 15:54 PDT (History)
4 users (show)

See Also:

Attachments
Patch (2.36 KB, patch)
2015-10-07 18:35 PDT, Chris Dumez 		no flags Details | Formatted Diff | Diff
[WIP] Full roll out (17.69 KB, patch)
2015-10-08 15:12 PDT, Chris Dumez 		no flags Details | Formatted Diff | Diff
Patch (20.77 KB, patch)
2015-10-08 15:49 PDT, Chris Dumez 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2015-10-07 14:44:05 PDT 
Roll out r187626 as it caused a PLT regression
Comment 1 Chris Dumez 2015-10-07 14:44:23 PDT 
rdar://problem/22657123
Comment 2 Chris Dumez 2015-10-07 14:55:19 PDT 
Looks like I may have to roll out <http://trac.webkit.org/changeset/188025> as well.
Comment 3 Myles C. Maxfield 2015-10-07 15:59:56 PDT 
You shouldn't need to roll out the stuff that depends on this. Instead, you can just mark the tests as expected to fail.

Instead, first try to revert only the changes in this file http://trac.webkit.org/changeset/187626/trunk/Source/WebCore/platform/graphics/FontCache.h

That will probably be effective, and is much smaller / simpler than trying to revert the patch and everything on top of it.
Comment 4 Chris Dumez 2015-10-07 16:25:20 PDT 
(In reply to comment #3)
> You shouldn't need to roll out the stuff that depends on this. Instead, you
> can just mark the tests as expected to fail.
> 
> Instead, first try to revert only the changes in this file
> http://trac.webkit.org/changeset/187626/trunk/Source/WebCore/platform/
> graphics/FontCache.h
> 
> That will probably be effective, and is much smaller / simpler than trying
> to revert the patch and everything on top of it.

Ok, I have a full revert patch but I'll try the partial revert first.
Comment 5 Chris Dumez 2015-10-07 18:35:39 PDT 
Created attachment 262665 [details]
Patch
Comment 6 Myles C. Maxfield 2015-10-07 18:36:38 PDT 
Comment on attachment 262665 [details]
Patch

This will likely cause tests to fail.
Comment 7 Chris Dumez 2015-10-07 18:48:00 PDT 
(In reply to comment #6)
> Comment on attachment 262665 [details]
> Patch
> 
> This will likely cause tests to fail.

I ran all the tests locally without problem but EWS will confirm.
Comment 8 Myles C. Maxfield 2015-10-07 19:05:53 PDT 
It's a little concerning that they all pass...
Comment 9 Chris Dumez 2015-10-07 19:07:07 PDT 
Comment on attachment 262665 [details]
Patch

Clearing flags on attachment: 262665

Committed r190701: <http://trac.webkit.org/changeset/190701>
Comment 10 Chris Dumez 2015-10-07 19:07:14 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 11 WebKit Commit Bot 2015-10-08 14:59:31 PDT 
Re-opened since this is blocked by bug 149937
Comment 12 Chris Dumez 2015-10-08 15:12:22 PDT 
Created attachment 262720 [details]
[WIP] Full roll out

WIP patch in case Myles has initial feedback. I haven't run the layout tests yet.
Comment 13 Myles C. Maxfield 2015-10-08 15:30:45 PDT 
(In reply to comment #12)
> Created attachment 262720 [details]
> [WIP] Full roll out
> 
> WIP patch in case Myles has initial feedback. I haven't run the layout tests
> yet.

Which specific revisions does this roll out?
Comment 14 Chris Dumez 2015-10-08 15:32:13 PDT 
(In reply to comment #13)
> (In reply to comment #12)
> > Created attachment 262720 [details]
> > [WIP] Full roll out
> > 
> > WIP patch in case Myles has initial feedback. I haven't run the layout tests
> > yet.
> 
> Which specific revisions does this roll out?

The ones in the bug title :)
Comment 15 Chris Dumez 2015-10-08 15:49:19 PDT 
Created attachment 262723 [details]
Patch
Comment 16 Chris Dumez 2015-10-08 15:54:21 PDT 
Comment on attachment 262723 [details]
Patch

Clearing flags on attachment: 262723

Committed r190754: <http://trac.webkit.org/changeset/190754>
Comment 17 Chris Dumez 2015-10-08 15:54:28 PDT 
All reviewed patches have been landed.  Closing bug.