make command buffer work in DRT

make command buffer work in DRT

Note several tests get different results than the non-command buffer path.
I'm not sure if that's bugs in this impl, bugs in the command buffer
or bugs else where but that's kind of the point. This CL is supposed
to help find those bugs.

Is there something I should do to mark them to be fixed before
checking this in? Should I leave webkit_support.cc out of this
CL?

TEST=ran layout tests
BUG=84157
R=jamesr@chromium.org


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=87879

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=87910

[jamesr, 2011-06-01 02:07:01]

left some comments, mostly about comments/code that's misleading/confusing in a
single-process implementation.

To land this I think we should do something like this:

1.) Get a list of all layout tests that this patch is expected to break - you
should be able to use the linux_layout,mac_layout,win_layout trybots to get this
list, or test locally.
2.) Add those tests to webkit/tools/layout_tests/test_expectations.txt in the
chromium repo when landing this patch.
3.) File bugs on the tests that break, migrating expectations to the upstream
(WebKit) version of test_expectations.txt and removing them from the
webkit/tools/layout_tests/ version as we go.
4.) ???
5.) Party!

If it's just on the order of 10 tests that break when landing then this method
should work fine.

It'll be great to get the test code closer to what we ship!

http://codereview.chromium.org/7085002/diff/3001/webkit/gpu/webgraphicscontex...
File webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc (right):

http://codereview.chromium.org/7085002/diff/3001/webkit/gpu/webgraphicscontex...
webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc:243: // Shared
memory allocator for latches. Creates a block of shared memory for
this isn't really shared memory in this implementation, is it?

http://codereview.chromium.org/7085002/diff/3001/webkit/gpu/webgraphicscontex...
webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc:263: //
Pointer to mapped shared memory.
this is confusing - can't we just use memory_ (and/or rename it to latches_)?

http://codereview.chromium.org/7085002/diff/3001/webkit/gpu/webgraphicscontex...
webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc:611: //
process and the GPU process.
comment isn't quite correct for this case.  what is it actually doing - does it
allocated a shared memory segment here, or just a normal block of memory?

http://codereview.chromium.org/7085002/diff/3001/webkit/gpu/webgraphicscontex...
webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc:622:
command_buffer_->GetTransferBuffer(transfer_buffer_id_);
this is a no-op for this case right?

http://codereview.chromium.org/7085002/diff/3001/webkit/gpu/webgraphicscontex...
webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc:628: //
Register transfer buffer so that the context can access latches.
comment is misleading

http://codereview.chromium.org/7085002/diff/3001/webkit/gpu/webgraphicscontex...
webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc:630: // 
latch_transfer_buffer_id_ = command_buffer_->RegisterTransferBuffer(
what does this really do?

http://codereview.chromium.org/7085002/diff/3001/webkit/gpu/webgraphicscontex...
webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc:841: //
GLInProcessContext::MakeCurrent(NULL);
comment and code are out of sync

[greggman, 2011-06-01 21:55:39]

http://codereview.chromium.org/7085002/diff/3001/webkit/gpu/webgraphicscontex...
File webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc (right):

http://codereview.chromium.org/7085002/diff/3001/webkit/gpu/webgraphicscontex...
webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc:243: // Shared
memory allocator for latches. Creates a block of shared memory for
On 2011/06/01 02:07:02, jamesr wrote:
> this isn't really shared memory in this implementation, is it?

Done.

http://codereview.chromium.org/7085002/diff/3001/webkit/gpu/webgraphicscontex...
webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc:263: //
Pointer to mapped shared memory.
On 2011/06/01 02:07:02, jamesr wrote:
> this is confusing - can't we just use memory_ (and/or rename it to latches_)?

Done.

http://codereview.chromium.org/7085002/diff/3001/webkit/gpu/webgraphicscontex...
webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc:611: //
process and the GPU process.
On 2011/06/01 02:07:02, jamesr wrote:
> comment isn't quite correct for this case.  what is it actually doing - does
it
> allocated a shared memory segment here, or just a normal block of memory?

Done.

http://codereview.chromium.org/7085002/diff/3001/webkit/gpu/webgraphicscontex...
webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc:622:
command_buffer_->GetTransferBuffer(transfer_buffer_id_);
On 2011/06/01 02:07:02, jamesr wrote:
> this is a no-op for this case right?

I don't actually know and I shouldn't have to. I want this code to appear as
though it's using multiple processes. I hope it will eventually.

http://codereview.chromium.org/7085002/diff/3001/webkit/gpu/webgraphicscontex...
webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc:628: //
Register transfer buffer so that the context can access latches.
On 2011/06/01 02:07:02, jamesr wrote:
> comment is misleading

Done.

http://codereview.chromium.org/7085002/diff/3001/webkit/gpu/webgraphicscontex...
webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc:630: // 
latch_transfer_buffer_id_ = command_buffer_->RegisterTransferBuffer(
On 2011/06/01 02:07:02, jamesr wrote:
> what does this really do?

This code has been deleted.

http://codereview.chromium.org/7085002/diff/3001/webkit/gpu/webgraphicscontex...
webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc:841: //
GLInProcessContext::MakeCurrent(NULL);
On 2011/06/01 02:07:02, jamesr wrote:
> comment and code are out of sync

I updated the comment though it still says basically the same thing. What would
you like it to say?

[greggman, 2011-06-02 21:37:37]

ping

[jamesr, 2011-06-02 21:44:33]

LGTM!  I'd recommend shooting a try job to linux_clang, or using the
commit-queue which will test on clang just to make sure that compiles.

http://codereview.chromium.org/7085002/diff/13001/webkit/tools/layout_tests/t...
File webkit/tools/layout_tests/test_expectations.txt (right):

http://codereview.chromium.org/7085002/diff/13001/webkit/tools/layout_tests/t...
webkit/tools/layout_tests/test_expectations.txt:9: // The need for rebaselines
is conflicting with the TIMEOUT expectation upstream
Looks like this mis-merged - all the lines other than your fast/canvas/ ones
were delete upstream.  Can you nuke everything down to line 30 before landing?

[M-A Ruel, 2011-06-03 13:15:09]

Sorry but the base url is wrong.

Make sure to add a new remote:
git remote add other http://git.chromium.org/git/chromium.git
git fetch other
git branch --set-upstream drt other/trunk
Then you can work on it.

Since I'm a nice guy, I've overridden your base url as you cannot change the
base url of an existing issue. So normally, fixing this requires uploading a new
issue.

[commit-bot: I haz the power, 2011-06-03 13:15:54]

Presubmit check for 7085002-14003 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Warnings **
Found lines longer than 80 characters (first 5 shown).
  webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc, line 710,
96 chars \
  webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc, line 1397,
84 chars \
  webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc, line 1421,
83 chars \
  webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc, line 1440,
82 chars

[M-A Ruel, 2011-06-03 13:18:32]

On 2011/06/03 13:15:54, commit-bot wrote:
> Presubmit check for 7085002-14003 failed and returned exit status 1.
> 
> Running presubmit commit checks ...
> 
> ** Presubmit Warnings **
> Found lines longer than 80 characters (first 5 shown).
>   webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc, line 710,
> 96 chars \
>   webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc, line
1397,
> 84 chars \
>   webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc, line
1421,
> 83 chars \
>   webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc, line
1440,
> 82 chars

You'll have to fix this first. Then, since the base url is now ok, it will just
work (tm) when you check the commit checkbox.

[commit-bot: I haz the power, 2011-06-03 21:55:26]

Change committed as 87879