843984 - msdmo should be delay loaded dll w/ MOZ_WEBRTC_IN_LIBXUL

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[Makoto Kato [:m_kato]]

Attachment: fix

By bug 780069, msdmo.dll is delay loading dll.  But it isn't be merged with MOZ_WEBRTC_IN_LIBXUL.

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 716970 [details] [diff] [review]
fix

-> ted as this is a build patch

I notice a bunch of libs were removed - these were copied in from the gyp/etc stuff; they may not actually be needed - ted, opinions?

If this is r+'d, please push a windows Try (win64 too perhaps) with tests before landing.

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

blocking not decided yet.  

What practical impact does this have?  Faster FF load times?  If so, how much?  And if so and "large enough", since this code is already in production builds there's a question if uplift to Aurora or Beta would be in order.

Comment 3

[(not currently active) Ted Mielczarek]

Comment on attachment 716970 [details] [diff] [review]
fix

Review of attachment 716970 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/media/Makefile.in
@@ -152,5 @@
> -ifdef MOZ_WEBRTC
> -EXTRA_DSO_LDOPTS += \
> -  -LIBPATH:"$(MOZ_DIRECTX_SDK_PATH)/lib/$(MOZ_DIRECTX_SDK_CPU_SUFFIX)" \
> -  $(NULL)
> -OS_LIBS += $(call EXPAND_LIBNAME,secur32 crypt32 iphlpapi strmiids dmoguids wmcodecdspuuid amstrmid msdmo wininet)

I assume that if we actually needed any of this we'd get link errors with this removed.

Comment 4

[Makoto Kato [:m_kato]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #3)
> Comment on attachment 716970 [details] [diff] [review]
> fix
> 
> Review of attachment 716970 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: layout/media/Makefile.in
> @@ -152,5 @@
> > -ifdef MOZ_WEBRTC
> > -EXTRA_DSO_LDOPTS += \
> > -  -LIBPATH:"$(MOZ_DIRECTX_SDK_PATH)/lib/$(MOZ_DIRECTX_SDK_CPU_SUFFIX)" \
> > -  $(NULL)
> > -OS_LIBS += $(call EXPAND_LIBNAME,secur32 crypt32 iphlpapi strmiids dmoguids wmcodecdspuuid amstrmid msdmo wininet)
> 
> I assume that if we actually needed any of this we'd get link errors with
> this removed.

this is duplicated lines.  

http://mxr.mozilla.org/mozilla-central/source/layout/media/Makefile.in?mark=136-156#136

136 ifdef MOZ_WEBRTC
137 EXTRA_DSO_LDOPTS += \
138   -LIBPATH:"$(MOZ_DIRECTX_SDK_PATH)/lib/$(MOZ_DIRECTX_SDK_CPU_SUFFIX)" \
139   $(NULL)
140 OS_LIBS += $(call EXPAND_LIBNAME,secur32 crypt32 iphlpapi strmiids dmoguids wmcodecdspuuid amstrmid msdmo wininet)
141 ifdef _MSC_VER
142 OS_LIBS += $(call EXPAND_LIBNAME,delayimp)
143 EXTRA_DSO_LDOPTS += \
144   -DELAYLOAD:msdmo.dll \
145   $(NULL)
146 endif
147 endif
148 
149 ifdef MOZ_WEBRTC
150 EXTRA_DSO_LDOPTS += \
151   -LIBPATH:"$(MOZ_DIRECTX_SDK_PATH)/lib/$(MOZ_DIRECTX_SDK_CPU_SUFFIX)" \
152   $(NULL)
153 OS_LIBS += $(call EXPAND_LIBNAME,secur32 crypt32 iphlpapi strmiids dmoguids wmcodecdspuuid amstrmid msdmo wininet)
154 endif
155 DEFFILE = symbols.def
156 endif

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

Is this patch ready to check in?  If so, mark the patch checkin-needed (you can target it to me)

Comment 6

[Makoto Kato [:m_kato]]

Ah, I'm sorry.  I will land this soon.

Comment 7

[Makoto Kato [:m_kato]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fcc9af890fed

Comment 8

[Joe Drew (not getting mail)]

https://hg.mozilla.org/mozilla-central/rev/fcc9af890fed