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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1150623003: Revamp asynchronous (load) event dispatching for SVG elements (Closed)

Created:
5 years, 7 months ago by fs
Modified:
5 years, 7 months ago
Reviewers:
pdr.
CC:
blink-reviews, krit, blink-reviews-style_chromium.org, kouhei+svg_chromium.org, f(malita), gyuyoung2, Stephen Chennney, pdr+svgwatchlist_chromium.org, rwlbuis, Erik Dahlström
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Revamp asynchronous (load) event dispatching for SVG elements Refactor asynchronous event dispatching for SVG elements to use EventSender instead of the home-grown mechanism. This eliminates the need for one timer (96 bytes on 64-bit targets) per <use> and <style> element. In SVGStyleElement, align with HTMLStyleElement, by adding an implementation of Node::notifyLoadedSheetAndAllCriticalSubresources(), replacing SVGStyleElement::sendSVGErrorEventAsynchronously(). The call in SVGUseElement::insertedInto is dropped, because it would never cause an event to be dispatched (an event will only be dispatched when the <use> is "structurally external"). There's a slight change in behavior for SVGUseElement, since the event is now always dispatched, regardless of existence of handlers. Drop sendSVGLoadEventIfPossibleAsynchronously(), svgLoadEventTimer() and svgLoadEventTimerFired() from SVGElement because they are now unused. Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=195678

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -40 lines) Patch
M Source/core/svg/SVGElement.h View 2 chunks +0 lines, -4 lines 0 comments Download
M Source/core/svg/SVGElement.cpp View 1 chunk +0 lines, -16 lines 0 comments Download
M Source/core/svg/SVGStyleElement.h View 1 chunk +1 line, -4 lines 0 comments Download
M Source/core/svg/SVGStyleElement.cpp View 4 chunks +6 lines, -6 lines 1 comment Download
M Source/core/svg/SVGUseElement.h View 5 chunks +4 lines, -6 lines 0 comments Download
M Source/core/svg/SVGUseElement.cpp View 7 chunks +16 lines, -4 lines 1 comment Download

Messages

Total messages: 7 (2 generated)
fs
https://codereview.chromium.org/1150623003/diff/1/Source/core/svg/SVGStyleElement.cpp File Source/core/svg/SVGStyleElement.cpp (right): https://codereview.chromium.org/1150623003/diff/1/Source/core/svg/SVGStyleElement.cpp#newcode154 Source/core/svg/SVGStyleElement.cpp:154: if (errorStatus != NoErrorLoadingSubresource) HTMLStyleElement actually dispatches 'load' events ...
5 years, 7 months ago (2015-05-20 14:43:35 UTC) #2
pdr.
dispatchLooksSoMuchCleanerEvent(). LGTM Minor nit: "Rewamp"->"Revamp".
5 years, 7 months ago (2015-05-20 22:08:53 UTC) #3
fs
On 2015/05/20 22:08:53, pdr wrote: > dispatchLooksSoMuchCleanerEvent(). LGTM > > > Minor nit: "Rewamp"->"Revamp". *hangs ...
5 years, 7 months ago (2015-05-21 07:58:00 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1150623003/1
5 years, 7 months ago (2015-05-21 07:58:13 UTC) #6
commit-bot: I haz the power
5 years, 7 months ago (2015-05-21 08:01:30 UTC) #7
Message was sent while issue was closed. 
Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=195678

Powered by Google App Engine
This is Rietveld 408576698