Bug 100013 - [chromium] Expose setAutomaticallyComputeRasterScale on the WebLayer API
Summary: [chromium] Expose setAutomaticallyComputeRasterScale on the WebLayer API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dana Jansens
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-22 10:33 PDT by Dana Jansens
Modified: 2012-10-23 07:51 PDT (History)
8 users (show)

See Also:


Attachments
Patch (1.76 KB, patch)
2012-10-22 10:34 PDT, Dana Jansens
no flags Details | Formatted Diff | Diff
Patch (3.05 KB, patch)
2012-10-22 12:58 PDT, Dana Jansens
no flags Details | Formatted Diff | Diff
Patch (2.57 KB, patch)
2012-10-22 13:21 PDT, Dana Jansens
no flags Details | Formatted Diff | Diff
Patch (2.60 KB, patch)
2012-10-22 15:48 PDT, Dana Jansens
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dana Jansens 2012-10-22 10:33:43 PDT
[chromium] Expose setInitialCssScale on the WebLayer API
Comment 1 Dana Jansens 2012-10-22 10:34:31 PDT
Created attachment 169937 [details]
Patch
Comment 2 Dana Jansens 2012-10-22 10:37:14 PDT
This depends on https://codereview.chromium.org/11227031/ and blocks relanding https://codereview.chromium.org/10915313
Comment 3 WebKit Review Bot 2012-10-22 10:38:31 PDT
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 4 WebKit Review Bot 2012-10-22 10:43:07 PDT
Comment on attachment 169937 [details]
Patch

Attachment 169937 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14486577
Comment 5 Peter Beverloo (cr-android ews) 2012-10-22 11:05:24 PDT
Comment on attachment 169937 [details]
Patch

Attachment 169937 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/14489580
Comment 6 Dana Jansens 2012-10-22 12:58:03 PDT
Created attachment 169960 [details]
Patch
Comment 7 Dana Jansens 2012-10-22 12:59:12 PDT
updated to opt-in
Comment 8 Adrienne Walker 2012-10-22 13:06:20 PDT
Comment on attachment 169960 [details]
Patch

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

> Source/Platform/chromium/public/WebLayer.h:108
> +    virtual void setAutomaticallyComputeRasterScale(bool) = 0;

I feel like this is more of a WebContentLayer concern than a general WebLayer one.
Comment 9 WebKit Review Bot 2012-10-22 13:14:09 PDT
Comment on attachment 169960 [details]
Patch

Attachment 169960 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14514001
Comment 10 Peter Beverloo (cr-android ews) 2012-10-22 13:18:21 PDT
Comment on attachment 169960 [details]
Patch

Attachment 169960 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/14516001
Comment 11 Dana Jansens 2012-10-22 13:21:54 PDT
Created attachment 169965 [details]
Patch

Moved to WebContentLayer
Comment 12 WebKit Review Bot 2012-10-22 13:39:57 PDT
Comment on attachment 169965 [details]
Patch

Attachment 169965 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14487555
Comment 13 Peter Beverloo (cr-android ews) 2012-10-22 13:59:36 PDT
Comment on attachment 169965 [details]
Patch

Attachment 169965 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/14496378
Comment 14 James Robinson 2012-10-22 15:16:15 PDT
Comment on attachment 169965 [details]
Patch

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

> Source/Platform/chromium/public/WebContentLayer.h:54
> +    virtual void setAutomaticallyComputeRasterScale(bool) = 0;

If you provide a default implementation instead of having it be pure virtual here landing will be a lot easier. 

Please document the default (false, I presume)
Comment 15 Dana Jansens 2012-10-22 15:24:17 PDT
Comment on attachment 169965 [details]
Patch

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

>> Source/Platform/chromium/public/WebContentLayer.h:54
>> +    virtual void setAutomaticallyComputeRasterScale(bool) = 0;
> 
> If you provide a default implementation instead of having it be pure virtual here landing will be a lot easier. 
> 
> Please document the default (false, I presume)

Oh, sure. 2 patches instead of 1. Or will I need to come back here and remove the default implementation after?

False, yep. k.
Comment 16 James Robinson 2012-10-22 15:42:56 PDT
(In reply to comment #15)
> (From update of attachment 169965 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=169965&action=review
> 
> >> Source/Platform/chromium/public/WebContentLayer.h:54
> >> +    virtual void setAutomaticallyComputeRasterScale(bool) = 0;
> > 
> > If you provide a default implementation instead of having it be pure virtual here landing will be a lot easier. 
> > 
> > Please document the default (false, I presume)
> 
> Oh, sure. 2 patches instead of 1. Or will I need to come back here and remove the default implementation after?

Wouldn't necessarily need to. It'd be good to be consistent, but might be better to just have all of these have default impls to make changing other APIs easier as well.  Even if you did you wouldn't have to worry about synchronizing it with anything else.
Comment 17 Dana Jansens 2012-10-22 15:48:28 PDT
Created attachment 170008 [details]
Patch

Default impl and document return value
Comment 18 WebKit Review Bot 2012-10-23 07:51:40 PDT
Comment on attachment 170008 [details]
Patch

Clearing flags on attachment: 170008

Committed r132223: <http://trac.webkit.org/changeset/132223>
Comment 19 WebKit Review Bot 2012-10-23 07:51:45 PDT
All reviewed patches have been landed.  Closing bug.