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

Issue 1296243004: Oilpan: Move MainThreadTaskRunner into Oilpan heap. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 1 month ago by Yuta Kitamura
Modified:
2 years, 1 month ago
CC:
blink-reviews, blink-reviews-dom_chromium.org, dglazkov+blink, eae+blinkwatch, rwlbuis
Target Ref:
refs/remotes/origin/master
Project:
blink
Visibility:
Public.

Description

Oilpan: Move MainThreadTaskRunner into Oilpan heap. Fixes a raw pointer in MainThreadTaskRunner. BUG=509911 R=haraken@chromium.org, keishi@chromium.org, oilpan-reviews@chromium.org Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=200972

Patch Set 1 #

Total comments: 3

Patch Set 2 : Change WeakPtr to WeakPersistent in MainThreadTask. #

Total comments: 3

Patch Set 3 : Use CrossThreadWeakPersistent. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+41 lines, -14 lines) Patch
M Source/core/dom/Document.h View 1 2 1 chunk +1 line, -1 line 0 comments Download
M Source/core/dom/Document.cpp View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/dom/MainThreadTaskRunner.h View 1 3 chunks +13 lines, -7 lines 0 comments Download
M Source/core/dom/MainThreadTaskRunner.cpp View 1 2 5 chunks +23 lines, -3 lines 0 comments Download
M Source/core/dom/MainThreadTaskRunnerTest.cpp View 3 chunks +3 lines, -3 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 20 (2 generated)
Yuta Kitamura
2 years, 1 month ago (2015-08-18 06:48:34 UTC) #1
haraken
LGTM
2 years, 1 month ago (2015-08-18 07:04:18 UTC) #2
sof
https://codereview.chromium.org/1296243004/diff/1/Source/core/dom/MainThreadTaskRunner.h File Source/core/dom/MainThreadTaskRunner.h (left): https://codereview.chromium.org/1296243004/diff/1/Source/core/dom/MainThreadTaskRunner.h#oldcode67 Source/core/dom/MainThreadTaskRunner.h:67: WeakPtrFactory<MainThreadTaskRunner> m_weakFactory; Doesn't this need to be eagerly finalized?
2 years, 1 month ago (2015-08-18 07:15:31 UTC) #4
haraken
https://codereview.chromium.org/1296243004/diff/1/Source/core/dom/MainThreadTaskRunner.h File Source/core/dom/MainThreadTaskRunner.h (left): https://codereview.chromium.org/1296243004/diff/1/Source/core/dom/MainThreadTaskRunner.h#oldcode67 Source/core/dom/MainThreadTaskRunner.h:67: WeakPtrFactory<MainThreadTaskRunner> m_weakFactory; On 2015/08/18 07:15:31, sof wrote: > Doesn't ...
2 years, 1 month ago (2015-08-18 08:08:54 UTC) #5
sof
https://codereview.chromium.org/1296243004/diff/1/Source/core/dom/MainThreadTaskRunner.h File Source/core/dom/MainThreadTaskRunner.h (left): https://codereview.chromium.org/1296243004/diff/1/Source/core/dom/MainThreadTaskRunner.h#oldcode67 Source/core/dom/MainThreadTaskRunner.h:67: WeakPtrFactory<MainThreadTaskRunner> m_weakFactory; On 2015/08/18 08:08:54, haraken wrote: > On ...
2 years, 1 month ago (2015-08-18 13:54:45 UTC) #6
haraken
On 2015/08/18 13:54:45, sof wrote: > https://codereview.chromium.org/1296243004/diff/1/Source/core/dom/MainThreadTaskRunner.h > File Source/core/dom/MainThreadTaskRunner.h (left): > > https://codereview.chromium.org/1296243004/diff/1/Source/core/dom/MainThreadTaskRunner.h#oldcode67 > ...
2 years, 1 month ago (2015-08-18 14:03:49 UTC) #7
sof
On 2015/08/18 14:03:49, haraken wrote: > On 2015/08/18 13:54:45, sof wrote: > > > https://codereview.chromium.org/1296243004/diff/1/Source/core/dom/MainThreadTaskRunner.h ...
2 years, 1 month ago (2015-08-18 14:13:57 UTC) #8
sof
On 2015/08/18 14:13:57, sof wrote: > ... > That would be more fitting with Oilpan ...
2 years, 1 month ago (2015-08-18 14:17:24 UTC) #9
Yuta Kitamura
PTAL at PS2. I assumed eager finalization was not necessary as per sof's reply. (Correct ...
2 years, 1 month ago (2015-08-19 05:10:17 UTC) #10
haraken
https://codereview.chromium.org/1296243004/diff/20001/Source/core/dom/MainThreadTaskRunner.cpp File Source/core/dom/MainThreadTaskRunner.cpp (right): https://codereview.chromium.org/1296243004/diff/20001/Source/core/dom/MainThreadTaskRunner.cpp#newcode88 Source/core/dom/MainThreadTaskRunner.cpp:88: Platform::current()->mainThread()->postTask(location, new MainThreadTask(createWeakPointerToSelf(), task, false)); Maybe I'm wrong but ...
2 years, 1 month ago (2015-08-19 05:18:17 UTC) #11
Yuta Kitamura
https://codereview.chromium.org/1296243004/diff/20001/Source/core/dom/MainThreadTaskRunner.cpp File Source/core/dom/MainThreadTaskRunner.cpp (right): https://codereview.chromium.org/1296243004/diff/20001/Source/core/dom/MainThreadTaskRunner.cpp#newcode88 Source/core/dom/MainThreadTaskRunner.cpp:88: Platform::current()->mainThread()->postTask(location, new MainThreadTask(createWeakPointerToSelf(), task, false)); On 2015/08/19 05:18:17, haraken ...
2 years, 1 month ago (2015-08-19 05:56:58 UTC) #12
haraken
On 2015/08/19 05:56:58, Yuta Kitamura wrote: > https://codereview.chromium.org/1296243004/diff/20001/Source/core/dom/MainThreadTaskRunner.cpp > File Source/core/dom/MainThreadTaskRunner.cpp (right): > > https://codereview.chromium.org/1296243004/diff/20001/Source/core/dom/MainThreadTaskRunner.cpp#newcode88 ...
2 years, 1 month ago (2015-08-19 06:02:58 UTC) #13
sof
https://codereview.chromium.org/1296243004/diff/20001/Source/core/dom/MainThreadTaskRunner.cpp File Source/core/dom/MainThreadTaskRunner.cpp (left): https://codereview.chromium.org/1296243004/diff/20001/Source/core/dom/MainThreadTaskRunner.cpp#oldcode51 Source/core/dom/MainThreadTaskRunner.cpp:51: WeakPtr<MainThreadTaskRunner> m_runner; The GC plugin ought to warn/disallow WeakPtr<T> ...
2 years, 1 month ago (2015-08-19 06:37:56 UTC) #14
haraken
On 2015/08/19 06:37:56, sof wrote: > https://codereview.chromium.org/1296243004/diff/20001/Source/core/dom/MainThreadTaskRunner.cpp > File Source/core/dom/MainThreadTaskRunner.cpp (left): > > https://codereview.chromium.org/1296243004/diff/20001/Source/core/dom/MainThreadTaskRunner.cpp#oldcode51 > ...
2 years, 1 month ago (2015-08-19 06:46:17 UTC) #15
Yuta Kitamura
CrossThreadWeakPersistent is landed. PTAL.
2 years, 1 month ago (2015-08-20 09:46:21 UTC) #16
haraken
On 2015/08/20 09:46:21, Yuta Kitamura wrote: > CrossThreadWeakPersistent is landed. PTAL. Thanks, LGTM
2 years, 1 month ago (2015-08-20 09:47:05 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1296243004/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1296243004/40001
2 years, 1 month ago (2015-08-21 01:55:30 UTC) #19
commit-bot: I haz the power
2 years, 1 month ago (2015-08-21 07:31:44 UTC) #20
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=200972
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b