Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(28)

Issue 10227004: RenderWidget's scheduleComposite should return a bool. (true iff a prompt composite will come). (Closed)

Created:
8 years, 8 months ago by Ian Vollick
Modified:
8 years, 7 months ago
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam
Visibility:
Public.

Description

RenderWidget should implement tryScheduleComposite rather than scheduleComposite. scheduleComposite has been deprecated. We now need to return a boolean indicating whether or not scheduling the composite was successful. In the multi-threaded case, this is always successful for RenderWidget. In the singe-threaded case, it is unsuccessful when we are hidden. Depends on: https://bugs.webkit.org/show_bug.cgi?id=84620 BUG=124823 TEST=Accelerated animations (e.g., poster circle) continue to tick when a tab is backgrounded.

Patch Set 1 #

Patch Set 2 : . #

Total comments: 2

Patch Set 3 : . #

Patch Set 4 : Fix aura build #

Patch Set 5 : . #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+59 lines, -12 lines) Patch
M content/renderer/render_widget.h View 1 2 3 4 2 chunks +10 lines, -0 lines 0 comments Download
M content/renderer/render_widget.cc View 1 2 3 4 3 chunks +34 lines, -11 lines 1 comment Download
M ui/gfx/compositor/compositor.h View 1 2 3 1 chunk +5 lines, -0 lines 0 comments Download
M ui/gfx/compositor/compositor.cc View 1 2 3 1 chunk +10 lines, -1 line 0 comments Download

Messages

Total messages: 9 (0 generated)
darin (slow to review)
Not sure why you need this. WebViewImpl already knows if we are hidden. Should be ...
8 years, 8 months ago (2012-04-26 05:17:41 UTC) #1
Ian Vollick
On 2012/04/26 05:17:41, darin wrote: > Not sure why you need this. WebViewImpl already knows ...
8 years, 8 months ago (2012-04-26 12:16:00 UTC) #2
Ian Vollick
Ditching bool tryScheduleComposite() in favor of bool scheduleComposite().
8 years, 8 months ago (2012-04-27 18:23:22 UTC) #3
nduca
https://chromiumcodereview.appspot.com/10227004/diff/7001/content/renderer/render_widget.cc File content/renderer/render_widget.cc (right): https://chromiumcodereview.appspot.com/10227004/diff/7001/content/renderer/render_widget.cc#newcode1165 content/renderer/render_widget.cc:1165: bool RenderWidget::scheduleComposite() { Can you fix this with less ...
8 years, 7 months ago (2012-05-02 06:53:00 UTC) #4
Ian Vollick
On 2012/05/02 06:53:00, nduca wrote: > https://chromiumcodereview.appspot.com/10227004/diff/7001/content/renderer/render_widget.cc > File content/renderer/render_widget.cc (right): > > https://chromiumcodereview.appspot.com/10227004/diff/7001/content/renderer/render_widget.cc#newcode1165 > ...
8 years, 7 months ago (2012-05-02 18:58:12 UTC) #5
Ian Vollick
+ben for OWNERS > > > https://chromiumcodereview.appspot.com/10227004/diff/7001/content/renderer/render_widget.cc#newcode1178 > > content/renderer/render_widget.cc:1178: return !is_hidden_; > > This ...
8 years, 7 months ago (2012-05-08 19:34:43 UTC) #6
Ben Goodger (Google)
Can you get a positive review from someone more qualified to review this before asking ...
8 years, 7 months ago (2012-05-08 19:37:53 UTC) #7
jamesr
On 2012/04/26 12:16:00, vollick wrote: > On 2012/04/26 05:17:41, darin wrote: > > Not sure ...
8 years, 7 months ago (2012-05-08 23:42:04 UTC) #8
jamesr
8 years, 7 months ago (2012-05-08 23:42:40 UTC) #9
https://chromiumcodereview.appspot.com/10227004/diff/18002/content/renderer/r...
File content/renderer/render_widget.cc (right):

https://chromiumcodereview.appspot.com/10227004/diff/18002/content/renderer/r...
content/renderer/render_widget.cc:1219: return CanDoDeferredUpdate() &&
!ShouldSuppressDeferredUpdates();
what happens if ShouldSuppress...() is false, but when the DoDeferredUpdate
actually runs ShouldSuppress..() has become true (and stays that way)? how do we
make sure that WebKit knows about this state transition?

Powered by Google App Engine
This is Rietveld 408576698