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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1274063003: [Loader] Make ThreadableLoader non-RefCounted and be managed by OwnPtr (Closed)

Created:
5 years, 4 months ago by hiroshige
Modified:
4 years, 9 months ago
CC:
blink-reviews, blink-reviews-events_chromium.org, blink-worker-reviews_chromium.org, dglazkov+blink, eae+blinkwatch, falken, gavinp+loader_chromium.org, horo+watch_chromium.org, Nate Chapin, kinuko+watch, kinuko+fileapi, kinuko+worker_chromium.org, nhiroki, tyoshino+watch_chromium.org, tzik
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[Loader] Make ThreadableLoader non-RefCounted and be managed by OwnPtr This CL makes ThreadableLoader non-RefCounted and to be managed by OwnPtr, as there is only one reference to ThreadableLoader after https://codereview.chromium.org/1262593004/. By this CL, the ownership of ThreadableLoader and the timing of ThreadableLoader destruction is clearer. BUG=518860 Committed: https://crrev.com/22002e9ce9be0df02da4a0f862fe46ceb8510896 Cr-Commit-Position: refs/heads/master@{#380764}

Patch Set 1 #

Patch Set 2 : Rebase. #

Patch Set 3 : #

Patch Set 4 : Rebase. #

Patch Set 5 : Rebase. #

Patch Set 6 : Rebase. #

Patch Set 7 : Rebasing. #

Patch Set 8 : Rebasing... #

Patch Set 9 : Rebase. #

Patch Set 10 : Rebase. #

Patch Set 11 : Rebase. #

Total comments: 20

Patch Set 12 : Rebase. #

Patch Set 13 : Reflect comments. #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+88 lines, -63 lines) Patch
M third_party/WebKit/Source/core/fileapi/FileReaderLoader.h View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +6 lines, -2 lines 2 comments Download
M third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp View 1 2 3 4 5 6 7 8 9 10 11 4 chunks +10 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/core/loader/MockThreadableLoader.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/loader/ThreadableLoader.h View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +4 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/core/loader/ThreadableLoader.cpp View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/loader/WorkerThreadableLoader.h View 1 2 3 4 5 6 7 8 2 chunks +3 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/loader/WorkerThreadableLoader.cpp View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/page/EventSource.h View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/workers/WorkerScriptLoader.h View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +2 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/modules/fetch/FetchBlobDataConsumerHandle.h View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/modules/fetch/FetchBlobDataConsumerHandle.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +4 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/modules/fetch/FetchBlobDataConsumerHandleTest.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 9 chunks +42 lines, -29 lines 0 comments Download
M third_party/WebKit/Source/modules/fetch/FetchFormDataConsumerHandleTest.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +3 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/modules/fetch/FetchManager.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +2 lines, -1 line 0 comments Download
M third_party/WebKit/Source/web/AssociatedURLLoader.h View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +2 lines, -1 line 2 comments Download

Dependent Patchsets:

Issue 1722893002 Patch 60001

Messages

Total messages: 46 (24 generated)
hiroshige
PTAL.
5 years, 4 months ago (2015-08-11 08:52:01 UTC) #3
kinuko
On 2015/08/11 08:52:01, hiroshige wrote: > PTAL. Wow, nice! (Just drive-by comment)
5 years, 4 months ago (2015-08-11 09:12:07 UTC) #4
tyoshino (SeeGerritForStatus)
lgtm! nice clean up!
5 years, 4 months ago (2015-08-13 04:26:43 UTC) #5
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1274063003/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1274063003/120001
5 years, 3 months ago (2015-09-15 12:50:17 UTC) #7
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/113616)
5 years, 3 months ago (2015-09-15 13:39:45 UTC) #9
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1274063003/220001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1274063003/220001
4 years, 10 months ago (2016-02-22 22:21:22 UTC) #15
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 10 months ago (2016-02-22 23:31:48 UTC) #17
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1274063003/240001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1274063003/240001
4 years, 10 months ago (2016-02-25 19:16:55 UTC) #19
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 10 months ago (2016-02-25 21:49:23 UTC) #21
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1274063003/240001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1274063003/240001
4 years, 10 months ago (2016-02-26 23:29:46 UTC) #23
hiroshige
Rebased and now all unit tests I added in https://codereview.chromium.org/1257343003/ pass. PTAL again.
4 years, 9 months ago (2016-02-29 16:31:44 UTC) #24
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1274063003/260001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1274063003/260001
4 years, 9 months ago (2016-02-29 16:35:37 UTC) #26
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 9 months ago (2016-02-29 17:57:41 UTC) #28
tyoshino (SeeGerritForStatus)
lgtm! https://codereview.chromium.org/1274063003/diff/260001/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h File third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h (right): https://codereview.chromium.org/1274063003/diff/260001/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h#newcode45 third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h:45: #include "wtf/PassRefPtr.h" Change to PassOwnPtr.h and add Handle.h ...
4 years, 9 months ago (2016-03-03 11:53:39 UTC) #29
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1274063003/320001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1274063003/320001
4 years, 9 months ago (2016-03-04 01:55:35 UTC) #32
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 9 months ago (2016-03-04 04:25:43 UTC) #34
hiroshige
+japhet@, could you take a look a core/ and web/ OWNER? https://codereview.chromium.org/1274063003/diff/260001/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h File third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h (right): ...
4 years, 9 months ago (2016-03-08 23:39:25 UTC) #36
Nate Chapin
LGTM https://codereview.chromium.org/1274063003/diff/320001/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h File third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h (right): https://codereview.chromium.org/1274063003/diff/320001/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h#newcode41 third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h:41: #include "platform/heap/Handle.h" Why this include? https://codereview.chromium.org/1274063003/diff/320001/third_party/WebKit/Source/web/AssociatedURLLoader.h File third_party/WebKit/Source/web/AssociatedURLLoader.h ...
4 years, 9 months ago (2016-03-09 17:51:09 UTC) #37
hiroshige
https://codereview.chromium.org/1274063003/diff/320001/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h File third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h (right): https://codereview.chromium.org/1274063003/diff/320001/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h#newcode41 third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h:41: #include "platform/heap/Handle.h" On 2016/03/09 17:51:09, Nate Chapin wrote: > ...
4 years, 9 months ago (2016-03-09 19:11:24 UTC) #38
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1274063003/320001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1274063003/320001
4 years, 9 months ago (2016-03-11 18:54:07 UTC) #42
commit-bot: I haz the power
Committed patchset #13 (id:320001)
4 years, 9 months ago (2016-03-11 22:42:14 UTC) #44
commit-bot: I haz the power
4 years, 9 months ago (2016-03-11 22:43:55 UTC) #46
Message was sent while issue was closed. 
Patchset 13 (id:??) landed as
https://crrev.com/22002e9ce9be0df02da4a0f862fe46ceb8510896
Cr-Commit-Position: refs/heads/master@{#380764}

Powered by Google App Engine
This is Rietveld 408576698