Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(18)

Issue 18226005: Do not fire beforeload events from frames with scripting disabled (Closed)

Created:
6 years, 5 months ago by pdr.
Modified:
6 years, 5 months ago
CC:
blink-reviews, dglazkov+blink, eae+blinkwatch
Visibility:
Public.

Description

Do not fire beforeload events from frames with scripting disabled This patch fixes a crash where an SVG CSS background image could cause all pending beforeload events to fire. This became problematic once data uri images started firing synchronously. The events we are interested in preventing are global pending beforeload events because they are cross- document and could be fired from an inner (sandboxed) SVG document. This patch disables beforeload events from frames where scripting is disabled. This is a followup to https://src.chromium.org/viewvc/blink?view=rev&revision=153029 which missed this case in XMLDocumentParser.cpp. I have verified the other global pending event handlers are not called elsewhere. BUG=256013 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=153969

Patch Set 1 #

Total comments: 1

Patch Set 2 : Do not fire events when the parser is stopped #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+34 lines, -2 lines) Patch
A LayoutTests/fast/images/image-beforeload-event-crash.html View 1 chunk +26 lines, -0 lines 0 comments Download
A LayoutTests/fast/images/image-beforeload-event-crash-expected.txt View 1 chunk +2 lines, -0 lines 0 comments Download
M Source/core/xml/parser/XMLDocumentParser.cpp View 1 2 chunks +6 lines, -2 lines 1 comment Download

Messages

Total messages: 10 (0 generated)
pdr.
6 years, 5 months ago (2013-07-10 03:59:42 UTC) #1
abarth-chromium
lgtm Are there more moles to whack here? https://codereview.chromium.org/18226005/diff/1/Source/core/xml/parser/XMLDocumentParser.cpp File Source/core/xml/parser/XMLDocumentParser.cpp (right): https://codereview.chromium.org/18226005/diff/1/Source/core/xml/parser/XMLDocumentParser.cpp#newcode339 Source/core/xml/parser/XMLDocumentParser.cpp:339: // ...
6 years, 5 months ago (2013-07-10 07:22:40 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/pdr@chromium.org/18226005/1
6 years, 5 months ago (2013-07-10 07:22:54 UTC) #3
commit-bot: I haz the power
Retried try job too often on linux_layout_rel for step(s) webkit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_layout_rel&number=15768
6 years, 5 months ago (2013-07-10 08:22:33 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/pdr@chromium.org/18226005/1
6 years, 5 months ago (2013-07-10 15:41:39 UTC) #5
commit-bot: I haz the power
Retried try job too often on linux_layout_rel for step(s) webkit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_layout_rel&number=15857
6 years, 5 months ago (2013-07-10 16:47:37 UTC) #6
pdr.
On 2013/07/10 07:22:40, abarth wrote: > lgtm > > Are there more moles to whack ...
6 years, 5 months ago (2013-07-10 21:31:27 UTC) #7
abarth-chromium
https://codereview.chromium.org/18226005/diff/17002/Source/core/xml/parser/XMLDocumentParser.cpp File Source/core/xml/parser/XMLDocumentParser.cpp (right): https://codereview.chromium.org/18226005/diff/17002/Source/core/xml/parser/XMLDocumentParser.cpp#newcode340 Source/core/xml/parser/XMLDocumentParser.cpp:340: return; LGTM
6 years, 5 months ago (2013-07-10 21:33:08 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/pdr@chromium.org/18226005/17002
6 years, 5 months ago (2013-07-10 21:39:26 UTC) #9
commit-bot: I haz the power
6 years, 5 months ago (2013-07-11 03:28:59 UTC) #10
Message was sent while issue was closed.
Change committed as 153969

Powered by Google App Engine
This is Rietveld 408576698