Wait for the GPU info to be collected.

Wait for the GPU info to be collected.
BUG=59131
TEST=none

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

[nduca, 2010-12-13 20:33:27]

Afaik, the "collected" callback is going to fire twice on windows, right? First
when the progress is partial, then again when the progress is complete. Partial
means you have basic fields but no dxdiag info.

Might be better to call this "GpuInfoProgressChanged," or maybe tighten it up so
it only fires on complete and call it GpuInfoProgressComplete.


On 2010/12/13 20:10:07, kkania wrote:

[apatrick_chromium, 2010-12-13 21:16:34]

http://codereview.chromium.org/5698009/diff/1/chrome/browser/gpu_process_host...
File chrome/browser/gpu_process_host_ui_shim.cc (right):

http://codereview.chromium.org/5698009/diff/1/chrome/browser/gpu_process_host...
chrome/browser/gpu_process_host_ui_shim.cc:111:
gpu_info_collected_callback_.reset();
I think the caller of set_gpu_info_collected_callback should set the callback to
null if they don't want the callback invoked in the future. As nduca pointed
out, the callback might be called multiple times with increasing amounts of
information available. Specifically the dxdiag info on windows takes a couple of
seconds to come back while the basic info like device and vendor id comes back
more quickly.

http://codereview.chromium.org/5698009/diff/1/chrome/test/gpu/gpu_pixel_brows...
File chrome/test/gpu/gpu_pixel_browsertest.cc (right):

http://codereview.chromium.org/5698009/diff/1/chrome/test/gpu/gpu_pixel_brows...
chrome/test/gpu/gpu_pixel_browsertest.cc:112: if (info.progress() !=
GPUInfo::kComplete) {
If you only need the device and vendor id, this should probably be kPartial. The
distinction between kPartial and kComplete is only significant on windows.
kPartial means everything except the dxdiag info. I looks like you only want the
device and vendor id.

[kkania, 2010-12-13 22:12:57]

Thanks for the info...I left the callback named gpu_info_collected so that it
matches with the function that gets called in the shim.

http://codereview.chromium.org/5698009/diff/1/chrome/browser/gpu_process_host...
File chrome/browser/gpu_process_host_ui_shim.cc (right):

http://codereview.chromium.org/5698009/diff/1/chrome/browser/gpu_process_host...
chrome/browser/gpu_process_host_ui_shim.cc:111:
gpu_info_collected_callback_.reset();
On 2010/12/13 21:16:35, apatrick_chromium wrote:
> I think the caller of set_gpu_info_collected_callback should set the callback
to
> null if they don't want the callback invoked in the future. As nduca pointed
> out, the callback might be called multiple times with increasing amounts of
> information available. Specifically the dxdiag info on windows takes a couple
of
> seconds to come back while the basic info like device and vendor id comes back
> more quickly.

Done.

http://codereview.chromium.org/5698009/diff/1/chrome/test/gpu/gpu_pixel_brows...
File chrome/test/gpu/gpu_pixel_browsertest.cc (right):

http://codereview.chromium.org/5698009/diff/1/chrome/test/gpu/gpu_pixel_brows...
chrome/test/gpu/gpu_pixel_browsertest.cc:112: if (info.progress() !=
GPUInfo::kComplete) {
On 2010/12/13 21:16:35, apatrick_chromium wrote:
> If you only need the device and vendor id, this should probably be kPartial.
The
> distinction between kPartial and kComplete is only significant on windows.
> kPartial means everything except the dxdiag info. I looks like you only want
the
> device and vendor id.

Done.

[apatrick_chromium, 2010-12-13 22:24:06]

lgtm

[nduca, 2010-12-13 22:24:27]

agreed
On 2010/12/13 22:24:06, apatrick_chromium wrote:
> lgtm