Bug 111146

Summary: REGRESSION(r144398): put WebTestProxy::scheduleComposite back in place
Product: WebKit Reporter: jochen
Component: New BugsAssignee: jochen
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dglazkov, fishd, jamesr, tkent+wkapi, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 111017    
Attachments:
Description Flags
Patch
none
Patch for landing none

Description jochen 2013-02-28 23:36:23 PST
[chromium] put WebTestProxy::scheduleComposite back in place
Comment 1 jochen 2013-02-28 23:38:31 PST
Created attachment 190892 [details]
Patch
Comment 2 WebKit Review Bot 2013-02-28 23:40:59 PST
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 3 jochen 2013-02-28 23:43:16 PST
broke almost all compositing tests on content_shell
Comment 4 Nico Weber 2013-03-01 02:16:49 PST
Comment on attachment 190892 [details]
Patch

Can you mention where this was removed in the changelog?
Comment 5 jochen 2013-03-01 02:20:41 PST
Created attachment 190913 [details]
Patch for landing
Comment 6 jochen 2013-03-01 02:21:29 PST
Comment on attachment 190913 [details]
Patch for landing

Clearing flags on attachment: 190913

Committed r144433: <http://trac.webkit.org/changeset/144433>
Comment 7 jochen 2013-03-01 02:21:33 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 James Robinson 2013-03-01 11:20:21 PST
This isn't a terribly good solution.  The basic problem here appears to be that WebTestProxy wants to intercept a call (scheduleComposite) that otherwise has no reason to go through any WebKit APIs.  I resolved this for DumpRenderTree by hooking a special call (DRTLayerTreeViewClient::ScheduleComposite) to DRT's test harness.  It looks like to maintain this behavior in content shell I'll have to hook up a call to WebTextProxy from content::RenderWidget somehow.  Is there an established pattern for this?

Having this WebTextProxy stuff live in the WebKit repo but need access to things that happen normally only inside the WebKit embedder is a big pain for refactors like this :(
Comment 9 jochen 2013-03-01 12:12:56 PST
(In reply to comment #8)
> This isn't a terribly good solution.  The basic problem here appears to be that WebTestProxy wants to intercept a call (scheduleComposite) that otherwise has no reason to go through any WebKit APIs.  I resolved this for DumpRenderTree by hooking a special call (DRTLayerTreeViewClient::ScheduleComposite) to DRT's test harness.  It looks like to maintain this behavior in content shell I'll have to hook up a call to WebTextProxy from content::RenderWidget somehow.  Is there an established pattern for this?

not really. I read through the code, and I think having the method on WebTestProxy is actually the cleanest solution. it's the only solution where only when running layout tests extra code is executed.

If we're adding some test code to renderwidget, we'd always execute the check whether we're in a test or not.

> 
> Having this WebTextProxy stuff live in the WebKit repo but need access to things that happen normally only inside the WebKit embedder is a big pain for refactors like this :(

Having it live in the content module would make it a big pain for any change to layout tests
Comment 10 James Robinson 2013-03-01 14:38:14 PST
Jochen and I chatted some more and I think we've sorted this out.  We'll keep scheduleComposite() on WebTestProxy and on RenderWidget and just remove it from WebWidgetClient.