Execute all GL commands up to the put offset reported by a flush.

Execute all GL commands up to the put offset reported by a each flush.This means glFlush is a barrier that prevents reordering of GL commands issued on different command buffers. I used it to replace latches for synchronizing the rendering of WebGL canvas and Pepper 3D with the accelerated compositor. The primary advantage is it is more robust than latches and there is no possibility of deadlock. It should also be possible for WebGL and Pepper 3D to use it whereas exposing SetLatch and WaitLatch would be dangerous.The calls to SetLatch and WaitLatch are still in webkit but they are no-ops. SetLatch and WaitLatch are completely removed elsewhere.I changed CommandBuffer::FlushSync to Finish to reflect the new semantics. Going forward, I will add a synchronous CommandBuffer::WaitForToken and WaitForAvailableEntries, which should eliminate the need to call Finish unless glFinish is called by the client. The Pepper interface is unchanged because I don't want to break binary compatibility.I fixed a bug where the last read token in CmdBufferHelper was stale after receiving a ReportState IPC. That was causing a redundant synchronous flush in the client side SwapBuffers throttling.I removed Yield because it does not make sense with the new semantics. There is no round robin scheduling.Tested with WebGL on Windows and Mac and checked that 72672 did not regress.
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=93066

[apatrick_chromium, 2011-07-01 01:30:25]

Scary review.

[piman, 2011-07-01 01:57:17]

On Thu, Jun 30, 2011 at 6:30 PM, <apatrick@chromium.org> wrote:

> Reviewers: jbates, piman,
>
> Message:
> Scary review.
>
> Description:
> Execute all GL commands up to the put offset reported by a each flush.
>
> This means glFlush is a barrier that prevents reordering of GL commands
> issued
> on different command buffers. I used it to replace latches for
> synchronizing the
> rendering of WebGL canvas and Pepper 3D with the accelerated compositor.
> The
> primary advantage is it is more robust than latches and there is no
> possibility
> of deadlock. It should also be possible for WebGL and Pepper 3D to use it
> whereas exposing SetLatch and WaitLatch would be dangerous.
>
> The calls to SetLatch and WaitLatch are still in webkit but they are
> no-ops.
> SetLatch and WaitLatch are completely removed elsewhere.
>
> I changed CommandBuffer::FlushSync to Finish to reflect the new semantics.
> Going
> forward, I will add a synchronous CommandBuffer::WaitForToken and
> WaitForAvailableEntries, which should eliminate the need to call Finish
> unless
> glFinish is called by the client. The Pepper interface is unchanged because
> I
> don't want to break binary compatibility.
>
> I fixed a bug where the last read token in CmdBufferHelper was stale after
> receiving a ReportState IPC. That was causing a redundant synchronous flush
> in
> the client side SwapBuffers throttling.
>
> I removed Yield because it does not make sense with the new semantics.
> There is
> no round robin scheduling.
>
> Tested with WebGL on Windows and Mac and checked that 72672 did not
> regress.
>

I will look at the change, I think it's a good change, but before I dive
deeply, some high level questions/comments:
- I'm pretty sure replacing FlushSync by Finish will be a performance hit on
Pepper. FlushSync is (unfortunately) still called every other frame from
Pepper, but returns early, making it not horribly bad. With the change,
we'll see a significant perf regression, until WaitForToken gets
implemented. Can you please file a ReleaseBlocker-Dev bug to get this
resolved ? Or keep the last_known_get/early-return and yield behaviors
(shouldn't be too difficult and would solve that issue) ?
- Not doing the round-robin thing means a lot more latency for IPCs such as
CreateTransferBuffer etc., whether or not WaitForGet etc. get implemented,
for workloads with many commands (e.g. webgl) as opposed to few
(compositor). Have we measured the perf impact of that ? Ideally a lot of
those could be handled on the IPC thread to avoid the issue, but that's
significant refactoring.
- Similarly, it sounds like in the case of a background page sending a lot
of work, the change would increase latency for the foreground pages. Have we
considered doing a round-robin between unrelated contexts (but doing strict
ordering for related ones) ?

Antoine


>
> Please review this at
http://codereview.chromium.**org/7253052/<http://codereview.chromium.org/7253...
>
> SVN Base: svn://chrome-svn/chrome/trunk/**src/
>
> Affected files:
>  M     content/common/gpu/gpu_**channel.h
>  M     content/common/gpu/gpu_**channel.cc
>  M     content/common/gpu/gpu_**command_buffer_stub.h
>  M     content/common/gpu/gpu_**command_buffer_stub.cc
>  M     content/common/gpu/gpu_**messages.h
>  M     content/renderer/gpu/command_**buffer_proxy.h
>  M     content/renderer/gpu/command_**buffer_proxy.cc
>  M     content/renderer/gpu/**webgraphicscontext3d_command_**
> buffer_impl.cc
>  M     gpu/command_buffer/build_**gles2_cmd_buffer.py
>  M     gpu/command_buffer/client/cmd_**buffer_helper.h
>  M     gpu/command_buffer/client/cmd_**buffer_helper.cc
>  M     gpu/command_buffer/client/cmd_**buffer_helper_test.cc
>  M     gpu/command_buffer/client/**fenced_allocator_test.cc
>  M     gpu/command_buffer/client/**gles2_c_lib_autogen.h
>  M     gpu/command_buffer/client/**gles2_cmd_helper_autogen.h
>  M     gpu/command_buffer/client/**gles2_demo.cc
>  M     gpu/command_buffer/client/**gles2_implementation.cc
>  M     gpu/command_buffer/client/**gles2_implementation_autogen.h
>  M     gpu/command_buffer/client/**gles2_implementation_unittest.**cc
>  M     gpu/command_buffer/client/**mapped_memory_unittest.cc
>  M     gpu/command_buffer/client/**ring_buffer_test.cc
>  M     gpu/command_buffer/common/cmd_**buffer_common.h
>  M     gpu/command_buffer/common/**command_buffer.h
>  M     gpu/command_buffer/common/**command_buffer_mock.h
>  M     gpu/command_buffer/common/**constants.h
>  M     gpu/command_buffer/common/**gles2_cmd_format_autogen.h
>  M     gpu/command_buffer/common/**gles2_cmd_format_test_autogen.**h
>  M     gpu/command_buffer/common/**gles2_cmd_ids_autogen.h
>  M     gpu/command_buffer/common/**gles2_cmd_utils_**
> implementation_autogen.h
>  M     gpu/command_buffer/service/**cmd_parser.cc
>  M     gpu/command_buffer/service/**cmd_parser_test.cc
>  M     gpu/command_buffer/service/**command_buffer_service.h
>  M     gpu/command_buffer/service/**command_buffer_service.cc
>  M     gpu/command_buffer/service/**common_decoder.cc
>  M     gpu/command_buffer/service/**common_decoder_unittest.cc
>  M     gpu/command_buffer/service/**gles2_cmd_decoder.h
>  M     gpu/command_buffer/service/**gles2_cmd_decoder.cc
>  M     gpu/command_buffer/service/**gles2_cmd_decoder_unittest.cc
>  M     gpu/command_buffer/service/**gles2_cmd_decoder_unittest_2_**
> autogen.h
>  M     gpu/command_buffer/service/**gles2_cmd_decoder_unittest_3_**
> autogen.h
>  M     gpu/command_buffer/service/**gpu_scheduler.h
>  M     gpu/command_buffer/service/**gpu_scheduler.cc
>  M     gpu/command_buffer/service/**gpu_scheduler_unittest.cc
>  M     gpu/command_buffer/service/**mocks.h
>  M     gpu/command_buffer/service/**mocks.cc
>  M     gpu/demos/framework/window.cc
>  M     gpu/gles2_conform_support/egl/**display.cc
>  M     ppapi/c/dev/ppb_opengles_dev.h
>  M     ppapi/lib/gl/gles2/gles2.c
>  M     ppapi/proxy/ppapi_messages.h
>  M     ppapi/proxy/ppb_context_3d_**proxy.h
>  M     ppapi/proxy/ppb_context_3d_**proxy.cc
>  M     ppapi/proxy/ppb_opengles2_**proxy.cc
>  M     webkit/gpu/**webgraphicscontext3d_in_**
> process_command_buffer_impl.cc
>  M     webkit/plugins/ppapi/ppb_**context_3d_impl.cc
>  M     webkit/plugins/ppapi/ppb_**opengles_impl.cc
>
>
>

[apatrick_chromium, 2011-07-01 20:46:15]

> I will look at the change, I think it's a good change, but before I dive
> deeply, some high level questions/comments:
> - I'm pretty sure replacing FlushSync by Finish will be a performance hit on
> Pepper. FlushSync is (unfortunately) still called every other frame from
> Pepper, but returns early, making it not horribly bad. With the change,
> we'll see a significant perf regression, until WaitForToken gets
> implemented. Can you please file a ReleaseBlocker-Dev bug to get this
> resolved ? Or keep the last_known_get/early-return and yield behaviors
> (shouldn't be too difficult and would solve that issue) ?

Okay I'll look into it. Was hoping to keep this change as simple as possible.

> - Not doing the round-robin thing means a lot more latency for IPCs such as
> CreateTransferBuffer etc., whether or not WaitForGet etc. get implemented,
> for workloads with many commands (e.g. webgl) as opposed to few
> (compositor). Have we measured the perf impact of that ? Ideally a lot of
> those could be handled on the IPC thread to avoid the issue, but that's
> significant refactoring.
> - Similarly, it sounds like in the case of a background page sending a lot
> of work, the change would increase latency for the foreground pages. Have we
> considered doing a round-robin between unrelated contexts (but doing strict
> ordering for related ones) ?

What I said about round robin wasn't true. It still round robins between
channels. So one unrelated page can still do useful work while another has
unscheduled its channel. Still, as things strand, an unrelated page cannot
preempt another and I think that is doable, though more complicated. I was
thinking though, going forward, that it might be better to have a separate
thread or process for each unrelated page. That gives preemption without more
complicated logic in the GPU process and it solves the vsync issue without
needing swap groups.

Al

[apatrick_chromium, 2011-07-07 23:38:58]

I think I got the flush semantics back as they were. I.e. flush is passed a last
known get offset and the sync IPC will return immediately if the client is
out-of-date. PTAL.

[nduca, 2011-07-08 19:52:10]

Are there any complications with this and the world where we have GL commands
being issued simultaneously on two threads? Am I reading this correctly that if
I issue a glFlush on the main thread, then any subsequent drawing on a
compositor-thread GL context will get "the state of the world" at that flush?

[apatrick_chromium, 2011-07-08 19:53:36]

On 2011/07/08 19:52:10, nduca wrote:
> Are there any complications with this and the world where we have GL commands
> being issued simultaneously on two threads? Am I reading this correctly that
if
> I issue a glFlush on the main thread, then any subsequent drawing on a
> compositor-thread GL context will get "the state of the world" at that flush?

Yes that should work even if the GL calls are being issued on different threads.
The glFlush would need to be synchronized of course.

[apatrick_chromium, 2011-07-08 19:55:31]

On 2011/07/08 19:53:36, apatrick_chromium wrote:
> On 2011/07/08 19:52:10, nduca wrote:
> > Are there any complications with this and the world where we have GL
commands
> > being issued simultaneously on two threads? Am I reading this correctly that
> if
> > I issue a glFlush on the main thread, then any subsequent drawing on a
> > compositor-thread GL context will get "the state of the world" at that
flush?
> 
> Yes that should work even if the GL calls are being issued on different
threads.
> The glFlush would need to be synchronized of course.

Actually, I need more info. Is the plan to have one GPU channel shared  by all
the threads and synchronize all the flushing of the threads, to have a separate
IPC channel for each thread, or something else?

[apatrick_chromium, 2011-07-08 23:47:04]

+kbr

[piman, 2011-07-09 20:40:22]

Overall, looks good, but I have a couple of comments.

Also one question: now that we block the entire GpuChannel if any context is
unscheduled, can there be a case where a background tab (on the same renderer,
i.e. same channel) can take an arbitrary long time to reschedule, blocking the
foreground tab? I guess that's a Mac-only problem, I don't know the mechanism
very well.

http://codereview.chromium.org/7253052/diff/23007/content/common/gpu/gpu_comm...
File content/common/gpu/gpu_command_buffer_stub.cc (right):

http://codereview.chromium.org/7253052/diff/23007/content/common/gpu/gpu_comm...
content/common/gpu/gpu_command_buffer_stub.cc:309:
task_factory_.NewRunnableMethod(&GpuCommandBufferStub::ReportState));
Because Flush will now fully execute the commands, you can now ReportState
immediately without posting a task (and remove the TODO).

http://codereview.chromium.org/7253052/diff/23007/gpu/command_buffer/client/g...
File gpu/command_buffer/client/gles2_implementation.cc (right):

http://codereview.chromium.org/7253052/diff/23007/gpu/command_buffer/client/g...
gpu/command_buffer/client/gles2_implementation.cc:827:
helper_->CommandBufferHelper::Flush();
It took me a while to figure this out, even with the comment...

So, IIUC, you need this because now when the service unschedules, it never goes
back to process remaining commands, until another Flush comes, breaking the
ordering semantics (if another Flush came on another context in the mean time).

I wonder if calling PutChanged from GpuScheduler::SetScheduled would solve that
problem, in a nicer way, because you wouldn't depend on the client to enforce
the semantics.

http://codereview.chromium.org/7253052/diff/23007/gpu/command_buffer/service/...
File gpu/command_buffer/service/gpu_scheduler.h (right):

http://codereview.chromium.org/7253052/diff/23007/gpu/command_buffer/service/...
gpu/command_buffer/service/gpu_scheduler.h:49: gles2::ContextGroup* group);
Because of the overload ambiguity between the 2 constructors, I wonder if this
one wouldn't be better served by a factory function, calling the other one. WDYT
?

[jbates, 2011-07-11 18:39:50]

Looks like a great cleanup. This brings us back closer to how GL is supposed to
work -- hmm, why didn't we do this in the first place?

http://codereview.chromium.org/7253052/diff/23007/gpu/command_buffer/client/c...
File gpu/command_buffer/client/cmd_buffer_helper.cc (right):

http://codereview.chromium.org/7253052/diff/23007/gpu/command_buffer/client/c...
gpu/command_buffer/client/cmd_buffer_helper.cc:139: while (AvailableEntries() <
count) {
Looks like this loop is no longer needed because of the call to Finish.

http://codereview.chromium.org/7253052/diff/23007/gpu/command_buffer/client/g...
File gpu/command_buffer/client/gles2_implementation.cc (right):

http://codereview.chromium.org/7253052/diff/23007/gpu/command_buffer/client/g...
gpu/command_buffer/client/gles2_implementation.cc:827:
helper_->CommandBufferHelper::Flush();
On 2011/07/09 20:40:22, piman wrote:
> It took me a while to figure this out, even with the comment...
> 
> So, IIUC, you need this because now when the service unschedules, it never
goes
> back to process remaining commands, until another Flush comes, breaking the
> ordering semantics (if another Flush came on another context in the mean
time).
> 
> I wonder if calling PutChanged from GpuScheduler::SetScheduled would solve
that
> problem, in a nicer way, because you wouldn't depend on the client to enforce
> the semantics.

I'm not sure why this Flush is needed. Why does getting unscheduled break
ordering? Sometime after this resize call, the client should issue a glFlush,
and that should maintain proper command ordering with respect to other command
buffers, right? If not, can we fix that so that regardless of
scheduled/unscheduled, client Flushes are always guaranteed to be completed in
order (unless an error occurs).

http://codereview.chromium.org/7253052/diff/23007/gpu/command_buffer/common/c...
File gpu/command_buffer/common/command_buffer.h (right):

http://codereview.chromium.org/7253052/diff/23007/gpu/command_buffer/common/c...
gpu/command_buffer/common/command_buffer.h:81: // sees the latest added
commands, and will eventually process them.
Could use an updated comment here right? Like "on the service side, commands are
processed up to the given put_offset before subsequent Flushes on the same
GpuChannel."

[apatrick_chromium, 2011-07-11 21:24:27]

> Also one question: now that we block the entire GpuChannel if any context is
> unscheduled, can there be a case where a background tab (on the same renderer,
> i.e. same channel) can take an arbitrary long time to reschedule, blocking the
> foreground tab? I guess that's a Mac-only problem, I don't know the mechanism
> very well.
> 

Unscheduling is now only used for waiting for the swap acknowledgement on mac
(retested with recent changes) and to wait for the window resize acknowledgement
on windows and linux. In all cases it is a bounded amount of time, unless there
is a bug.

[apatrick_chromium, 2011-07-11 21:25:45]

All done.

Also, see this related follow up patch:

http://codereview.chromium.org/7313032/

http://codereview.chromium.org/7253052/diff/23007/content/common/gpu/gpu_comm...
File content/common/gpu/gpu_command_buffer_stub.cc (right):

http://codereview.chromium.org/7253052/diff/23007/content/common/gpu/gpu_comm...
content/common/gpu/gpu_command_buffer_stub.cc:309:
task_factory_.NewRunnableMethod(&GpuCommandBufferStub::ReportState));
On 2011/07/09 20:40:22, piman wrote:
> Because Flush will now fully execute the commands, you can now ReportState
> immediately without posting a task (and remove the TODO).

Done.

http://codereview.chromium.org/7253052/diff/23007/gpu/command_buffer/client/c...
File gpu/command_buffer/client/cmd_buffer_helper.cc (right):

http://codereview.chromium.org/7253052/diff/23007/gpu/command_buffer/client/c...
gpu/command_buffer/client/cmd_buffer_helper.cc:139: while (AvailableEntries() <
count) {
On 2011/07/11 18:39:50, jbates wrote:
> Looks like this loop is no longer needed because of the call to Finish.

It should actually be a FlushSync. Fixed.

http://codereview.chromium.org/7253052/diff/23007/gpu/command_buffer/client/g...
File gpu/command_buffer/client/gles2_implementation.cc (right):

http://codereview.chromium.org/7253052/diff/23007/gpu/command_buffer/client/g...
gpu/command_buffer/client/gles2_implementation.cc:827:
helper_->CommandBufferHelper::Flush();
On 2011/07/11 18:39:50, jbates wrote:
> On 2011/07/09 20:40:22, piman wrote:
> > It took me a while to figure this out, even with the comment...
> > 
> > So, IIUC, you need this because now when the service unschedules, it never
> goes
> > back to process remaining commands, until another Flush comes, breaking the
> > ordering semantics (if another Flush came on another context in the mean
> time).
> > 
> > I wonder if calling PutChanged from GpuScheduler::SetScheduled would solve
> that
> > problem, in a nicer way, because you wouldn't depend on the client to
enforce
> > the semantics.
> 
> I'm not sure why this Flush is needed. Why does getting unscheduled break
> ordering? Sometime after this resize call, the client should issue a glFlush,
> and that should maintain proper command ordering with respect to other command
> buffers, right? If not, can we fix that so that regardless of
> scheduled/unscheduled, client Flushes are always guaranteed to be completed in
> order (unless an error occurs).

It's complicated and confusing and liable to cause breakage in the future. I'll
fix it as piman suggests.

http://codereview.chromium.org/7253052/diff/23007/gpu/command_buffer/common/c...
File gpu/command_buffer/common/command_buffer.h (right):

http://codereview.chromium.org/7253052/diff/23007/gpu/command_buffer/common/c...
gpu/command_buffer/common/command_buffer.h:81: // sees the latest added
commands, and will eventually process them.
On 2011/07/11 18:39:50, jbates wrote:
> Could use an updated comment here right? Like "on the service side, commands
are
> processed up to the given put_offset before subsequent Flushes on the same
> GpuChannel."

Done.

http://codereview.chromium.org/7253052/diff/23007/gpu/command_buffer/service/...
File gpu/command_buffer/service/gpu_scheduler.h (right):

http://codereview.chromium.org/7253052/diff/23007/gpu/command_buffer/service/...
gpu/command_buffer/service/gpu_scheduler.h:49: gles2::ContextGroup* group);
On 2011/07/09 20:40:22, piman wrote:
> Because of the overload ambiguity between the 2 constructors, I wonder if this
> one wouldn't be better served by a factory function, calling the other one.
WDYT
> ?

Done.

[apatrick_chromium, 2011-07-11 21:28:23]

Oh wait. Missed something. Hold on...

[apatrick_chromium, 2011-07-11 23:13:07]

Okay fixed now PTAL.

The issue was, in general, the command buffer that causes a channel to be
unscheduled is not necessarily the one that causes it to be rescheduled and
therefore not the correct one to call PutChanged on. As it happens, with the
demise of latches, this will never happen. I just didn't want that subtle
assumption so well hidden in the code.

[piman, 2011-07-11 23:38:33]

one fix then LGTM. Adding a ReportState in OnRescheduled() would mean the client
can avoid a round trip in some cases (consistent with not being unscheduled).

http://codereview.chromium.org/7253052/diff/30022/content/common/gpu/gpu_comm...
File content/common/gpu/gpu_command_buffer_stub.cc (right):

http://codereview.chromium.org/7253052/diff/30022/content/common/gpu/gpu_comm...
content/common/gpu/gpu_command_buffer_stub.cc:313:
command_buffer_->Flush(state.put_offset);
Adding a ReportState() here would be nice to the client.

[apatrick_chromium, 2011-07-11 23:56:33]

http://codereview.chromium.org/7253052/diff/30022/content/common/gpu/gpu_comm...
File content/common/gpu/gpu_command_buffer_stub.cc (right):

http://codereview.chromium.org/7253052/diff/30022/content/common/gpu/gpu_comm...
content/common/gpu/gpu_command_buffer_stub.cc:313:
command_buffer_->Flush(state.put_offset);
On 2011/07/11 23:38:33, piman wrote:
> Adding a ReportState() here would be nice to the client.

Done.

[piman, 2011-07-11 23:57:31]

LGTM

[apatrick_chromium, 2011-07-12 00:01:37]

+brettw for ppapi owners review.

+vrk for progress on seeing if this regresses the accelerated ppapi video stuff.
I'll hold off on landing until this is known.

[apatrick_chromium, 2011-07-12 00:03:48]

+jam for webkit/plugins owners review.
+kbr for webkit/gpu owners review.

[Ken Russell (switch to Gerrit), 2011-07-12 00:22:07]

One minor issue with the webkit/gpu changes.

http://codereview.chromium.org/7253052/diff/29077/webkit/gpu/webgraphicsconte...
File webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc (right):

http://codereview.chromium.org/7253052/diff/29077/webkit/gpu/webgraphicsconte...
webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc:45: }
This doesn't look like it's part of this CL.

[apatrick_chromium, 2011-07-12 01:00:42]

http://codereview.chromium.org/7253052/diff/29077/webkit/gpu/webgraphicsconte...
File webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc (right):

http://codereview.chromium.org/7253052/diff/29077/webkit/gpu/webgraphicsconte...
webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc:45: }
On 2011/07/12 00:22:07, kbr wrote:
> This doesn't look like it's part of this CL.

Done.

[Ken Russell (switch to Gerrit), 2011-07-12 01:01:41]

webkit/gpu LGTM

[jam, 2011-07-12 01:03:26]

On 2011/07/12 00:03:48, apatrick_chromium wrote:
> +jam for webkit/plugins owners review.

brettw can review webkit/plugins/pppai as well

> +kbr for webkit/gpu owners review.

[piman, 2011-07-12 01:08:42]

On Mon, Jul 11, 2011 at 6:03 PM, <jam@chromium.org> wrote:

> On 2011/07/12 00:03:48, apatrick_chromium wrote:
>
>> +jam for webkit/plugins owners review.
>>
>
> brettw can review webkit/plugins/pppai as well


Me too and LGTM

>
>
>  +kbr for webkit/gpu owners review.
>>
>
>
>
>
http://codereview.chromium.**org/7253052/<http://codereview.chromium.org/7253...
>

[vrk (LEFT CHROMIUM), 2011-07-13 22:15:38]

> +vrk for progress on seeing if this regresses the accelerated ppapi video
stuff.
> I'll hold off on landing until this is known.

Video stuff still works as of r92083 (i.e. the revision that the currently
uploaded patch is rebased to) but could you rebase to ToT so I can test the
changes that landed yesterday? Thanks!

[jbates, 2011-07-14 21:27:52]

LGTM

[apatrick_chromium, 2011-07-18 21:52:18]

On 2011/07/13 22:15:38, Victoria Kirst wrote:
> > +vrk for progress on seeing if this regresses the accelerated ppapi video
> stuff.
> > I'll hold off on landing until this is known.
> 
> Video stuff still works as of r92083 (i.e. the revision that the currently
> uploaded patch is rebased to) but could you rebase to ToT so I can test the
> changes that landed yesterday? Thanks!

Rebased to tot and uploaded. Thanks.

[vrk (LEFT CHROMIUM), 2011-07-18 22:47:16]

On 2011/07/18 21:52:18, apatrick_chromium wrote:
> Rebased to tot and uploaded. Thanks.

LGTM

Everything still works great! Thanks!