Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(243)

Issue 787143002: Enable SVGImage to dispatch events in the destructor. (Closed)

Created:
6 years ago by reni
Modified:
5 years, 11 months ago
Reviewers:
esprehn
CC:
blink-reviews, krit, ed+blinkwatch_opera.com, f(malita), fs, gyuyoung.kim_webkit.org, kouhei+svg_chromium.org, pdr+svgwatchlist_chromium.org, rwlbuis, Stephen Chennney, sof
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Enable SVGImage to dispatch events in the destructor. The destructor of SVGImage can dispatch harmless events that can be called in EventDispatchForbiddenScope scope. BUG=378524 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=188801

Patch Set 1 #

Total comments: 1

Patch Set 2 : Update according to esprehn's comment. #

Total comments: 1

Patch Set 3 : Update #

Unified diffs Side-by-side diffs Delta from patch set Stats (+13 lines, -1 line) Patch
A LayoutTests/svg/dispatch-event-crash-on-destruct.html View 1 chunk +9 lines, -0 lines 0 comments Download
A + LayoutTests/svg/dispatch-event-crash-on-destruct-expected.txt View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M Source/core/loader/FrameLoader.cpp View 1 2 2 chunks +2 lines, -1 line 0 comments Download

Messages

Total messages: 17 (4 generated)
reni
6 years ago (2014-12-09 19:09:49 UTC) #2
pdr.
On 2014/12/09 at 19:09:49, rhodovan.u-szeged wrote: > I am skeptical that these would be harmless. ...
6 years ago (2014-12-09 19:47:25 UTC) #3
esprehn
On 2014/12/09 at 19:47:25, pdr wrote: > On 2014/12/09 at 19:09:49, rhodovan.u-szeged wrote: > > ...
6 years ago (2014-12-18 06:57:30 UTC) #4
reni
@esprehn, @pdr: thanks for the comments and reviews.
6 years ago (2014-12-18 18:39:32 UTC) #5
reni
Could someone take a look at this, please?
5 years, 11 months ago (2015-01-07 14:18:18 UTC) #6
esprehn
https://codereview.chromium.org/787143002/diff/20001/Source/core/svg/graphics/SVGImage.cpp File Source/core/svg/graphics/SVGImage.cpp (right): https://codereview.chromium.org/787143002/diff/20001/Source/core/svg/graphics/SVGImage.cpp#newcode69 Source/core/svg/graphics/SVGImage.cpp:69: EventDispatchForbiddenScope::AllowUserAgentEvents allowEvents; Why do you still need this if ...
5 years, 11 months ago (2015-01-09 00:55:06 UTC) #7
esprehn
lgtm
5 years, 11 months ago (2015-01-21 09:48:28 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/787143002/40001
5 years, 11 months ago (2015-01-21 09:48:45 UTC) #10
commit-bot: I haz the power
Failed to commit the patch. svn: E155010: Commit failed (details follow): svn: E155010: '/b/infra_internal/commit_queue/workdir/blink/LayoutTests/platform/linux/fast/masking' is ...
5 years, 11 months ago (2015-01-21 11:45:53 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/787143002/40001
5 years, 11 months ago (2015-01-22 09:46:37 UTC) #14
commit-bot: I haz the power
Committed patchset #3 (id:40001) as https://src.chromium.org/viewvc/blink?view=rev&revision=188801
5 years, 11 months ago (2015-01-22 09:47:11 UTC) #15
reni
On 2015/01/21 09:48:28, esprehn wrote: > lgtm Thank you :)
5 years, 11 months ago (2015-01-22 09:47:29 UTC) #16
sof
5 years, 11 months ago (2015-01-22 15:06:48 UTC) #17
Message was sent while issue was closed.
Thanks for fixing this, but doesn't it do the opposite of what the description
says? i.e., it doesn't dispatch unload events for SVGImages.

Powered by Google App Engine
This is Rietveld 408576698