Improve GPU process URL crash reporting

Improve GPU process URL crash reporting.

With this CL: if the GPU process crashes we will probably get the correct URL whether the crash occurred during off or onscreen context command processing.

If we still see blank URLs, more work can be done on the webkit side to report offscreen context URLs via webkitplatformsupport.

BUG=131466
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=146488

[jbates, 2012-07-10 23:06:44]

kbr: ptal
brettw: */OWNERS

[Ken Russell (switch to Gerrit), 2012-07-10 23:38:01]

https://chromiumcodereview.appspot.com/10695133/diff/1/content/common/gpu/gpu...
File content/common/gpu/gpu_command_buffer_stub.cc (right):

https://chromiumcodereview.appspot.com/10695133/diff/1/content/common/gpu/gpu...
content/common/gpu/gpu_command_buffer_stub.cc:36: // url_hash matches.
Any indication of what kind of speedup this yields? Would it be better to just
optimize the implementation of ContentClient::SetActiveURL()?

https://chromiumcodereview.appspot.com/10695133/diff/1/content/renderer/brows...
File content/renderer/browser_plugin/old/guest_to_embedder_channel.cc (right):

https://chromiumcodereview.appspot.com/10695133/diff/1/content/renderer/brows...
content/renderer/browser_plugin/old/guest_to_embedder_channel.cc:99: if
(render_view->webview()->mainFrame())
Under what conditions would this not be the case? (iframes?) Is there any
fallback URL we could use?

[jbates, 2012-07-11 00:07:18]

https://chromiumcodereview.appspot.com/10695133/diff/1/content/common/gpu/gpu...
File content/common/gpu/gpu_command_buffer_stub.cc (right):

https://chromiumcodereview.appspot.com/10695133/diff/1/content/common/gpu/gpu...
content/common/gpu/gpu_command_buffer_stub.cc:36: // url_hash matches.
On 2012/07/10 23:38:01, kbr wrote:
> Any indication of what kind of speedup this yields? Would it be better to just
> optimize the implementation of ContentClient::SetActiveURL()?

From a quick printf run, the majority of FastSetActiveURL calls had a matching
hash, so it's definitely a speedup but I'm not sure how much. Since URLs can be
very long, even a perfect implementation of ContentClient::SetActiveURL(url)
would require a O(n) memcpy or strcmp for every call.
ContentClient::SetActiveURL could be refactored to take a hash, but the other
use cases that I found are not as high frequency as GpuScheduler::PutChanged, so
it seemed appropriate to just implement this here. In the grand scheme of things
I doubt we'd ever notice, but I will sleep better if this optimization stays in
:)

https://chromiumcodereview.appspot.com/10695133/diff/1/content/renderer/brows...
File content/renderer/browser_plugin/old/guest_to_embedder_channel.cc (right):

https://chromiumcodereview.appspot.com/10695133/diff/1/content/renderer/brows...
content/renderer/browser_plugin/old/guest_to_embedder_channel.cc:99: if
(render_view->webview()->mainFrame())
On 2012/07/10 23:38:01, kbr wrote:
> Under what conditions would this not be the case? 

Not sure, this was copied from render_view_impl.cc.

(iframes?) Is there any
> fallback URL we could use?

At least we can fall back on the method name - updated this instance as well as
render_view_impl.cc.

[Ken Russell (switch to Gerrit), 2012-07-11 01:46:32]

Sounds fine. LGTM

I gather another patch is coming but consider my LGTM to apply to it.

[brettw, 2012-07-12 14:16:43]

https://chromiumcodereview.appspot.com/10695133/diff/2002/content/renderer/re...
File content/renderer/render_thread_impl.cc (right):

https://chromiumcodereview.appspot.com/10695133/diff/2002/content/renderer/re...
content/renderer/render_thread_impl.cc:747:
GURL("RenderThreadImpl::GetGpuVDAContext3D")));
In the places you use these fake URLs, I'd prefer something that looks more like
a URL. As-is, it looks like a mistake. Maybe
"chrome://gpu/RenderThreadImpl::GetGpuVDAContext3D"?

[jbates, 2012-07-12 17:51:13]

https://chromiumcodereview.appspot.com/10695133/diff/2002/content/renderer/re...
File content/renderer/render_thread_impl.cc (right):

https://chromiumcodereview.appspot.com/10695133/diff/2002/content/renderer/re...
content/renderer/render_thread_impl.cc:747:
GURL("RenderThreadImpl::GetGpuVDAContext3D")));
On 2012/07/12 14:16:43, brettw wrote:
> In the places you use these fake URLs, I'd prefer something that looks more
like
> a URL. As-is, it looks like a mistake. Maybe
> "chrome://gpu/RenderThreadImpl::GetGpuVDAContext3D"?

Done.

[brettw, 2012-07-12 18:06:12]

lgtm

[commit-bot: I haz the power, 2012-07-12 18:11:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jbates@chromium.org/10695133/2003

[commit-bot: I haz the power, 2012-07-12 18:27:42]

Try job failure for 10695133-2003 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-07-12 22:56:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jbates@chromium.org/10695133/14002

[commit-bot: I haz the power, 2012-07-12 23:48:12]

Try job failure for 10695133-14002 (retry) on linux_rel for step
"browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...