In order to improve the quality of crash reports and lost context notifications, a URL needs to be associated with the GraphicsContext3D instances created on behalf of WebGL. As of the fix for Bug 101826, the FrameLoaderClient gets the opportunity to veto WebGL context creation, but the context creation code itself does not have enough information to determine the URL of the page which is creating the context. This information is most easily passed down from WebCore to the platform.
Created attachment 177069 [details] Patch
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment on attachment 177069 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177069&action=review > Source/Platform/chromium/public/WebGraphicsContext3D.h:112 > + WebString urlOfCreatingDocument; Why a WebString an not a WebURL?
(In reply to comment #3) > (From update of attachment 177069 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=177069&action=review > > > Source/Platform/chromium/public/WebGraphicsContext3D.h:112 > > + WebString urlOfCreatingDocument; > > Why a WebString an not a WebURL? No good reason. It turns out there was a technical problem using WebURL here, but it wasn't difficult to fix in the gyp files. New patch coming up.
Created attachment 177411 [details] Patch
Comment on attachment 177411 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177411&action=review > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:428 > + attributes.topDocumentURL = document->frame()->tree()->top()->document()->url(); document->topDocument() will save you a lot of typing.
Created attachment 177643 [details] Patch for landing
Comment on attachment 177411 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177411&action=review >> Source/WebCore/html/canvas/WebGLRenderingContext.cpp:428 >> + attributes.topDocumentURL = document->frame()->tree()->top()->document()->url(); > > document->topDocument() will save you a lot of typing. Fixed, thanks.
Committed r136609: <http://trac.webkit.org/changeset/136609>
Reverted r136609 for reason: Breaking Chrome/Mac Debug builds. Committed r136734: <http://trac.webkit.org/changeset/136734>
Example broken build: http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac%20Builder%20%28dbg%29/builds/34923
@thakis indicates this bot is using the shared library build.
I tried, but it is infeasible to pass an instance of WebURL (not a reference) across the WebKit boundary, in particular in the component build. The problem is that WebURL.h has inlined constructors which call out-of-line code in the googleurl library, and there is just no way using GYP to express that dependents of targets in WebKit.gyp need to depend on (i.e., include on their link line) the googleurl library. Neither export_dependent_settings nor direct_dependent_settings do this; note for example that the icuuc library (currently in the export_dependent_settings of WebKit.gyp:webkit) does not show up on the link line of targets that depend on webkit, such as src/webkit/gpu/webkit_gpu.gypi:webkit_gpu. If I try to use a WebURL here then some (but not all) downstream targets of the WebKit API will have to mysteriously add a googleurl dependency in the component build. I found at least two targets (src/webkit/gpu/webkit_gpu.gypi:webkit_gpu and src/ui/compositor/compositor.gyp:compositor) which need it, but I'm not going to pursue this further as it would just add a time bomb to the code base. Tomorrow I'll recast this patch to use WebString. abarth suggested adding a FIXME indicating why it doesn't use WebURL; this comment documents the reason.
Created attachment 178083 [details] Revised patch using WebString
abarth doesn't seem to be around today, but yesterday indicated offline that the change to WebString is OK. I can't afford another delay of this patch and it looks good on the EWS so am landing it by hand now.
Committed r136907: <http://trac.webkit.org/changeset/136907>
Comment on attachment 178083 [details] Revised patch using WebString Delayed LGTM