http://code.google.com/p/chromium/issues/detail?id=88042 WebFrameImpl::fromFrame may return null, and DatabaseObserver::canEstablishDatabase does not check for this, leading to potential crashes.
Created attachment 107549 [details] Patch
this looks reasonable to me i think changing the return value at the end to false is reasonable, but i don't know if there are chromium/webkit api users that are depending on the true return value at this time... are their any consumers that don't provide a permissionClient that this change would break? also, i'm noticing that 'true' is the default return value for the allowDatabase method in WebPermissionsClient.h maybe it's safer and more consistent to not change that return value at the end in this crash fixing patch wdyt?
I investigated further, looking for cases in the chromium code base where the WebPermissionsClient is set on the WebViewImpl. The only location is in chrome_render_view_observer, and the AllowDatabase setting for that ultimately comes from the browser's cookie settings. So it is certainly plausible that there exist other users of the database code that do not set any permissions client. There is also database related code in WebWorkerClientImpl, and it defaults to true when there is no permissions client (and the permissions client always returns true, as far as I can tell). Given this, and the default of true in the permissions client, I will change the return value.
Created attachment 107663 [details] Patch
fwiw lgtm (but i dont have review rights)
adding some webkit reviewers
Comment on attachment 107663 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=107663&action=review Is it possible to test this? Is it possible for webFrame->viewImpl() to be 0? What about document->frame()? > Source/WebKit/chromium/ChangeLog:10 > + null frame and return false when the frame is null. Investiagted typo: Investiagted
I have no idea how to test that this resolves the crash, but I will look into it. At the same time I'll try to figure out why one thing may be null but not another.
Is it possible to test this? There was no repro case in the original crash report. It clearly depends on having a script execution context with a valid document (if you have the context you have the document) but an invalid frame. I'll try to repro with that in mind. Is it possible for webFrame->viewImpl() to be 0? webFrame->viewImpl() may be 0 if the frame is 0 or the page for the frame is 0. That means it should be checked too. What about document->frame()? document->frame() is probably the 0 object in this situation, but that does not need to be checked because WebFrameImpl::fromFrame(document->frame()) is happy to return 0 when it gets 0.
Created attachment 108481 [details] Patch
I have no commit status at this time. Please do the honors for me.
Comment on attachment 108481 [details] Patch Feel free to mark your patch as cq? if you'd like it marked cq+ :)
Comment on attachment 108481 [details] Patch Attachment 108481 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9818156 New failing tests: svg/custom/svg-fonts-word-spacing.html
Comment on attachment 108481 [details] Patch If at first you don't succeed... that test failure in svg is totally unrelated to this change.
Comment on attachment 108481 [details] Patch Clearing flags on attachment: 108481 Committed r95871: <http://trac.webkit.org/changeset/95871>
All reviewed patches have been landed. Closing bug.