Bug 165923 – Arguments called in wrong order

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Brent Fulgham]

Static analysis has identified several places where the order of arguments to functions does not match the expected signature.

[Brent Fulgham]

I corrected a few other small bugs at the same time.

[Brent Fulgham]

Created attachment 297257 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 297257 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=297257&action=review

> Source/WebCore/ChangeLog:3
> +        Arguments called in wrong order

That doesn't really cover all the problems.

> Source/WebCore/platform/graphics/GraphicsContext3D.cpp:332
> -    if (!computeFormatAndTypeParameters(format, type, &bytesPerComponent, &componentsPerPixel))
> +    if (!computeFormatAndTypeParameters(format, type, &componentsPerPixel, &bytesPerComponent))

It seems like this should be testable.

[Tim Horton]

Comment on attachment 297257 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=297257&action=review

> Tools/TestWebKitAPI/Tests/WebKit2/UserMedia.cpp:54
> -        WKUserMediaPermissionRequestAllow(permissionRequest, videoDeviceUID.get(), audioDeviceUID.get());
> +        WKUserMediaPermissionRequestAllow(permissionRequest, audioDeviceUID.get(), videoDeviceUID.get());

Seems bad that the test didn't catch this?

[Brent Fulgham]

Comment on attachment 297257 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=297257&action=review

>> Source/WebCore/platform/graphics/GraphicsContext3D.cpp:332
>> +    if (!computeFormatAndTypeParameters(format, type, &componentsPerPixel, &bytesPerComponent))
> 
> It seems like this should be testable.

It's actually not, because the only use of 'bytesPerComponent' and 'componentsPerPixel' is to multiply by each-other and use that result.

So this change is more about correctness than any actual behavior problem.

I kind of wonder why we bother returning both values, since they are only ever used to do this one calculation.

>> Tools/TestWebKitAPI/Tests/WebKit2/UserMedia.cpp:54
>> +        WKUserMediaPermissionRequestAllow(permissionRequest, audioDeviceUID.get(), videoDeviceUID.get());
> 
> Seems bad that the test didn't catch this?

Agreed!

[Brent Fulgham]

Committed r209924: <http://trac.webkit.org/changeset/209924>

[David Kilzer (:ddkilzer)]

Antti:  FYI, note the cache validation fix in Source/WebCore/platform/network/CacheValidation.cpp.