Follow up on bug 57140. I tried to find the cause of this problem but the backtrace does not help (see the previous bug for the EWS one). I suspect that it is an issue unrelated to the change that is making it fail as other platforms work fine (Qt/Linux + Mac).
Crashing on all Chromium platforms.
Created attachment 120881 [details] Patch
Darin, can you review please? Is my assumption correct that detached iframes don't have a webView?
Comment on attachment 120881 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=120881&action=review > Source/WebKit/chromium/src/StorageAreaProxy.cpp:169 > { it might be more canonical to add an early check to see if frame->page() is null, and then return if so.
Created attachment 120978 [details] Patch
Attachment 120978 [details] did not pass style-queue: Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9 Updating OpenSource From git://git.webkit.org/WebKit 8e973cb..e1e2538 master -> origin/master Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ... Currently at 103955 = 6aab0dafec68b43a47f81ab79d03001e05e99b64 r103949 = e1e2538cff7d320c2b7cc22c0b75c9369ce9cdb2 Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc A LayoutTests/svg/custom/webkit-transform-crash.html A LayoutTests/svg/custom/webkit-transform-crash-expected.txt M LayoutTests/ChangeLog M Source/WebCore/ChangeLog M Source/WebCore/svg/SVGStyledTransformableElement.cpp 103950 = 8282a1d85ad097ce4064a5896912bdb211b115e6 already exists! Why are we refetching it? at /usr/lib/git-core/git-svn line 5210 Died at Tools/Scripts/update-webkit line 158. If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 120978 [details] Patch Attachment 120978 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11084081
Created attachment 120979 [details] Patch
Attachment 120979 [details] did not pass style-queue: Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9 Updating OpenSource Index mismatch: 3a7674dadd24ecd6a1b023eba203ecf2ed62fc59 != edb861612940a3244431d239dacdbc36317b627c rereading e1e2538cff7d320c2b7cc22c0b75c9369ce9cdb2 A LayoutTests/svg/custom/webkit-transform-crash.html A LayoutTests/svg/custom/webkit-transform-crash-expected.txt M LayoutTests/ChangeLog M Source/WebCore/ChangeLog M Source/WebCore/svg/SVGStyledTransformableElement.cpp 103950 = 8282a1d85ad097ce4064a5896912bdb211b115e6 already exists! Why are we refetching it? at /usr/lib/git-core/git-svn line 5210 Died at Tools/Scripts/update-webkit line 158. If any of these errors are false positives, please file a bug against check-webkit-style.
PTAL The style bot error is because of a problem with the bots checkout, unrelated to my CL
Comment on attachment 120979 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=120979&action=review > Source/WebKit/chromium/src/StorageAreaProxy.cpp:171 > + return false; Bad indent. I don't see where frame->page() is accessed in this function. Would it be better to null check something else? For example, maybe we should null-check the webView. > LayoutTests/fast/storage/storage-detached-iframe.html:26 > + } catch (e) { > + // Throws an exception on chromium, and fails silently on other platforms. > + } Should we make Chromium and other ports work the same?
(In reply to comment #11) > (From update of attachment 120979 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=120979&action=review > > > Source/WebKit/chromium/src/StorageAreaProxy.cpp:171 > > + return false; > > Bad indent. I don't see where frame->page() is accessed in this function. Would it be better to null check something else? For example, maybe we should null-check the webView. See Darin's comment. Basically, the fact that frame->page() is NULL is canonical for the frame being detached. > > > LayoutTests/fast/storage/storage-detached-iframe.html:26 > > + } catch (e) { > > + // Throws an exception on chromium, and fails silently on other platforms. > > + } > > Should we make Chromium and other ports work the same? Actually, other ports don't fail silently, but the access works for them :-/ We can't just allow access if there's no page. A possible work around would be to move permissionClient() from WebView to WebFrameClient, wdyt?
(In reply to comment #12) > (In reply to comment #11) > > (From update of attachment 120979 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=120979&action=review > > > > > Source/WebKit/chromium/src/StorageAreaProxy.cpp:171 > > > + return false; > > > > Bad indent. I don't see where frame->page() is accessed in this function. Would it be better to null check something else? For example, maybe we should null-check the webView. > > See Darin's comment. > > Basically, the fact that frame->page() is NULL is canonical for the frame being detached. Ok. > > > LayoutTests/fast/storage/storage-detached-iframe.html:26 > > > + } catch (e) { > > > + // Throws an exception on chromium, and fails silently on other platforms. > > > + } > > > > Should we make Chromium and other ports work the same? > > Actually, other ports don't fail silently, but the access works for them :-/ > > We can't just allow access if there's no page. A possible work around would be to move permissionClient() from WebView to WebFrameClient, wdyt? Hum... How do other browsers handle this case? We probably don't want any ports to allow access to local storage while detached.
Created attachment 121334 [details] Patch
Comment on attachment 121334 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=121334&action=review > Source/WebKit/chromium/src/StorageAreaProxy.cpp:170 > + // Disallow all access to storage on detached frames. I would skip these comments. They're redundant with the code. > LayoutTests/fast/storage/storage-detached-iframe.html:24 > + } catch (e) { Should we print a success message in this catch?
Created attachment 121346 [details] Patch
Comment on attachment 121346 [details] Patch Clearing flags on attachment: 121346 Committed r104257: <http://trac.webkit.org/changeset/104257>
All reviewed patches have been landed. Closing bug.