Bug 168939 – Make ImageDiff stand-alone

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Jonathan Bedard]

ImageDiff should not depend on WTF and bmalloc.  Remove these dependencies and make the project a stand-alone project on Mac.

[Radar WebKit Bug Importer]

<rdar://problem/30743805>

[Jonathan Bedard]

Created attachment 308910 [details]
Patch

[Build Bot]

Attachment 308910 [details] did not pass style-queue:


ERROR: Tools/ImageDiff/cg/ImageDiff.cpp:34:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Tools/ImageDiff/cg/ImageDiff.cpp:35:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Tools/ImageDiff/cg/ImageDiff.cpp:152:  Missing space before ( in for(  [whitespace/parens] [5]
ERROR: Tools/ImageDiff/cg/ImageDiff.cpp:161:  An else should appear on the same line as the preceding }  [whitespace/newline] [4]
Total errors found: 4 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jonathan Bedard]

Created attachment 308913 [details]
Patch

[Build Bot]

Attachment 308913 [details] did not pass style-queue:


ERROR: Tools/ImageDiff/cg/ImageDiff.cpp:34:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Tools/ImageDiff/cg/ImageDiff.cpp:35:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Tools/ImageDiff/cg/ImageDiff.cpp:152:  Missing space before ( in for(  [whitespace/parens] [5]
Total errors found: 3 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jonathan Bedard]

(In reply to Build Bot from comment #5)
> Attachment 308913 [details] did not pass style-queue:
> 
> 
> ERROR: Tools/ImageDiff/cg/ImageDiff.cpp:34:  Alphabetical sorting problem. 
> [build/include_order] [4]
> ERROR: Tools/ImageDiff/cg/ImageDiff.cpp:35:  Alphabetical sorting problem. 
> [build/include_order] [4]
> ERROR: Tools/ImageDiff/cg/ImageDiff.cpp:152:  Missing space before ( in for(
> [whitespace/parens] [5]
> Total errors found: 3 in 7 files
> 
> 
> If any of these errors are false positives, please file a bug against
> check-webkit-style.

Oddly, these do not show up locally when I run check-webkit-style.

The first 2 are not issues that should be fixed, if I recall correctly, winsock2.h and windows.h are a pair of headers which ware order-sensitive when imported.

The second is an error which will be fixed in the next iteration of this patch.

[Build Bot]

Comment on attachment 308913 [details]
Patch

Attachment 308913 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3665618

New failing tests:
fast/borders/border-image-fill-no-border.html

[Build Bot]

Created attachment 308924 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 308913 [details]
Patch

Attachment 308913 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3665626

New failing tests:
fast/borders/border-image-fill-no-border.html

[Build Bot]

Created attachment 308925 [details]
Archive of layout-test-results from ews116 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 308913 [details]
Patch

Attachment 308913 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3665824

New failing tests:
compositing/overlap-blending/children-opacity-no-overlap.html

[Build Bot]

Created attachment 308931 [details]
Archive of layout-test-results from ews102 for mac-elcapitan

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

[Jonathan Bedard]

Created attachment 308933 [details]
Patch

[Build Bot]

Attachment 308933 [details] did not pass style-queue:


ERROR: Tools/ImageDiff/cg/ImageDiff.cpp:34:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Tools/ImageDiff/cg/ImageDiff.cpp:35:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[David Kilzer (:ddkilzer)]

Comment on attachment 308933 [details]
Patch

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

r- to fix xcconfig files.

> Tools/ChangeLog:10
> +        Create ImageDiff without dependencies on bmalloc and WTF so that it exists as a
> +        stand-alone project. Note that this change does not eliminate the ImageDiff inside
> +        the DumpRenderTree project.

I don't see any changes to cmake files for the moved ImageDiff sources.

Maybe we're only moving iOS and macOS first?

> Tools/ChangeLog:18
> +        * ImageDiff/cg/Configurations: Added.
> +        * ImageDiff/cg/Configurations/BaseTarget.xcconfig: Copied from Tools/DumpRenderTree/mac/Configurations/BaseTarget.xcconfig.
> +        * ImageDiff/cg/Configurations/ImageDiff.xcconfig: Copied from Tools/DumpRenderTree/mac/Configurations/ImageDiff.xcconfig.

You're missing Tools/DumpRenderTree/mac/Configurations/DebugRelease.xcconfig.

DebugRelease.xcconfig is the basis for the Debug/Release configurations in the Xcode project, which are set to ImageDiff.xcconfig now.

> Tools/ImageDiff/ImageDiff.xcodeproj/project.pbxproj:179
> +			buildSettings = {
> +				ALWAYS_SEARCH_USER_PATHS = NO;
> +				CLANG_ANALYZER_NONNULL = YES;
> +				CLANG_ANALYZER_NUMBER_OBJECT_CONVERSION = YES_AGGRESSIVE;
> +				CLANG_CXX_LANGUAGE_STANDARD = "gnu++14";
> +				CLANG_CXX_LIBRARY = "libc++";
> +				CLANG_ENABLE_MODULES = YES;
> +				CLANG_ENABLE_OBJC_ARC = YES;
> +				CLANG_WARN_BOOL_CONVERSION = YES;
> +				CLANG_WARN_CONSTANT_CONVERSION = YES;
> +				CLANG_WARN_DIRECT_OBJC_ISA_USAGE = YES_ERROR;
> +				CLANG_WARN_DOCUMENTATION_COMMENTS = YES;
> +				CLANG_WARN_EMPTY_BODY = YES;
> +				CLANG_WARN_ENUM_CONVERSION = YES;
> +				CLANG_WARN_INFINITE_RECURSION = YES;
> +				CLANG_WARN_INT_CONVERSION = YES;
> +				CLANG_WARN_OBJC_ROOT_CLASS = YES_ERROR;
> +				CLANG_WARN_RANGE_LOOP_ANALYSIS = YES;
> +				CLANG_WARN_SUSPICIOUS_MOVE = YES;
> +				CLANG_WARN_UNREACHABLE_CODE = YES;
> +				CLANG_WARN__DUPLICATE_METHOD_MATCH = YES;
> +				CODE_SIGN_IDENTITY = "-";
> +				COPY_PHASE_STRIP = NO;
> +				DEBUG_INFORMATION_FORMAT = "dwarf-with-dsym";
> +				ENABLE_NS_ASSERTIONS = NO;
> +				ENABLE_STRICT_OBJC_MSGSEND = YES;
> +				GCC_C_LANGUAGE_STANDARD = gnu11;
> +				GCC_NO_COMMON_BLOCKS = YES;
> +				GCC_WARN_64_TO_32_BIT_CONVERSION = YES;
> +				GCC_WARN_ABOUT_RETURN_TYPE = YES_ERROR;
> +				GCC_WARN_UNDECLARED_SELECTOR = YES;
> +				GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE;
> +				GCC_WARN_UNUSED_FUNCTION = YES;
> +				GCC_WARN_UNUSED_VARIABLE = YES;
> +				MACOSX_DEPLOYMENT_TARGET = 10.13;
> +				MTL_ENABLE_DEBUG_INFO = NO;
> +				SDKROOT = macosx;
> +			};

This section should be blank because these settings are picked up from the *.xcconfig files.

> Tools/ImageDiff/ImageDiff.xcodeproj/project.pbxproj:236
> +			buildSettings = {
> +				ALWAYS_SEARCH_USER_PATHS = NO;
> +				CLANG_ANALYZER_NONNULL = YES;
> +				CLANG_ANALYZER_NUMBER_OBJECT_CONVERSION = YES_AGGRESSIVE;
> +				CLANG_CXX_LANGUAGE_STANDARD = "gnu++14";
> +				CLANG_CXX_LIBRARY = "libc++";
> +				CLANG_ENABLE_MODULES = YES;
> +				CLANG_ENABLE_OBJC_ARC = YES;
> +				CLANG_WARN_BOOL_CONVERSION = YES;
> +				CLANG_WARN_CONSTANT_CONVERSION = YES;
> +				CLANG_WARN_DIRECT_OBJC_ISA_USAGE = YES_ERROR;
> +				CLANG_WARN_DOCUMENTATION_COMMENTS = YES;
> +				CLANG_WARN_EMPTY_BODY = YES;
> +				CLANG_WARN_ENUM_CONVERSION = YES;
> +				CLANG_WARN_INFINITE_RECURSION = YES;
> +				CLANG_WARN_INT_CONVERSION = YES;
> +				CLANG_WARN_OBJC_ROOT_CLASS = YES_ERROR;
> +				CLANG_WARN_RANGE_LOOP_ANALYSIS = YES;
> +				CLANG_WARN_SUSPICIOUS_MOVE = YES;
> +				CLANG_WARN_UNREACHABLE_CODE = YES;
> +				CLANG_WARN__DUPLICATE_METHOD_MATCH = YES;
> +				CODE_SIGN_IDENTITY = "-";
> +				COPY_PHASE_STRIP = NO;
> +				DEBUG_INFORMATION_FORMAT = dwarf;
> +				ENABLE_STRICT_OBJC_MSGSEND = YES;
> +				ENABLE_TESTABILITY = YES;
> +				GCC_C_LANGUAGE_STANDARD = gnu11;
> +				GCC_DYNAMIC_NO_PIC = NO;
> +				GCC_NO_COMMON_BLOCKS = YES;
> +				GCC_OPTIMIZATION_LEVEL = 0;
> +				GCC_PREPROCESSOR_DEFINITIONS = (
> +					"DEBUG=1",
> +					"$(inherited)",
> +				);
> +				GCC_WARN_64_TO_32_BIT_CONVERSION = YES;
> +				GCC_WARN_ABOUT_RETURN_TYPE = YES_ERROR;
> +				GCC_WARN_UNDECLARED_SELECTOR = YES;
> +				GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE;
> +				GCC_WARN_UNUSED_FUNCTION = YES;
> +				GCC_WARN_UNUSED_VARIABLE = YES;
> +				MACOSX_DEPLOYMENT_TARGET = 10.13;
> +				MTL_ENABLE_DEBUG_INFO = YES;
> +				ONLY_ACTIVE_ARCH = YES;
> +				SDKROOT = macosx;
> +			};

Ditto.

> Tools/ImageDiff/ImageDiff.xcodeproj/project.pbxproj:278
> +			buildSettings = {
> +				ALWAYS_SEARCH_USER_PATHS = NO;
> +				CLANG_ANALYZER_NONNULL = YES;
> +				CLANG_ANALYZER_NUMBER_OBJECT_CONVERSION = YES_AGGRESSIVE;
> +				CLANG_CXX_LANGUAGE_STANDARD = "gnu++14";
> +				CLANG_CXX_LIBRARY = "libc++";
> +				CLANG_ENABLE_MODULES = YES;
> +				CLANG_ENABLE_OBJC_ARC = YES;
> +				CLANG_WARN_BOOL_CONVERSION = YES;
> +				CLANG_WARN_CONSTANT_CONVERSION = YES;
> +				CLANG_WARN_DIRECT_OBJC_ISA_USAGE = YES_ERROR;
> +				CLANG_WARN_DOCUMENTATION_COMMENTS = YES;
> +				CLANG_WARN_EMPTY_BODY = YES;
> +				CLANG_WARN_ENUM_CONVERSION = YES;
> +				CLANG_WARN_INFINITE_RECURSION = YES;
> +				CLANG_WARN_INT_CONVERSION = YES;
> +				CLANG_WARN_OBJC_ROOT_CLASS = YES_ERROR;
> +				CLANG_WARN_RANGE_LOOP_ANALYSIS = YES;
> +				CLANG_WARN_SUSPICIOUS_MOVE = YES;
> +				CLANG_WARN_UNREACHABLE_CODE = YES;
> +				CLANG_WARN__DUPLICATE_METHOD_MATCH = YES;
> +				CODE_SIGN_IDENTITY = "-";
> +				COPY_PHASE_STRIP = NO;
> +				DEBUG_INFORMATION_FORMAT = "dwarf-with-dsym";
> +				ENABLE_NS_ASSERTIONS = NO;
> +				ENABLE_STRICT_OBJC_MSGSEND = YES;
> +				GCC_C_LANGUAGE_STANDARD = gnu11;
> +				GCC_NO_COMMON_BLOCKS = YES;
> +				GCC_WARN_64_TO_32_BIT_CONVERSION = YES;
> +				GCC_WARN_ABOUT_RETURN_TYPE = YES_ERROR;
> +				GCC_WARN_UNDECLARED_SELECTOR = YES;
> +				GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE;
> +				GCC_WARN_UNUSED_FUNCTION = YES;
> +				GCC_WARN_UNUSED_VARIABLE = YES;
> +				MACOSX_DEPLOYMENT_TARGET = 10.13;
> +				MTL_ENABLE_DEBUG_INFO = NO;
> +				SDKROOT = macosx;
> +			};

Ditto.

> Tools/ImageDiff/cg/ImageDiff.cpp:2
> + * Copyright (C) 2005, 2007, 2015 Apple Inc. All rights reserved.

Maybe update copyright.

Why isn't this named Tools/ImageDiff/cg/ImageDiffCG.cpp?

> Tools/ImageDiff/cg/ImageDiff.cpp:85
> +    CGDataProviderRef dataProvider = CGDataProviderCreateWithCFData(data);
> +    CGImageRef result = CGImageCreateWithPNGDataProvider(dataProvider, 0, false, kCGRenderingIntentDefault);
> +    CFRelease(data);
> +    CFRelease(dataProvider);

Nit:  Could call CFRelease(data) one line earlier.

> Tools/ImageDiff/cg/ImageDiff.cpp:151
> +            diff = (unsigned char*) diffBuffer;

No space for the C-style cast here.  Could use a C++ cast here:

            diff = reinterpret_cast<unsigned char*>(diffBuffer);

> Tools/ImageDiff/cg/Configurations/BaseTarget.xcconfig:25
> +OTHER_CFLAGS = $(inherited) -iframework $(SDKROOT)$(SYSTEM_LIBRARY_DIR)/Frameworks/Quartz.framework/Frameworks -iframework $(SDKROOT)$(SYSTEM_LIBRARY_DIR)/Frameworks/ApplicationServices.framework/Frameworks -iframework $(SDKROOT)$(SYSTEM_LIBRARY_DIR)/Frameworks/CoreServices.framework/Frameworks -iframework $(SDKROOT)$(SYSTEM_LIBRARY_DIR)/Frameworks/WebKit.framework/Frameworks;
> +OTHER_CPLUSPLUSFLAGS = $(OTHER_CFLAGS);

I'm pretty sure these only apply to [sdk=macosx*].  They probably don't do anything on iOS, but they should be:

OTHER_CFLAGS[sdk=macosx*] = $(inherited) -iframework $(SDKROOT)$(SYSTEM_LIBRARY_DIR)/Frameworks/Quartz.framework/Frameworks -iframework $(SDKROOT)$(SYSTEM_LIBRARY_DIR)/Frameworks/ApplicationServices.framework/Frameworks -iframework $(SDKROOT)$(SYSTEM_LIBRARY_DIR)/Frameworks/CoreServices.framework/Frameworks -iframework $(SDKROOT)$(SYSTEM_LIBRARY_DIR)/Frameworks/WebKit.framework/Frameworks;
OTHER_CPLUSPLUSFLAGS[sdk=macosx*] = $(OTHER_CFLAGS);

Oh, I guess we're only building ImageDiff for macOS going forward, though.  If so, we don't need the OTHER_LDFLAGS[sdk=iphone*] in ImageDiff.xcconfig then.

> Tools/ImageDiff/cg/Configurations/ImageDiff.xcconfig:24
> +#include "BaseTarget.xcconfig"

You can probably inline BaseTarget.xcconfig into this file, and remove BaseTarget.xcconfig.

It only existed in the DumpRenderTree project to prevent duplication of xcconfig settings.  No need for it here anymore.

[Jonathan Bedard]

Created attachment 308978 [details]
Patch

[Build Bot]

Attachment 308978 [details] did not pass style-queue:


ERROR: Tools/ImageDiff/cg/ImageDiff.cpp:34:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Tools/ImageDiff/cg/ImageDiff.cpp:35:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jonathan Bedard]

Comment on attachment 308978 [details]
Patch

A few comments on the changes.

Generally, yes, this is meant to be built only on Mac.  But, I'm not going to specifically disable building it for other platforms (namely iOS simulator) so that we don't break anything while I'm making these changes.

First, I moved over some of the CMake stuff.  That does need to be done since the next patch will move ImageDiff out of DumpRenderTree entirely.

Second, along with DebugRelease.xcconfig, Base.xcconfig needed to be moved as well.  Both had some WebKit references in them, those were removed.

Lastly, I don't think your comment about OTHER_CPLUSPLUSFLAGS is correct.  Or at least, reading through our other config files, it doesn't seem to be correct.  Looking at something like WebKitTestRunnerApp.xcconfig, it seems that a variable without [platform*] applies to all platforms.  Otherwise, to point back to the WebKitTestRunnerApp example, that configuration file would not link WebKitTestRunnerApp against the required frameworks.

[Jonathan Bedard]

Created attachment 309049 [details]
Patch

[Build Bot]

Attachment 309049 [details] did not pass style-queue:


ERROR: Tools/ImageDiff/cg/ImageDiff.cpp:34:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Tools/ImageDiff/cg/ImageDiff.cpp:35:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jonathan Bedard]

Created attachment 309050 [details]
Patch

[Build Bot]

Attachment 309050 [details] did not pass style-queue:


ERROR: Tools/ImageDiff/cg/ImageDiff.cpp:34:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Tools/ImageDiff/cg/ImageDiff.cpp:35:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jonathan Bedard]

Comment on attachment 309050 [details]
Patch

Needed to get the GTK build working.

[David Kilzer (:ddkilzer)]

Comment on attachment 309050 [details]
Patch

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

r=me if you fix the cmake issues and Xcode project file issue.  Get Alex Christensen to look at the cmake stuff if you want a second opinion.

> Tools/ImageDiff/PlatformGTK.cmake:2
> +set(PLATFORM_WIN_CAIRO 1)
> +include(PlatformWin.cmake)

This seems a bit dodgy.  GTK port != WinCairo port.  Need something more specific like PLATFORM_CAIRO that's set for both GTK and WIN_CAIRO to do this.

I really think you should keep Cairo.cmake, and then just include(Cairo.cmake) both here and in PlatformWin.cmake.

> Tools/ImageDiff/PlatformWin.cmake:26
> +    list(APPEND IMAGE_DIFF_LIBRARIES
> +        CFNetwork
> +        CoreGraphics
> +        CoreText
> +    )
> +    set(IMAGE_DIFF_SOURCES
> +        ${IMAGE_DIFF_DIR}/cg/ImageDiff.cpp
> +    )
> +    list(APPEND ImageDiff_LIBRARIES
> +        CoreFoundation
> +        CoreGraphics
> +        CoreText
> +    )

This is identical to PlatformMac.cmake.  Can we just include(PlatformMac.cmake) here?

> Tools/ImageDiff/ImageDiff.xcodeproj/project.pbxproj:152
> +		318E7A281EB939AD00DAE9FC /* Debug */ = {
> +			isa = XCBuildConfiguration;
> +			baseConfigurationReference = 317C0C271EB9543900439F71 /* ImageDiff.xcconfig */;

This baseConfigurationReference should be DebugRelease.xcconfig.

> Tools/ImageDiff/ImageDiff.xcodeproj/project.pbxproj:160
> +		318E7A291EB939AD00DAE9FC /* Release */ = {
> +			isa = XCBuildConfiguration;
> +			baseConfigurationReference = 317C0C271EB9543900439F71 /* ImageDiff.xcconfig */;

This baseConfigurationReference should be DebugRelease.xcconfig.

[Jonathan Bedard]

Created attachment 309094 [details]
Patch

[Jonathan Bedard]

Comment on attachment 309094 [details]
Patch

CQ-ing this because I will need to land it locally since it also requires that we ignore some files.

[Build Bot]

Attachment 309094 [details] did not pass style-queue:


ERROR: Tools/ImageDiff/cg/ImageDiff.cpp:34:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Tools/ImageDiff/cg/ImageDiff.cpp:35:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jonathan Bedard]

Committed in <https://trac.webkit.org/changeset/216207/webkit>.

[Carlos Garcia Campos]

Comment on attachment 309094 [details]
Patch

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

> Tools/ChangeLog:10
> +        Create ImageDiff without dependencies on bmalloc and WTF so that it exists as a
> +        stand-alone project. Note that this change does not eliminate the ImageDiff inside
> +        the DumpRenderTree project.

So, now both ImageDiff are built at the same time, and to the same output? I was trying to remove all duplicated implementations of Imagediff in bug #170608 and this si adding yet another one. Should I rebase that patch on top of this new imageDiff?

[Jonathan Bedard]

(In reply to Carlos Garcia Campos from comment #29)
> Comment on attachment 309094 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=309094&action=review
> 
> > Tools/ChangeLog:10
> > +        Create ImageDiff without dependencies on bmalloc and WTF so that it exists as a
> > +        stand-alone project. Note that this change does not eliminate the ImageDiff inside
> > +        the DumpRenderTree project.
> 
> So, now both ImageDiff are built at the same time, and to the same output? I
> was trying to remove all duplicated implementations of Imagediff in bug
> #170608 and this si adding yet another one. Should I rebase that patch on
> top of this new imageDiff?

https://bugs.webkit.org/show_bug.cgi?id=168945 will be removing the ImageDiff inside DumpRenderTree.  That should be landing shortly and leave all ImageDiff code inside Tools/ImageDiff.

The reason for this, by the way, is so that iOS can use a Mac ImageDiff instead of an iOS image.

[Jonathan Bedard]

(In reply to Carlos Garcia Campos from comment #29)
> Comment on attachment 309094 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=309094&action=review
> 
> > Tools/ChangeLog:10
> > +        Create ImageDiff without dependencies on bmalloc and WTF so that it exists as a
> > +        stand-alone project. Note that this change does not eliminate the ImageDiff inside
> > +        the DumpRenderTree project.
> 
> So, now both ImageDiff are built at the same time, and to the same output? I
> was trying to remove all duplicated implementations of Imagediff in bug
> #170608 and this si adding yet another one. Should I rebase that patch on
> top of this new imageDiff?

Just looked at #170608.  Yes, I would rebase that patch on top of this new ImageDiff.  Don't worry about the old one in DumpRenderTree, I'm about to remove it.  We're building double for the time being so that we don't break any infrastructure.

[Carlos Garcia Campos]

(In reply to Jonathan Bedard from comment #30)
> (In reply to Carlos Garcia Campos from comment #29)
> > Comment on attachment 309094 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=309094&action=review
> > 
> > > Tools/ChangeLog:10
> > > +        Create ImageDiff without dependencies on bmalloc and WTF so that it exists as a
> > > +        stand-alone project. Note that this change does not eliminate the ImageDiff inside
> > > +        the DumpRenderTree project.
> > 
> > So, now both ImageDiff are built at the same time, and to the same output? I
> > was trying to remove all duplicated implementations of Imagediff in bug
> > #170608 and this si adding yet another one. Should I rebase that patch on
> > top of this new imageDiff?
> 
> https://bugs.webkit.org/show_bug.cgi?id=168945 will be removing the
> ImageDiff inside DumpRenderTree.  That should be landing shortly and leave
> all ImageDiff code inside Tools/ImageDiff.

Ah, perfect, thanks!

> The reason for this, by the way, is so that iOS can use a Mac ImageDiff
> instead of an iOS image.