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

Issue 1310643003: Make classes and structures in core/events and core/fetch fast-allocated. (Closed)

Created:
5 years, 4 months ago by tasak
Modified:
5 years, 1 month ago
Reviewers:
haraken, Yuta Kitamura
CC:
blink-reviews, gavinp+loader_chromium.org, Nate Chapin, tyoshino+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Make classes and structures in core/events and core/fetch fast-allocated. Added STATIC_ONLY, STACK_ALLOCATED, DISALLOW_ALLOCATION, or ALLOW_ONLY_INLINE_ALLOCATION instead of WTF_MAKE_FAST_ALLOCATED(_WILL_BE_REMOVED) if possible. BUG=523249 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=201203

Patch Set 1 : #

Total comments: 4

Patch Set 2 : #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+46 lines, -4 lines) Patch
M Source/core/events/EventFactory.h View 1 2 chunks +2 lines, -0 lines 0 comments Download
M Source/core/events/EventPath.h View 1 chunk +2 lines, -0 lines 0 comments Download
M Source/core/events/EventTarget.h View 2 chunks +2 lines, -0 lines 2 comments Download
M Source/core/events/NavigatorEvents.h View 1 chunk +3 lines, -0 lines 0 comments Download
M Source/core/events/PointerIdManager.h View 2 chunks +2 lines, -0 lines 0 comments Download
M Source/core/events/ScopedEventQueue.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/events/WindowEventContext.h View 1 chunk +2 lines, -0 lines 0 comments Download
M Source/core/fetch/ClientHintsPreferences.h View 2 chunks +2 lines, -0 lines 0 comments Download
M Source/core/fetch/CrossOriginAccessControl.h View 2 chunks +2 lines, -0 lines 0 comments Download
M Source/core/fetch/DocumentResourceReference.h View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/fetch/FetchInitiatorInfo.h View 2 chunks +3 lines, -0 lines 0 comments Download
M Source/core/fetch/FetchRequest.h View 1 chunk +3 lines, -0 lines 0 comments Download
M Source/core/fetch/FetchUtils.h View 3 chunks +2 lines, -3 lines 0 comments Download
M Source/core/fetch/MemoryCache.h View 3 chunks +3 lines, -0 lines 0 comments Download
M Source/core/fetch/Resource.h View 3 chunks +3 lines, -0 lines 0 comments Download
M Source/core/fetch/ResourceClientWalker.h View 2 chunks +2 lines, -0 lines 0 comments Download
M Source/core/fetch/ResourceFetcher.h View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/fetch/ResourceLoadPriorityOptimizer.h View 1 chunk +2 lines, -0 lines 0 comments Download
M Source/core/fetch/ResourceLoaderOptions.h View 3 chunks +4 lines, -0 lines 0 comments Download
M Source/core/fetch/ResourcePtr.h View 1 chunk +2 lines, -0 lines 0 comments Download
M Source/core/fetch/SubstituteData.h View 2 chunks +2 lines, -0 lines 0 comments Download

Messages

Total messages: 17 (7 generated)
tasak
Would you review this CL?
5 years, 4 months ago (2015-08-24 10:51:51 UTC) #4
haraken
https://codereview.chromium.org/1310643003/diff/40001/Source/core/events/EventFactory.h File Source/core/events/EventFactory.h (right): https://codereview.chromium.org/1310643003/diff/40001/Source/core/events/EventFactory.h#newcode48 Source/core/events/EventFactory.h:48: WTF_MAKE_FAST_ALLOCATED(EventFactory); Our plan is to add the macro to ...
5 years, 4 months ago (2015-08-24 10:59:13 UTC) #5
tasak
Thank you for review. https://codereview.chromium.org/1310643003/diff/40001/Source/core/events/EventFactory.h File Source/core/events/EventFactory.h (right): https://codereview.chromium.org/1310643003/diff/40001/Source/core/events/EventFactory.h#newcode48 Source/core/events/EventFactory.h:48: WTF_MAKE_FAST_ALLOCATED(EventFactory); On 2015/08/24 10:59:12, haraken ...
5 years, 4 months ago (2015-08-25 03:54:41 UTC) #6
haraken
On 2015/08/25 03:54:41, tasak wrote: > Thank you for review. > > https://codereview.chromium.org/1310643003/diff/40001/Source/core/events/EventFactory.h > File ...
5 years, 4 months ago (2015-08-25 03:56:20 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1310643003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1310643003/60001
5 years, 4 months ago (2015-08-25 09:12:44 UTC) #9
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/103739)
5 years, 4 months ago (2015-08-25 10:16:12 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1310643003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1310643003/60001
5 years, 4 months ago (2015-08-26 03:55:52 UTC) #13
commit-bot: I haz the power
Committed patchset #2 (id:60001) as https://src.chromium.org/viewvc/blink?view=rev&revision=201203
5 years, 4 months ago (2015-08-26 06:16:43 UTC) #14
Yuta Kitamura
tasak: I've come across with this change and I have a question. https://codereview.chromium.org/1310643003/diff/60001/Source/core/events/EventTarget.h File Source/core/events/EventTarget.h ...
5 years, 1 month ago (2015-10-26 08:15:54 UTC) #16
tasak
5 years, 1 month ago (2015-10-26 08:19:01 UTC) #17
Message was sent while issue was closed.
https://codereview.chromium.org/1310643003/diff/60001/Source/core/events/Even...
File Source/core/events/EventTarget.h (right):

https://codereview.chromium.org/1310643003/diff/60001/Source/core/events/Even...
Source/core/events/EventTarget.h:54: ALLOW_ONLY_INLINE_ALLOCATION();
On 2015/10/26 08:15:54, Yuta Kitamura wrote:
> Is this needed?
> 
> FiringEventIterator only contains off-heap objects, so having
> ALLOW_ONLY_INLINE_ALLOCATION() here seems meritless.

I added this to ensure that no one invokes FiringEventIterator's new / delete.

Powered by Google App Engine
This is Rietveld 408576698