Created attachment 398401 [details]
Patch

Created attachment 398403 [details]
Patch

(In reply to Darin Adler from comment #0)
> Remove HAVE(IOSURFACE) checks in Cocoa-platform-specific code

Is this true even for the simulator nowadays? (I'm assuming yes since ios-sim is green).

The relevant quote from PlatformHave.h is:

#if PLATFORM(COCOA)
#define HAVE_IOSURFACE 1
#endif

I was going based on that. Not changing anything.

Comment on attachment 398403 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=398403&action=review

> Source/WebKit/ChangeLog:35
> +        Merged #if expressions into a single one.

I went back to ChangeLog to understand why. Since this is in Cocoa directory, when is HAVE(IOSURFACE) false here?

> Tools/DumpRenderTree/ios/PixelDumpSupportIOS.mm:-58
> -    // TODO: <rdar://problem/6558366> DumpRenderTree: Investigate testRepaintSweepHorizontally and dumpSelectionRect
> -    BEGIN_BLOCK_OBJC_EXCEPTIONS;

I'm not sure if/how the TODO is related to BEGIN_BLOCK_OBJC_EXCEPTIONS. In fact, they were added at different times. Perhaps it should stay?

(In reply to Alexey Proskuryakov from comment #5)
> Comment on attachment 398403 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=398403&action=review
> 
> > Source/WebKit/ChangeLog:35
> > +        Merged #if expressions into a single one.
> 
> I went back to ChangeLog to understand why. Since this is in Cocoa
> directory, when is HAVE(IOSURFACE) false here?

Used to be false in the Sim, right? But no longer.

(In reply to Tim Horton from comment #6)
> (In reply to Alexey Proskuryakov from comment #5)
> > Comment on attachment 398403 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=398403&action=review
> > 
> > > Source/WebKit/ChangeLog:35
> > > +        Merged #if expressions into a single one.
> > 
> > I went back to ChangeLog to understand why. Since this is in Cocoa
> > directory, when is HAVE(IOSURFACE) false here?
> 
> Used to be false in the Sim, right? But no longer.

Oh, undersea already said that.

Comment on attachment 398403 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=398403&action=review

>>>> Source/WebKit/ChangeLog:35
>>>> +        Merged #if expressions into a single one.
>>> 
>>> I went back to ChangeLog to understand why. Since this is in Cocoa directory, when is HAVE(IOSURFACE) false here?
>> 
>> Used to be false in the Sim, right? But no longer.
> 
> Oh, undersea already said that.

OK, will go further on this.

>> Tools/DumpRenderTree/ios/PixelDumpSupportIOS.mm:-58
>> -    BEGIN_BLOCK_OBJC_EXCEPTIONS;
> 
> I'm not sure if/how the TODO is related to BEGIN_BLOCK_OBJC_EXCEPTIONS. In fact, they were added at different times. Perhaps it should stay?

Yes, they are not related.

I removed the two for two different reasons, and forgot to mention one in the change log.

I removed the "TODO" comment because there is no real reason to have a comment in the code. It’s OK to have a bug about perhaps implementing this some day, but I don’t think the comment in the code helps enough to be worth it. The real driver is the skipped tests, not the comment in the code. Also, these are features of pixel tests. I’m thinking that pixel tests of legacy WebKit are not a very high priority for future enhancement.

If you don’t agree, I would be happy to bring the TODO back (although I will say FIXME instead).

(In reply to Tim Horton from comment #7)
> Oh, undersea already said that.

Thanks, autocorrect!

Committed r261133: <https://trac.webkit.org/changeset/261133>

<rdar://problem/62870016>