Bug 148001 - [Win] Replace the MIDL comments ([in], [inout]) in interface code with equivalent SAL declarations
Summary: [Win] Replace the MIDL comments ([in], [inout]) in interface code with equiva...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords:
Depends on:
Blocks: 148240
  Show dependency treegraph
 
Reported: 2015-08-13 16:17 PDT by Brent Fulgham
Modified: 2015-08-20 14:37 PDT (History)
2 users (show)

See Also:


Attachments
Patch (1.30 MB, patch)
2015-08-19 11:04 PDT, Brent Fulgham
thorton: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2015-08-13 16:17:23 PDT
Our Windows COM interfaces are peppered with old, possibly invalid comments based on the IDL used to generate the original Windows headers years ago.

Instead, we should have the MIDL compiler generate output with appropriate SAL declarations, and modify our Windows code to use the right SAL declarations instead of these comments.

While this won't make the code any prettier on Windows, it has the advantage of allowing the compiler tell us if any of our implementations deviate from the interface declarations, and will help Microsoft's static analyzer do a better job of analyzing our Windows code.
Comment 1 Brent Fulgham 2015-08-19 11:04:41 PDT
Created attachment 259380 [details]
Patch
Comment 2 WebKit Commit Bot 2015-08-19 11:07:45 PDT
Attachment 259380 [details] did not pass style-queue:


ERROR: Source/WebKit/win/WebDataSource.cpp:53:  IID_WebDataSource is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/Interfaces/Accessible2/AccessibleEditableText.idl:54:  One or more unexpected \r (^M) found; better to use only a \n  [whitespace/carriage_return] [1]
ERROR: Suppressing further [whitespace/carriage_return] reports for this file.
ERROR: Source/WebKit/win/WebArchive.h:55:  The parameter name "subResources" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit/win/WebArchive.h:56:  The parameter name "subFrameArchives" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit/win/AccessibleBase.h:51:  The parameter name "targets" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit/win/AccessibleBase.h:65:  The parameter name "extendedStates" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit/win/AccessibleBase.h:66:  The parameter name "localizedExtendedStates" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit/win/Interfaces/Accessible2/AccessibleText2.idl:54:  One or more unexpected \r (^M) found; better to use only a \n  [whitespace/carriage_return] [1]
ERROR: Suppressing further [whitespace/carriage_return] reports for this file.
ERROR: Source/WebKit/win/WebHTMLRepresentation.h:61:  The parameter name "labels" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit/win/WebHTMLRepresentation.h:62:  The parameter name "labels" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit/win/WebHTMLRepresentation.h:63:  The parameter name "labels" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit/win/DOMHTMLClasses.h:154:  Omit int when using unsigned  [runtime/unsigned] [1]
ERROR: Source/WebKit/win/DOMHTMLClasses.h:912:  Extra space after ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/win/DOMHTMLClasses.cpp:390:  DOMHTMLDocument::getElementById_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:141:  AccessibleBase::get_attribute is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:153:  AccessibleBase::get_accessibleWithCaret is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:163:  AccessibleBase::get_relationTargetsOfType is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:170:  AccessibleBase::get_nRelations is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:182:  AccessibleBase::get_relation is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:193:  AccessibleBase::get_relations is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:230:  AccessibleBase::get_groupPosition is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:236:  AccessibleBase::get_states is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:249:  AccessibleBase::get_extendedRole is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:261:  AccessibleBase::get_localizedExtendedRole is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:273:  AccessibleBase::get_nExtendedStates is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:286:  AccessibleBase::get_extendedStates is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:292:  AccessibleBase::get_localizedExtendedStates is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:298:  AccessibleBase::get_uniqueID is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:310:  AccessibleBase::get_windowHandle is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:316:  AccessibleBase::get_indexInParent is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:321:  AccessibleBase::get_locale is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:334:  AccessibleBase::get_attributes is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:347:  AccessibleBase::get_accParent is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:371:  AccessibleBase::get_accChildCount is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:381:  AccessibleBase::get_accChild is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:399:  AccessibleBase::get_accName is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:417:  AccessibleBase::get_accValue is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:543:  AccessibleBase::get_accState is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:562:  AccessibleBase::get_accHelp is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/AccessibleBase.cpp:580:  AccessibleBase::get_accKeyboardShortcut is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/WebView.cpp:4722:  WebView::delete_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/WebView.cpp:6543:  __inout_ecount_full is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/WebView.cpp:6544:  __inout_ecount_full is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/WebView.cpp:6563:  __inout_ecount_full is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/WebView.cpp:6564:  __inout_ecount_full is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/WebView.cpp:6587:  __inout_ecount_full is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/WebView.cpp:6593:  __inout_ecount_full is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/Interfaces/Accessible2/AccessibleText.idl:54:  One or more unexpected \r (^M) found; better to use only a \n  [whitespace/carriage_return] [1]
ERROR: Suppressing further [whitespace/carriage_return] reports for this file.
ERROR: Source/WebKit/win/WebView.h:118:  The parameter name "mimeTypes" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit/win/WebView.h:169:  The parameter name "types" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit/win/WebView.h:171:  The parameter name "withPasteboardTypes" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit/win/WebView.h:345:  __inout_ecount_full is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/WebView.h:345:  The parameter name "whitelist" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit/win/WebView.h:346:  The parameter name "blacklist" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit/win/WebView.h:348:  The parameter name "whitelist" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit/win/WebView.h:348:  The parameter name "blacklist" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit/win/WebView.h:348:  __inout_ecount_full is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/WebView.h:368:  The parameter name "position" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit/win/WebView.h:369:  The parameter name "error" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit/win/WebView.h:389:  The parameter name "whitelist" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit/win/WebView.h:389:  The parameter name "blacklist" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit/win/WebView.h:389:  __inout_ecount_full is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebKit/win/WebView.h:391:  The parameter name "whitelist" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit/win/WebView.h:391:  The parameter name "blacklist" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit/win/WebView.h:391:  __inout_ecount_full is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 71 in 134 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Brent Fulgham 2015-08-19 11:11:55 PDT
(In reply to comment #2)
> Attachment 259380 [details] did not pass style-queue:
> 
> 
> ERROR: Source/WebKit/win/WebDataSource.cpp:53:  IID_WebDataSource is
> incorrectly named. Don't use underscores in your identifier names. 
> [readability/naming/underscores] [4]
> ERROR:
> Source/WebKit/win/Interfaces/Accessible2/AccessibleEditableText.idl:54:  One

[ ... ]

> named. Don't use underscores in your identifier names. 
> [readability/naming/underscores] [4]
> Total errors found: 71 in 134 files
> 

These are all required names due to various COM interfaces defined by Microsoft and the Accessibility specifications.
Comment 4 Tim Horton 2015-08-19 11:17:36 PDT
Comment on attachment 259380 [details]
Patch

yikes. rs=me, but please test carefully.
Comment 5 Brent Fulgham 2015-08-19 17:00:51 PDT
Committed r188662: <http://trac.webkit.org/changeset/188662>