Bug 135919 - Add LayoutUnit argument coders to WebCoreArgumentCoders
Summary: Add LayoutUnit argument coders to WebCoreArgumentCoders
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Enhancement
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-08-13 17:43 PDT by Wenson Hsieh
Modified: 2014-08-13 23:07 PDT (History)
2 users (show)

See Also:


Attachments
Patch (3.05 KB, patch)
2014-08-13 20:05 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Patch (3.05 KB, patch)
2014-08-13 21:23 PDT, Wenson Hsieh
ap: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2014-08-13 17:43:20 PDT
Related to the encoding/decoding of snap offsets. LayoutUnits currently lack an implementation in WebCoreArgumentCoders.
Comment 1 Wenson Hsieh 2014-08-13 20:05:39 PDT
Created attachment 236574 [details]
Patch
Comment 2 Tim Horton 2014-08-13 21:07:08 PDT
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.
Comment 3 Wenson Hsieh 2014-08-13 21:23:42 PDT
Created attachment 236576 [details]
Patch
Comment 4 Tim Horton 2014-08-13 21:54:33 PDT
I think we decided that you didn't need to do this?
Comment 5 Alexey Proskuryakov 2014-08-13 22:59:11 PDT
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.
Comment 6 Tim Horton 2014-08-13 23:03:49 PDT
Indeed, and we (wenson/zalan mostly) already established that he doesn't need to do this anyway.
Comment 7 Wenson Hsieh 2014-08-13 23:07:10 PDT
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.