Bug 127412 – [iOS] Upstream changes in Tools/DumpRenderTree

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Andy Estes]

Multiple patches forthcoming.

[Andy Estes]

Created attachment 221924 [details]
[iOS] Prepare for upstreaming DumpRenderTree changes

[Andy Estes]

Committed r162573: <http://trac.webkit.org/changeset/162573>

[Andy Estes]

Created attachment 222124 [details]
[iOS] Upstream changes in Tools/DumpRenderTree

[Simon Fraser (smfr)]

Comment on attachment 222124 [details]
[iOS] Upstream changes in Tools/DumpRenderTree

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

> Tools/DumpRenderTree/AccessibilityTextMarker.h:61
> +#if PLATFORM(MAC) && SUPPORTS_AX_TEXTMARKERS

Don't think you need the PLATFORM(MAC) here any more.

> Tools/DumpRenderTree/AccessibilityTextMarker.h:81
> +#if PLATFORM(MAC) && SUPPORTS_AX_TEXTMARKERS

Ditto.

> Tools/DumpRenderTree/ios/Info.plist:12
> +	<string>com.apple.DumpRenderTree</string>

org.webkit?

> Tools/DumpRenderTree/ios/PerlSupport/IPhoneSimulatorNotification/Makefile.PL:1
> +# Copyright (C) 2009 Apple Inc. All rights reserved.

2014?

> Tools/DumpRenderTree/ios/PerlSupport/IPhoneSimulatorNotification/lib/IPhoneSimulatorNotification.pm:1
> +# Copyright (C) 2009 Apple Inc. All rights reserved.

2014?

> Tools/DumpRenderTree/ios/PerlSupport/Makefile:1
> +# Copyright (C) 2009 Apple Inc. All rights reserved.

2014

> Tools/DumpRenderTree/ios/PixelDumpSupportIOS.mm:2
> + * Copyright (C) 2005, 2006, 2007, 2009 Apple Inc. All rights reserved.

2014

> Tools/DumpRenderTree/mac/DumpRenderTree.mm:762
> +    // See also -[TabDocument _updateTextSize] in MobileSafari/TabDocument.m

Remove this comment.

> Tools/DumpRenderTree/mac/EventSendingController.mm:423
> +#if !PLATFORM(IOS)
>          flags |= NSControlKeyMask;
> +#else
> +        flags |= WebEventFlagMaskControl;
> +#endif

These are ugly. Can we just make some common #ifdefs or an enum?

> Tools/DumpRenderTree/mac/ObjCPlugin.m:133
> +#if PLATFORM(IOS)
> +    return NSStringFromClass([obj class]);
> +#else
>      return [obj className];
> +#endif

No idea why this is different.

[Build Bot]

Comment on attachment 222124 [details]
[iOS] Upstream changes in Tools/DumpRenderTree

Attachment 222124 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6287300484923392

New failing tests:
http/tests/misc/link-rel-icon-beforeload.html

[Build Bot]

Created attachment 222139 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Andy Estes]

(In reply to comment #4)
> (From update of attachment 222124 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=222124&action=review
> 
> > Tools/DumpRenderTree/AccessibilityTextMarker.h:61
> > +#if PLATFORM(MAC) && SUPPORTS_AX_TEXTMARKERS
> 
> Don't think you need the PLATFORM(MAC) here any more.

Will change.

> 
> > Tools/DumpRenderTree/AccessibilityTextMarker.h:81
> > +#if PLATFORM(MAC) && SUPPORTS_AX_TEXTMARKERS
> 
> Ditto.
> 
> > Tools/DumpRenderTree/ios/Info.plist:12
> > +	<string>com.apple.DumpRenderTree</string>
> 
> org.webkit?

Sure.

> 
> > Tools/DumpRenderTree/ios/PerlSupport/IPhoneSimulatorNotification/Makefile.PL:1
> > +# Copyright (C) 2009 Apple Inc. All rights reserved.
> 
> 2014?

I think 2009 is probably correct. This code was in open source, then removed, then added back (albeit to a different directory) unchanged.

> 
> > Tools/DumpRenderTree/ios/PerlSupport/IPhoneSimulatorNotification/lib/IPhoneSimulatorNotification.pm:1
> > +# Copyright (C) 2009 Apple Inc. All rights reserved.
> 
> 2014?
> 
> > Tools/DumpRenderTree/ios/PerlSupport/Makefile:1
> > +# Copyright (C) 2009 Apple Inc. All rights reserved.
> 
> 2014
> 
> > Tools/DumpRenderTree/ios/PixelDumpSupportIOS.mm:2
> > + * Copyright (C) 2005, 2006, 2007, 2009 Apple Inc. All rights reserved.
> 
> 2014
> 
> > Tools/DumpRenderTree/mac/DumpRenderTree.mm:762
> > +    // See also -[TabDocument _updateTextSize] in MobileSafari/TabDocument.m
> 
> Remove this comment.

Ok.

> 
> > Tools/DumpRenderTree/mac/EventSendingController.mm:423
> > +#if !PLATFORM(IOS)
> >          flags |= NSControlKeyMask;
> > +#else
> > +        flags |= WebEventFlagMaskControl;
> > +#endif
> 
> These are ugly. Can we just make some common #ifdefs or an enum?

Probably. I'll look into it.

> 
> > Tools/DumpRenderTree/mac/ObjCPlugin.m:133
> > +#if PLATFORM(IOS)
> > +    return NSStringFromClass([obj class]);
> > +#else
> >      return [obj className];
> > +#endif
> 
> No idea why this is different.

Me neither :( Maybe we don't need it? I'll see.

[Andy Estes]

Committed r162817: <http://trac.webkit.org/changeset/162817>

[Alexey Proskuryakov]

This change broke http/tests/misc/link-rel-icon-beforeload.html, and EWS even told so. How could this patch get an r+ when it was known that it breaks a test?

This test is now broken on all Mac bots.

[Alexey Proskuryakov]

Hopefully fixed in <http://trac.webkit.org/r162826>.

I think that this patch could really benefit from another EWS round before landing - it obviously broke Mac, and it also was breaking EFL and Gtk builds. Were those fixed before landing?

[Andy Estes]

(In reply to comment #10)
> Hopefully fixed in <http://trac.webkit.org/r162826>.
> 
> I think that this patch could really benefit from another EWS round before landing - it obviously broke Mac, and it also was breaking EFL and Gtk builds. Were those fixed before landing?

Sorry Alexey. I was focusing on the build errors and overlooked the failing test. I did fix the build errors before landing.