Bug 135919 – Add LayoutUnit argument coders to WebCoreArgumentCoders

• Component: WebKit2
• Status: RESOLVED INVALID

[Wenson Hsieh]

Related to the encoding/decoding of snap offsets. LayoutUnits currently lack an implementation in WebCoreArgumentCoders.

[Wenson Hsieh]

Created attachment 236574 [details]
Patch

[Tim Horton]

Comment on attachment 236574 [details]
Patch

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

> Source/WebKit2/Shared/WebCoreArgumentCoders.cpp:338
> +    encoder << layoutUnit.toDouble();

Why toDouble instead of encoding raw LayoutUnit numerator/denominator? Is that safe? If so, why? That information should be in the changelog.

[Wenson Hsieh]

Created attachment 236576 [details]
Patch

[Tim Horton]

I think we decided that you didn't need to do this?

[Alexey Proskuryakov]

Comment on attachment 236576 [details]
Patch

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

r- for using int.

> Source/WebKit2/ChangeLog:3
> +        Add LayoutUnit argument coders to WebCoreArgumentCoders

It seems a little bit strange to expose internals of layout machinery even to WebKit, and more so to UI process. Can we use whatever type is exposed in API until we get down to WebCore?

> Source/WebKit2/Shared/WebCoreArgumentCoders.cpp:343
> +    int decodedRawValue;

IPC always uses fixed size types, like uint64_t, not int.

[Tim Horton]

Indeed, and we (wenson/zalan mostly) already established that he doesn't need to do this anyway.

[Wenson Hsieh]

Comment on attachment 236576 [details]
Patch

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

Thanks for taking a look at this! However, after talking with Tim and Zalan, we've decided against IPC-ing LayoutUnit (if we ever do decide to do it though, we'll keep that in mind)

>> Source/WebKit2/Shared/WebCoreArgumentCoders.cpp:343
>> +    int decodedRawValue;
> 
> IPC always uses fixed size types, like uint64_t, not int.

Got it -- thanks for the tip.