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

Issue 17682003: Do not fire load 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, adamk+blink_chromium.org
Visibility:
Public.

Description

Do not fire load events from frames with scripting disabled This patch fixes a crash where an SVG CSS background image could cause all pending load events to fire. This became problematic once data uri images started firing synchronously. The events we are interested in preventing are global pending load events because they are cross- document and will be fired from an inner (sandboxed) SVG document. This patch disables load events from frames where scripting is disabled. BUG=248843 R=japhet@chromium.org Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=153029

Patch Set 1 #

Patch Set 2 : Only suppress global events #

Total comments: 1

Patch Set 3 : Do not send events for detached documents #

Unified diffs Side-by-side diffs Delta from patch set Stats (+31 lines, -5 lines) Patch
A LayoutTests/fast/images/image-load-event-crash.html View 1 chunk +22 lines, -0 lines 0 comments Download
A LayoutTests/fast/images/image-load-event-crash-expected.txt View 1 chunk +2 lines, -0 lines 0 comments Download
M Source/core/dom/Document.cpp View 1 2 1 chunk +7 lines, -5 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
pdr.
6 years, 5 months ago (2013-06-25 17:56:15 UTC) #1
Nate Chapin
https://codereview.chromium.org/17682003/diff/4001/Source/core/dom/Document.cpp File Source/core/dom/Document.cpp (right): https://codereview.chromium.org/17682003/diff/4001/Source/core/dom/Document.cpp#newcode2278 Source/core/dom/Document.cpp:2278: if (!f || f->script()->canExecuteScripts(NotAboutToExecuteScript)) { Should we be firing ...
6 years, 5 months ago (2013-06-25 20:43:22 UTC) #2
pdr.
On 2013/06/25 20:43:22, Nate Chapin wrote: > https://codereview.chromium.org/17682003/diff/4001/Source/core/dom/Document.cpp > File Source/core/dom/Document.cpp (right): > > https://codereview.chromium.org/17682003/diff/4001/Source/core/dom/Document.cpp#newcode2278 ...
6 years, 5 months ago (2013-06-25 21:15:08 UTC) #3
Nate Chapin
Seems reasonable, so long as the tests still pass. :) LGTM
6 years, 5 months ago (2013-06-25 21:22:30 UTC) #4
pdr.
6 years, 5 months ago (2013-06-25 21:54:25 UTC) #5
Message was sent while issue was closed.
Committed patchset #3 manually as r153029 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698