Bug 82265

Summary: [chromium] Add isInputThrottled/didBecomeReadyForAddionalInput to WebWidget to fix input throttling
Product: WebKit Reporter: Nat Duca <nduca>
Component: New BugsAssignee: Nat Duca <nduca>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, cc-bugs, dglazkov, fishd, jamesr, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 80977    
Attachments:
Description Flags
Patch
none
Patch
none
oops jamesr: review+

Description Nat Duca 2012-03-26 17:09:09 PDT
[chromium] Fix input flow control in threaded compositing mode
Comment 1 Nat Duca 2012-03-26 17:09:45 PDT
Created attachment 133933 [details]
Patch
Comment 2 Nat Duca 2012-03-26 17:11:04 PDT
This is a temporary fix due to M19 timing and this causing jank there. We will fix this properly with 80977
Comment 3 WebKit Review Bot 2012-03-26 17:14:30 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-03-26 17:14:54 PDT
Attachment 133933 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/Platform/ChangeLog', u'Source/Platf..." exit_code: 1
Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 James Robinson 2012-03-26 17:22:18 PDT
Comment on attachment 133933 [details]
Patch

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

> Source/WebKit/chromium/ChangeLog:3
> +        [chromium] Fix input flow control in threaded compositing mode

this isn't terribly descriptive - can you change the title to describe what this patch actually has and put some prose in the body of the ChangeLog describing the expected semantics / flow of these calls?

> Source/WebCore/platform/graphics/chromium/cc/CCSingleThreadProxy.cpp:241
> +    ASSERT_NOT_REACHED();

what's keeping this from being hit? is the expectation that chromium-side code will only call WebWidget::isInputThrottled() if it is not handling scheduling (i.e. WebWidgetHandlesCompositorScheduling() is true)? if so you should document that on WebWidget. Might be easier to just have this return false and have the chromium code unconditionally check

> Source/WebKit/chromium/public/WebWidgetClient.h:77
> +    // FIXME: Make pure virtual once upstream deps are satisfied.

it doesn't look like anything else in this file is pure virtual
Comment 6 Nat Duca 2012-03-26 17:31:53 PDT
Created attachment 133938 [details]
Patch
Comment 7 Nat Duca 2012-03-26 17:35:47 PDT
Created attachment 133939 [details]
oops
Comment 8 James Robinson 2012-03-26 18:24:21 PDT
Comment on attachment 133939 [details]
oops

Cool. R=me
Comment 9 Nat Duca 2012-03-26 19:01:58 PDT
Committed r112191: <http://trac.webkit.org/changeset/112191>