1288199 - Modify ia2marshal.dll to expose additional type information

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[(No longer employed by Mozilla) Aaron Klotz]

In order to make IA2 interfaces compatible with the mscom com registration in bug 1286986 and mscom interceptor code from bug 1263224, we need to expose some additional information from ia2marshal.dll.

Firstly, we need to export the GetProxyDllInfo function in the def file.

Secondly, we need to expose the IA2 interfaces as a typelib:

1) Add the merged IA2 IDL file, ia2_api_all.idl. This is added verbatim from the ia2 website, sourced from http://accessibility.linuxfoundation.org/a11yspecs/ia2/ia2_api_all.idl

2) Generate a typelib from the merged IDL;

3) Embed that typelib in ia2marshal.dll's resources.

Comment 1

[(No longer employed by Mozilla) Aaron Klotz]

Note about generating the IA2 typelib:

By default, HWND marshaling is not compatible with the mscom interceptor because it contains a union (see the RemotableHandle structure declared in wtypes.idl in the Windows SDK).

Fortunately both fields in that union are the same type (LONG) but just different names. This gives us some flexibility at the binary level.

Instead of running midl directly on ia2_api_all.idl, I have written accessible/interfaces/ia2/IA2Typelib.idl. This generates the same typelib as ia2_api_all.idl would generate, but with the exception that is uses slightly different marshaling types for Windows UI handles. These types are wire-compatible with existing handle marshaling but also work with the mscom interceptor because they lack the union.

Comment 2

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Bug 1288199: Add building of typelib to be embedded in ia2marshal.dll;

Trevor, please read comment 1 in the bug for some discussion around the idl files in this patch.

Review commit: https://reviewboard.mozilla.org/r/65628/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/65628/

Comment 3

[Trevor Saunders (:tbsaunde)]

Comment on attachment 8772959 [details]
Bug 1288199: Add building of typelib to be embedded in ia2marshal.dll;

This all seems "reasonable", but I'm not really sure I'm enough of a make wizard to say that part is ok.

Comment 4

[Michael Shal [:mshal]]

Comment on attachment 8772959 [details]
Bug 1288199: Add building of typelib to be embedded in ia2marshal.dll;

https://reviewboard.mozilla.org/r/65628/#review63008

Looks reasonable to me, though I wish we had a better way to handle midl in moz.build. Just a few nits & comments to fix.

::: accessible/interfaces/ia2/Makefile.in:8
(Diff revision 1)
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
>  IA2DIR        = $(topsrcdir)/other-licenses/ia2
>  
> -GARBAGE       += $(MIDL_GENERATED_FILES)
> +GARBAGE       += $(MIDL_GENERATED_FILES) \
> +								 $(MIDL_UNUSED_GENERATED_FILES) \

nit: use spaces to indent here, not tabs.

::: accessible/interfaces/ia2/Makefile.in:45
(Diff revision 1)
>    AccessibleStates.idl \
>    IA2CommonTypes.idl \
>    $(NULL)
>  
> +MIDL_LIBRARIES = \
> +	IA2Typelib.idl \

nit: use spaces here too.

::: accessible/interfaces/ia2/Makefile.in:62
(Diff revision 1)
>    $(MIDL_INTERFACES:%.idl=%_i.c) \
>    $(MIDL_INTERFACES:%.idl=%.h) \
>    $(MIDL_ENUMS:%.idl=%.h) \
>    $(NULL)
>  
> +MIDL_UNUSED_GENERATED_FILES = \

Can you document what MIDL_UNUSED_GENERATED_FILES is intended for as a comment here? It's not immediately obvious without understanding how the .tlb file is generated.

::: accessible/interfaces/ia2/Makefile.in:63
(Diff revision 1)
>    $(MIDL_INTERFACES:%.idl=%.h) \
>    $(MIDL_ENUMS:%.idl=%.h) \
>    $(NULL)
>  
> +MIDL_UNUSED_GENERATED_FILES = \
> +	$(MIDL_LIBRARIES:%.idl=%_p.c) \

nit: spaces here too.

::: accessible/interfaces/ia2/Makefile.in:96
(Diff revision 1)
>  	for idl in $(sort $(subst FORCE,,$?) $(addsuffix .idl,$(addprefix $(IA2DIR)/,$(missing_base)))); do \
>  	  $(MIDL) $(MIDL_FLAGS) -app_config -I $(IA2DIR) -Oicf $$idl; \
>  	done
>  	touch $@
>  
> -# This marshall dll is also registered in the installer
> +typelib_done : $(MIDL_LIBRARIES)

Please also add a comment to this rule that the intent is to generate the .tlb file used by the .rc file.

Comment 5

[(No longer employed by Mozilla) Aaron Klotz]

Comment on attachment 8772959 [details]
Bug 1288199: Add building of typelib to be embedded in ia2marshal.dll;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65628/diff/1-2/

Comment 6

[(No longer employed by Mozilla) Aaron Klotz]

Comment on attachment 8772959 [details]
Bug 1288199: Add building of typelib to be embedded in ia2marshal.dll;

Carrying forward r+

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/73dff614932f
Add building of typelib to be embedded in ia2marshal.dll; r=mshal

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/73dff614932f