1. go to https://chrome.google.com/extensions 2. open js console 3. click "install" See the "failure to load resource" message Original bug report: http://code.google.com/p/chromium/issues/detail?id=62888
Created attachment 84735 [details] patch
Comment on attachment 84735 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=84735&action=review > Source/WebCore/loader/MainResourceLoader.cpp:122 > + error.setIsCancellation(true); I am not sure what this would result in. You should call for more reviewers. Adding ap for now.
Good question. I spent a couple of minutes looking into this, and couldn't easily answer if this this is OK - Safari uses PolicyIgnore in many cases. Will look again later. How does this affect bug 43614 and bug 38346? Can this be tested? See fast/loader/onload-policy-ignore-for-frame.html for an example of policy delegate test.
Comment on attachment 84735 [details] patch I think that this change is acceptable. It's a mess how WebCore decides what a cancellation is, but this is not making it worse. However, I have one more question in addition to ones in comment 3. Why does ResourceLoader::didCancel() even call didFailToLoad()? Besides logging an error to Inspector, that also calls FrameLoaderClient::dispatchDidFailLoading(), and that also looks wrong at a first glance. If there is some historic reason, there should probably be a comment. Since this needs more work (at least a test), I'm marking r- for now.
+ - fixed error formatting in case resource error code not present I've noticed that error is completely broken in Safari nightlies when installing a Safari extension from a local file: GET file:///Users/ap/Downloads/AdBlockForSafari.safariextz undefined (undefined) In Safari 5.0.3, it's: Failed to load resource: Загрузка фрейма прервана Should I file a separate bug about that? Clearly, we shouldn't report any error, but could this be a general problem with console logging?
(In reply to comment #5) > + - fixed error formatting in case resource error code not present > > I've noticed that error is completely broken in Safari nightlies when installing a Safari extension from a local file: > > GET file:///Users/ap/Downloads/AdBlockForSafari.safariextz undefined (undefined) > > In Safari 5.0.3, it's: > > Failed to load resource: Загрузка фрейма прервана > > Should I file a separate bug about that? Clearly, we shouldn't report any error, but could this be a general problem with console logging? I don't think this is worth an additional bug -- this change fixes both the error message format in case there's no error code (see the diff for ConsoleView.js) and reporting cancellation as an error. Thanks for mentioning bug 43614 and bug 38346 -- unfortunately I missed them. The latter is fixed by this change, and as for the former -- well, the poor "canceled" message is gone with this change, and someone else apparently added (brought back?) proper diagnostics -- so this does not reproduce anymore (I added a test for that). > However, I have one more question in addition to ones in comment 3. > Why does ResourceLoader::didCancel() even call didFailToLoad()? > Besides logging an error to Inspector, that also calls FrameLoaderClient::dispatchDidFailLoading(), and that also looks wrong at a first glance. > If there is some historic reason, there should probably be a comment. Well, my guess would be that this is indeed historical, probably done for the lack of separate notification method for cancel in FrameLoaderClient. While this looks like a misnomer to me, having one method for both cancel and fail is probably appropriate, since for most of the clients handling of an aborted request is perhaps similar no matter whether the reason is cancellation or early failure.
Created attachment 85214 [details] patch - added a test for lack of console errors in case of policy change - added a test for cross-origin XHR error - rebaselined
OK, thanks for the investigation! Please do resolve the other bugs as appropriate.
Created attachment 85318 [details] patch (fixed binary diff) Well, if everyone is happy, does anyone mind r+'ing this? ;-)
*** Bug 43614 has been marked as a duplicate of this bug. ***
Comment on attachment 85318 [details] patch (fixed binary diff) View in context: https://bugs.webkit.org/attachment.cgi?id=85318&action=review Please address my comment prior to landing. > Source/WebCore/inspector/front-end/ConsoleView.js:726 > + else if (resource.statusCode) You should make sure InspectorInstrumentation marks resources as failed prior to reporting error to the console. Then you can revert this change.
Created attachment 85447 [details] landed patch
Manually committed r80832: http://trac.webkit.org/changeset/80832