Bug 80751

Created attachment 131172 [details] Patch

Added CC list based on previous commits.

Comment on attachment 131172 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=131172&action=review > Source/WebKit/win/ChangeLog:3 > + Windows IDL files should use generic pointers for Windows Handles can you explain the why in the changelog > Source/WebKit/win/Interfaces/WebKit.idl:61 > +cpp_quote("#if 0") > +typedef long HWND; > +typedef long HDC; > +typedef long HACCEL; > +typedef long HMENU; > +typedef long HICON; > +typedef long HFONT; > +typedef long HRGN; > +cpp_quote("#endif") can you explain this more detailed (in the ChangeLog) Ususally HWND is defined as HANDLE, which is defined as void*. Why can't we do that here? > Source/WebKit/win/WebKit.vcproj/Interfaces.vcproj:38 > + PreprocessorDefinitions="_MIDL_DECLARE_WIREM_HANDLE" is there no better place to set the PreporcessorDefinitions in one of the vsprops files?

(In reply to comment #3) > (From update of attachment 131172 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=131172&action=review > > > Source/WebKit/win/ChangeLog:3 > > + Windows IDL files should use generic pointers for Windows Handles > > can you explain the why in the changelog > > > Source/WebKit/win/Interfaces/WebKit.idl:61 > > +cpp_quote("#if 0") > > +typedef long HWND; > > +typedef long HDC; > > +typedef long HACCEL; > > +typedef long HMENU; > > +typedef long HICON; > > +typedef long HFONT; > > +typedef long HRGN; > > +cpp_quote("#endif") > > can you explain this more detailed (in the ChangeLog) > Ususally HWND is defined as HANDLE, which is defined as void*. Why can't we do that here? HANDLE is defined as void*, but that's not a portable type. This change is meaningful for non-C languages such as VB and .Net. The one reference I found using this trick is http://miranda-gds-v2.googlecode.com/svn/trunk/common/GoogleDesktopDisplayAPI.idl. This change shouldn't negatively affect C/C++ but should help other languages. Is there a way to test whether or not this will break some code in the wild? Does any of our EWS servers consume WebKit COM? > > > Source/WebKit/win/WebKit.vcproj/Interfaces.vcproj:38 > > + PreprocessorDefinitions="_MIDL_DECLARE_WIREM_HANDLE" > > is there no better place to set the PreporcessorDefinitions in one of the vsprops files? This should remain local to this project. If there is an appropriate prop file, I'll add it there, otherwise, vcproj is reasonable.

Comment on attachment 131172 [details] Patch Have you e-mailed webkit-dev about your desire to support non-C languages such as VB and .Net?

Your email to webkit-dev suggests that this change would break binary compatibility with previous versions of WebKit. Can you confirm that is the case?

Comment on attachment 131172 [details] Patch Attachment 131172 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/11942271

(In reply to comment #6) > Your email to webkit-dev suggests that this change would break binary compatibility with previous versions of WebKit. Can you confirm that is the case? That is the fear, yes. I can't confirm it without testing. But right now more interestingly the Win EWS shows errors. I don't know if it was offline or what, but for the longest time it didn't complain. Since I'm building with vs2010 I didn't get no build errors (yes, I've consumed the modified COM successfully and without hickups). So first I have to resolve the build issues and, considering there isn't much feedback on (in)compatibility of my changes, I'll have to do testing and report my results to the community to decide whether or not this is an desired change.

If this breaks binary compatibility then there's no way we can accept it.

(In reply to comment #9) > If this breaks binary compatibility then there's no way we can accept it. What are the known consumers of WebKit COM? What kind of effort is involved in changing binary compatibility?

Clients include Safari and iTunes on Windows. You can test binary compatibility by building the Apple Windows WebKit port and running it against installed Safari. "Changing binary compatibility" is not something we do. We preserve binary compatibility for existing clients.

Comment on attachment 131172 [details] Patch Attachment 131172 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/41012

Comment on attachment 131172 [details] Patch Attachment 131172 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/74132 New failing tests: css3/filters/custom/filter-fallback-to-software.html

Created attachment 197660 [details] Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-16 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.2

Comment on attachment 131172 [details] Patch We cannot break binary compatibility.