media_Bench and ffmpeg_tests disable exception handler by default...

media_Bench and ffmpeg_tests disable exception handler by default
BUG=47307
TEST=no change in normal behavior.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=50684

[fbarchard, 2010-06-23 18:36:17]

Disable exception handling by default.
user must manually reenable it to build tools that capture exceptions.

[scherkus (not reviewing), 2010-06-23 18:48:26]

LGTM but what's the cause for the change?

[Victor Wang, 2010-06-23 20:42:41]

http://codereview.chromium.org/2811026/diff/1/2
File media/test/ffmpeg_tests/ffmpeg_tests.cc (right):

http://codereview.chromium.org/2811026/diff/1/2#newcode38
media/test/ffmpeg_tests/ffmpeg_tests.cc:38: #pragma warning(disable:4509)
Think this warning is also related to the compiler error (C2712) I am seeing
when enabling exception. Looks to me, if "Enable C++ Exceptions" is "No", you
get this compiler warning (C4509); otherwise, you get compiler error C2712.

Think a better solution is refactoring the code to not use objects with
destructors inside SEH. This should fix both C4509 and C2712.

[fbarchard, 2010-06-23 22:36:12]

Remove pragma when not using exception handling.

http://codereview.chromium.org/2811026/diff/1/2
File media/test/ffmpeg_tests/ffmpeg_tests.cc (right):

http://codereview.chromium.org/2811026/diff/1/2#newcode38
media/test/ffmpeg_tests/ffmpeg_tests.cc:38: #pragma warning(disable:4509)
Which version of Visual C are you using?

[Victor Wang, 2010-06-23 22:40:51]

http://codereview.chromium.org/2811026/diff/1/2
File media/test/ffmpeg_tests/ffmpeg_tests.cc (right):

http://codereview.chromium.org/2811026/diff/1/2#newcode38
media/test/ffmpeg_tests/ffmpeg_tests.cc:38: #pragma warning(disable:4509)
On 2010/06/23 22:36:12, fbarchard wrote:
> Which version of Visual C are you using?
MSVC 2005 Version 8.0.50727.867.

[fbarchard, 2010-06-23 23:20:12]

I'm using 2008, so its possible the warning number changed.
Will the change as is, solve your problem?

[Victor Wang, 2010-06-23 23:24:18]

On 2010/06/23 23:20:12, fbarchard wrote:
> I'm using 2008, so its possible the warning number changed.
> Will the change as is, solve your problem?

Do you see any compiler error if you enable C++ exception (/EHa or /EHsc)? Your
change disables __try by default so I am sure it works for me.

[fbarchard, 2010-06-24 00:01:46]

Yes, I'm able to produce an error if I hand compile it

c:\src\chromium\src\media\test\ffmpeg_tests>cl /EHsc
/DENABLE_WINDOWS_EXCEPTIONS=1 /I ../../.. /I ..\..\..\third_party\ffmpeg\includ
e\win /I ..\..\..\third_party\ffmpeg\patched-ffmpeg-mt /I
..\..\..\third_party\ffmpeg\config /c ffmpeg_tests.cc
Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 15.00.30729.01 for 80x86
Copyright (C) Microsoft Corporation.  All rights reserved.

ffmpeg_tests.cc
../../..\base/observer_list.h(84) : warning C4003: not enough actual parameters
for macro 'max'
ffmpeg_tests.cc(493) : error C2712: Cannot use __try in functions that require
object unwinding

[Victor Wang, 2010-06-24 00:06:24]

That's what I got if I enable c++ exception for multi dll build...

On 2010/06/24 00:01:46, fbarchard wrote:
> Yes, I'm able to produce an error if I hand compile it
> 
> c:\src\chromium\src\media\test\ffmpeg_tests>cl /EHsc
> /DENABLE_WINDOWS_EXCEPTIONS=1 /I ../../.. /I
..\..\..\third_party\ffmpeg\includ
> e\win /I ..\..\..\third_party\ffmpeg\patched-ffmpeg-mt /I
> ..\..\..\third_party\ffmpeg\config /c ffmpeg_tests.cc
> Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 15.00.30729.01 for
80x86
> Copyright (C) Microsoft Corporation.  All rights reserved.
> 
> ffmpeg_tests.cc
> ../../..\base/observer_list.h(84) : warning C4003: not enough actual
parameters
> for macro 'max'
> ffmpeg_tests.cc(493) : error C2712: Cannot use __try in functions that require
> object unwinding

[fbarchard, 2010-06-24 00:36:57]

Regular chromium builds work though.

[Victor Wang, 2010-06-24 00:49:27]

On 2010/06/24 00:36:57, fbarchard wrote:
> Regular chromium builds work though.

The current chromium build (linked static libraries) disables c++ exception. I
am working on multi dll chromium build and enable the c++ exception
(http://src.chromium.org/viewvc/chrome?view=rev&revision=50362), so this will
fail with multi dll version once we enable it. I think it would be ideal to
refactor the code so it compiles without error (c2712) or warning (c4509) in
both cases (c++ exception enabled or disabled). Of course, disabling __try like
what you did also works if you think it is not critical to have them for
multi-dll version.