Bug 149898

Summary:

Revert r187626 (and r188025) as it caused a PLT regression

Product:

WebKit

Reporter:

Chris Dumez <cdumez>

Component:

Page Loading

Assignee:

Chris Dumez <cdumez>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

barraclough, commit-queue, mmaxfield, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

149937

Bug Blocks:

147457

Attachments:

Description	Flags
Patch	none
[WIP] Full roll out	none
Patch	none

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.