Bug 211389 – Remove HAVE(IOSURFACE) checks in Cocoa-platform-specific code

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Darin Adler]

Remove HAVE(IOSURFACE) checks in Cocoa-platform-specific code

[Darin Adler]

Created attachment 398401 [details]
Patch

[Darin Adler]

Created attachment 398403 [details]
Patch

[Anders Carlsson]

(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).

[Darin Adler]

The relevant quote from PlatformHave.h is:

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

I was going based on that. Not changing anything.

[Alexey Proskuryakov]

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?

[Tim Horton]

(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.

[Tim Horton]

(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.

[Darin Adler]

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).

[Darin Adler]

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

Thanks, autocorrect!

[Darin Adler]

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

[Radar WebKit Bug Importer]

<rdar://problem/62870016>