Bug 133236

Summary: [GStreamer] Add missing <cmath> include
Product: WebKit Reporter: Adrian Perez <aperez>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: agomez, buildbot, calvaris, cgarcia, commit-queue, gustavo, menard, mrobinson, pnormand, rniwa, vjaquez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2
none
Patch none

Description Adrian Perez 2014-05-23 14:36:22 PDT
[GStreamer] Add missing <cmath> include
Comment 1 Adrian Perez 2014-05-23 14:38:17 PDT
Created attachment 231990 [details]
Patch
Comment 2 WebKit Commit Bot 2014-05-23 14:39:33 PDT
Attachment 231990 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/gstreamer/GStreamerUtilities.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Adrian Perez 2014-05-23 14:49:24 PDT
Created attachment 231992 [details]
Patch
Comment 4 Martin Robinson 2014-05-23 14:54:22 PDT
Comment on attachment 231990 [details]
Patch

It might be better to use wtf/MathExtras.h here.
Comment 5 Build Bot 2014-05-23 15:54:13 PDT
Comment on attachment 231992 [details]
Patch

Attachment 231992 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5020357526290432

New failing tests:
media/W3C/video/networkState/networkState_during_loadstart.html
Comment 6 Build Bot 2014-05-23 15:54:16 PDT
Created attachment 231998 [details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 7 Adrian Perez 2014-05-24 02:11:51 PDT
Created attachment 232018 [details]
Patch
Comment 8 Adrian Perez 2014-05-24 02:14:04 PDT
(In reply to comment #4)
> (From update of attachment 231990 [details])
> It might be better to use wtf/MathExtras.h here.

Changed, thanks for the tip!
Comment 9 Philippe Normand 2014-05-24 02:14:38 PDT
(In reply to comment #4)
> (From update of attachment 231990 [details])
> It might be better to use wtf/MathExtras.h here.

Well MathExtras.h unconditionally includes cmath and we don't use any of its functions. So not sure it's really worth
Comment 10 Martin Robinson 2014-05-24 10:13:32 PDT
(In reply to comment #9)
> (In reply to comment #4)
> > (From update of attachment 231990 [details] [details])
> > It might be better to use wtf/MathExtras.h here.
> 
> Well MathExtras.h unconditionally includes cmath and we don't use any of its functions. So not sure it's really worth

If I recall correctly, it takes care of standardizing cmath across different platforms.
Comment 11 Adrian Perez 2014-05-26 03:00:33 PDT
(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #4)
> > > (From update of attachment 231990 [details] [details] [details])
> > > It might be better to use wtf/MathExtras.h here.
> > 
> > Well MathExtras.h unconditionally includes cmath and we don't use any of its functions. So not sure it's really worth
> 
> If I recall correctly, it takes care of standardizing cmath across different platforms.

I have checked this, it just includes <cmath> unconditionally, and for
modf/modff it does not have any platform-dependant fixes. Anyway, I think
it is better to include <MathExtras.h>, to avoid issues later on if
GStreamerUtilitiers.cpp starts using functions for which there are fixes
(and avoid subtle bugs).
Comment 12 WebKit Commit Bot 2014-05-27 03:56:59 PDT
Comment on attachment 232018 [details]
Patch

Clearing flags on attachment: 232018

Committed r169378: <http://trac.webkit.org/changeset/169378>
Comment 13 WebKit Commit Bot 2014-05-27 03:57:05 PDT
All reviewed patches have been landed.  Closing bug.