Move DragImage from platform to platform/graphics
Created attachment 187175 [details] Patch
Attachment 187175 [details] did not pass style-queue: Source/WebCore/platform/graphics/DragImage.h:30: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/platform/graphics/DragImage.h:51: Should have a space between // and comment [whitespace/comments] [4] Source/WebCore/platform/graphics/DragImage.h:56: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/WebCore/platform/graphics/DragImage.h:78: Should have a space between // and comment [whitespace/comments] [4] Source/WebCore/platform/graphics/DragImage.h:79: Should have a space between // and comment [whitespace/comments] [4] Source/WebCore/platform/graphics/DragImage.h:79: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WebCore/platform/graphics/DragImage.h:80: Should have a space between // and comment [whitespace/comments] [4] Source/WebCore/platform/graphics/DragImage.h:81: The parameter name "image" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/graphics/DragImage.h:81: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/graphics/DragImage.h:83: The parameter name "image" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/graphics/DragImage.h:93: Should have a space between // and comment [whitespace/comments] [4] Source/WebCore/platform/graphics/win/DragImageWin.cpp:43: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/platform/graphics/win/DragImageWin.cpp:68: Should have a space between // and comment [whitespace/comments] [4] Source/WebCore/platform/graphics/win/DragImageWin.cpp:71: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/WebCore/platform/graphics/win/DragImageWin.cpp:191: Use adoptPtr and OwnPtr<HDC> when calling CreateCompatibleDC to avoid potential memory leaks. [runtime/leaky_pattern] [5] Source/WebCore/platform/graphics/win/DragImageWin.cpp:210: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/win/DragImageCairoWin.cpp:35: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/platform/graphics/win/DragImageCairoWin.cpp:53: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WebCore/platform/graphics/win/DragImageCairoWin.cpp:60: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/win/DragImageCairoWin.cpp:61: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/win/DragImageCairoWin.cpp:62: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/win/DragImageCairoWin.cpp:63: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/win/DragImageCairoWin.cpp:73: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WebCore/platform/graphics/win/DragImageCairoWin.cpp:95: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WebCore/platform/graphics/win/DragImageCairoWin.cpp:99: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/win/DragImageCairoWin.cpp:100: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/win/DragImageCairoWin.cpp:101: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/win/DragImageCairoWin.cpp:102: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/win/DragImageCairoWin.cpp:116: UFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/PlatformBlackBerry.cmake', u'Source/WebCore/PlatformEfl.cmake', u'Source/WebCore/PlatformWinCE.cmake', u'Source/WebCore/Target.pri', u'Source/WebCore/WebCore.gypi', u'Source/WebCore/WebCore.vcproj/WebCore.vcproj', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/platform/DragImage.cpp', u'Source/WebCore/platform/DragImage.h', u'Source/WebCore/platform/blackberry/DragImageBlackBerry.cpp', u'Source/WebCore/platform/chromium/DragImageChromiumSkia.cpp', u'Source/WebCore/platform/chromium/DragImageRef.h', u'Source/WebCore/platform/efl/DragImageEfl.cpp', u'Source/WebCore/platform/graphics/DragImage.cpp', u'Source/WebCore/platform/graphics/DragImage.h', u'Source/WebCore/platform/graphics/blackberry/DragImageBlackBerry.cpp', u'Source/WebCore/platform/graphics/chromium/DragImageChromiumSkia.cpp', u'Source/WebCore/platform/graphics/chromium/DragImageRef.h', u'Source/WebCore/platform/graphics/efl/DragImageEfl.cpp', u'Source/WebCore/platform/graphics/gtk/DragImageGtk.cpp', u'Source/WebCore/platform/graphics/mac/DragImageMac.mm', u'Source/WebCore/platform/graphics/qt/DragImageQt.cpp', u'Source/WebCore/platform/graphics/win/DragImageCGWin.cpp', u'Source/WebCore/platform/graphics/win/DragImageCairoWin.cpp', u'Source/WebCore/platform/graphics/win/DragImageWin.cpp', u'Source/WebCore/platform/graphics/wx/DragImageWx.cpp', u'Source/WebCore/platform/gtk/DragImageGtk.cpp', u'Source/WebCore/platform/mac/DragImageMac.mm', u'Source/WebCore/platform/qt/DragImageQt.cpp', u'Source/WebCore/platform/win/DragImageCGWin.cpp', u'Source/WebCore/platform/win/DragImageCairoWin.cpp', u'Source/WebCore/platform/win/DragImageWin.cpp', u'Source/WebCore/platform/wx/DragImageWx.cpp']" exit_code: 1 se the class HWndDC instead of calling GetDC to avoid potential memory leaks. [runtime/leaky_pattern] [5] Source/WebCore/platform/graphics/win/DragImageCairoWin.cpp:117: Use adoptPtr and OwnPtr<HDC> when calling CreateCompatibleDC to avoid potential memory leaks. [runtime/leaky_pattern] [5] Source/WebCore/platform/graphics/win/DragImageCairoWin.cpp:157: Use the class HWndDC instead of calling GetDC to avoid potential memory leaks. [runtime/leaky_pattern] [5] Source/WebCore/platform/graphics/win/DragImageCairoWin.cpp:158: Use adoptPtr and OwnPtr<HDC> when calling CreateCompatibleDC to avoid potential memory leaks. [runtime/leaky_pattern] [5] Source/WebCore/platform/graphics/wx/DragImageWx.cpp:27: You should add a blank line after implementation file's own header. [build/include_order] [4] Source/WebCore/platform/graphics/wx/DragImageWx.cpp:52: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/WebCore/platform/graphics/win/DragImageCGWin.cpp:37: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/platform/graphics/win/DragImageCGWin.cpp:58: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/win/DragImageCGWin.cpp:59: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/win/DragImageCGWin.cpp:76: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/win/DragImageCGWin.cpp:94: Use adoptPtr and OwnPtr<HDC> when calling CreateCompatibleDC to avoid potential memory leaks. [runtime/leaky_pattern] [5] Source/WebCore/platform/graphics/win/DragImageCGWin.cpp:127: Use adoptPtr and OwnPtr<HDC> when calling CreateCompatibleDC to avoid potential memory leaks. [runtime/leaky_pattern] [5] Source/WebCore/platform/graphics/win/DragImageCGWin.cpp:147: Extra space before [ [whitespace/braces] [5] Source/WebCore/platform/graphics/chromium/DragImageChromiumSkia.cpp:74: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/chromium/DragImageChromiumSkia.cpp:92: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/chromium/DragImageChromiumSkia.cpp:93: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/chromium/DragImageChromiumSkia.cpp:94: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 45 in 23 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 187175 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=187175&action=review > Source/WebCore/ChangeLog:9 > + The code of DragImage depends on the graphics backend, > + so it should be placed into the graphics folder. I’m not sure I agree. It depends on at least the graphics back end, but on other things as well. It’s about integration with the system dragging API. I don’t think this move is necessarily a good idea.
(In reply to comment #3) > (From update of attachment 187175 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=187175&action=review > > > Source/WebCore/ChangeLog:9 > > + The code of DragImage depends on the graphics backend, > > + so it should be placed into the graphics folder. > > I’m not sure I agree. It depends on at least the graphics back end, but on other things as well. It’s about integration with the system dragging API. I don’t think this move is necessarily a good idea. The main reason for this is that I want to merge all platform/wince files into platform/win, since the only difference between them is that WinCE supports a subset of Win32 API. But the DragImageWin has two implementations for the Cairo and CG graphics backend. If you compare http://trac.webkit.org/browser/trunk/Source/WebCore/platform/win?rev=142170 to http://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/win?rev=142170 only DragImage has a *CairoWin.cpp and *CGWin.cpp. So it seams to me that platform/graphics is the better location (at least with the current implementation).
Comment on attachment 187175 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=187175&action=review > Source/WebCore/platform/graphics/DragImage.h:60 > + class CachedImage; > + class Frame; > + class Image; > + class KURL; > + class Range; none of these dependencies make sense for WebCore/platform or for WebCore/platform/graphics. Nothing under the /platform/ tree should be aware of Frame/DOM concepts. If anything, the DOM-aware parts of this class should be moved out of WebCore/platform/ completely.