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

Issue 314333002: Enable Oilpan by default in modules/filesystem/ (Closed)

Created:
6 years, 6 months ago by sof
Modified:
6 years, 6 months ago
CC:
blink-reviews, jamesr, arv+blink, tzik, nhiroki, abarth-chromium, dglazkov+blink, blink-reviews-bindings_chromium.org, kinuko+fileapi
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Patch Set 1 #

Patch Set 2 : Non-oilpan compile fixes (GC plugin) #

Patch Set 3 : Tidy up #

Total comments: 26

Patch Set 4 : Make FileWriterSync [GarbageCollected] #

Patch Set 5 : No longer use transition type for FileWriter constructor either #

Patch Set 6 : Rebased + be const-consistent in WebDOMFileSystem constructor #

Total comments: 2

Patch Set 7 : Remove consts from conversion ctor + copy assignment op #

Unified diffs Side-by-side diffs Delta from patch set Stats (+246 lines, -215 lines) Patch
M Source/bindings/scripts/v8_callback_interface.py View 1 chunk +1 line, -1 line 0 comments Download
M Source/bindings/v8/SerializedScriptValue.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/modules/filesystem/DOMFilePath.h View 1 chunk +0 lines, -1 line 0 comments Download
M Source/modules/filesystem/DOMFileSystem.h View 1 2 6 chunks +42 lines, -6 lines 0 comments Download
M Source/modules/filesystem/DOMFileSystem.cpp View 1 2 3 4 4 chunks +11 lines, -11 lines 0 comments Download
M Source/modules/filesystem/DOMFileSystem.idl View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/filesystem/DOMFileSystemBase.h View 3 chunks +2 lines, -3 lines 0 comments Download
M Source/modules/filesystem/DOMFileSystemBase.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/filesystem/DOMFileSystemSync.h View 1 2 3 1 chunk +5 lines, -5 lines 0 comments Download
M Source/modules/filesystem/DOMFileSystemSync.cpp View 1 2 3 4 chunks +8 lines, -8 lines 0 comments Download
M Source/modules/filesystem/DOMFileSystemSync.idl View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/filesystem/DataTransferItemFileSystem.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/filesystem/DataTransferItemFileSystem.cpp View 1 2 3 2 chunks +5 lines, -5 lines 0 comments Download
M Source/modules/filesystem/DirectoryEntry.h View 3 chunks +4 lines, -5 lines 0 comments Download
M Source/modules/filesystem/DirectoryEntry.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/modules/filesystem/DirectoryEntrySync.h View 2 chunks +6 lines, -7 lines 0 comments Download
M Source/modules/filesystem/DirectoryEntrySync.cpp View 1 chunk +6 lines, -6 lines 0 comments Download
M Source/modules/filesystem/DirectoryReader.h View 2 chunks +6 lines, -8 lines 0 comments Download
M Source/modules/filesystem/DirectoryReader.cpp View 1 2 3 4 chunks +7 lines, -7 lines 0 comments Download
M Source/modules/filesystem/DirectoryReader.idl View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/filesystem/DirectoryReaderBase.h View 2 chunks +3 lines, -4 lines 0 comments Download
M Source/modules/filesystem/DirectoryReaderSync.h View 3 chunks +4 lines, -6 lines 0 comments Download
M Source/modules/filesystem/DirectoryReaderSync.cpp View 1 2 3 3 chunks +5 lines, -5 lines 0 comments Download
M Source/modules/filesystem/DirectoryReaderSync.idl View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/filesystem/DraggedIsolatedFileSystem.h View 1 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/filesystem/EntriesCallback.h View 1 chunk +1 line, -2 lines 0 comments Download
M Source/modules/filesystem/Entry.h View 2 chunks +3 lines, -4 lines 0 comments Download
M Source/modules/filesystem/Entry.cpp View 2 chunks +5 lines, -5 lines 0 comments Download
M Source/modules/filesystem/Entry.idl View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/filesystem/EntryBase.h View 3 chunks +3 lines, -4 lines 0 comments Download
M Source/modules/filesystem/EntryBase.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/filesystem/EntrySync.h View 2 chunks +6 lines, -7 lines 0 comments Download
M Source/modules/filesystem/EntrySync.cpp View 2 chunks +8 lines, -8 lines 0 comments Download
M Source/modules/filesystem/EntrySync.idl View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/filesystem/FileEntry.h View 2 chunks +3 lines, -3 lines 0 comments Download
M Source/modules/filesystem/FileEntry.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/filesystem/FileEntrySync.h View 1 2 3 2 chunks +4 lines, -5 lines 0 comments Download
M Source/modules/filesystem/FileEntrySync.cpp View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M Source/modules/filesystem/FileSystemCallbacks.h View 1 2 3 4 5 chunks +16 lines, -13 lines 0 comments Download
M Source/modules/filesystem/FileSystemCallbacks.cpp View 1 8 chunks +25 lines, -14 lines 0 comments Download
M Source/modules/filesystem/FileWriter.h View 1 2 3 4 1 chunk +2 lines, -2 lines 0 comments Download
M Source/modules/filesystem/FileWriter.cpp View 1 2 3 4 1 chunk +3 lines, -3 lines 0 comments Download
M Source/modules/filesystem/FileWriter.idl View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/filesystem/FileWriterBase.h View 1 chunk +1 line, -2 lines 0 comments Download
M Source/modules/filesystem/FileWriterSync.h View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M Source/modules/filesystem/FileWriterSync.idl View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/filesystem/HTMLInputElementFileSystem.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/filesystem/InspectorFileSystemAgent.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/modules/filesystem/InspectorFrontendHostFileSystem.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/filesystem/InspectorFrontendHostFileSystem.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/filesystem/Metadata.h View 1 chunk +5 lines, -6 lines 0 comments Download
M Source/modules/filesystem/Metadata.idl View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/filesystem/SyncCallbackHelper.h View 1 2 3 2 chunks +5 lines, -5 lines 0 comments Download
M Source/modules/filesystem/WorkerGlobalScopeFileSystem.h View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/modules/filesystem/WorkerGlobalScopeFileSystem.cpp View 1 2 3 3 chunks +7 lines, -10 lines 0 comments Download
M Source/web/WebDOMFileSystem.cpp View 1 2 3 4 5 6 2 chunks +3 lines, -3 lines 0 comments Download
M public/web/WebDOMFileSystem.h View 1 2 3 4 5 6 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 15 (0 generated)
sof
Please take a look. Verified with clang & the GC plugin with enable_oilpan={0,1}
6 years, 6 months ago (2014-06-06 15:25:57 UTC) #1
haraken
https://codereview.chromium.org/314333002/diff/40001/Source/modules/filesystem/DOMFileSystem.h File Source/modules/filesystem/DOMFileSystem.h (right): https://codereview.chromium.org/314333002/diff/40001/Source/modules/filesystem/DOMFileSystem.h#newcode125 Source/modules/filesystem/DOMFileSystem.h:125: RefPtrWillBePersistent<CBArg> m_callbackArg; Just to confirm: Not related to CL, ...
6 years, 6 months ago (2014-06-07 08:11:04 UTC) #2
sof
https://codereview.chromium.org/314333002/diff/40001/Source/modules/filesystem/DOMFileSystem.h File Source/modules/filesystem/DOMFileSystem.h (right): https://codereview.chromium.org/314333002/diff/40001/Source/modules/filesystem/DOMFileSystem.h#newcode125 Source/modules/filesystem/DOMFileSystem.h:125: RefPtrWillBePersistent<CBArg> m_callbackArg; On 2014/06/07 08:11:04, haraken wrote: > > ...
6 years, 6 months ago (2014-06-07 13:18:54 UTC) #3
haraken
https://codereview.chromium.org/314333002/diff/40001/Source/modules/filesystem/DirectoryReader.cpp File Source/modules/filesystem/DirectoryReader.cpp (right): https://codereview.chromium.org/314333002/diff/40001/Source/modules/filesystem/DirectoryReader.cpp#newcode93 Source/modules/filesystem/DirectoryReader.cpp:93: filesystem()->scheduleCallback(errorCallback, PassRefPtrWillBeRawPtr<FileError>(m_error.get())); On 2014/06/07 13:18:53, sof wrote: > On ...
6 years, 6 months ago (2014-06-07 16:45:27 UTC) #4
sof
https://codereview.chromium.org/314333002/diff/40001/Source/modules/filesystem/FileWriter.cpp File Source/modules/filesystem/FileWriter.cpp (right): https://codereview.chromium.org/314333002/diff/40001/Source/modules/filesystem/FileWriter.cpp#newcode47 Source/modules/filesystem/FileWriter.cpp:47: PassRefPtrWillBeRawPtr<FileWriter> FileWriter::create(ExecutionContext* context) On 2014/06/07 16:45:27, haraken wrote: > ...
6 years, 6 months ago (2014-06-08 16:34:34 UTC) #5
haraken
LGTM, thanks!
6 years, 6 months ago (2014-06-09 00:39:50 UTC) #6
haraken
tzik@: Would it be OK to enable oilpan by default for filesystem/ ? (kinuko-san is ...
6 years, 6 months ago (2014-06-09 00:40:58 UTC) #7
tzik
LGTM. OK, go ahead.
6 years, 6 months ago (2014-06-09 03:26:06 UTC) #8
sof
Source/web/ + public/ approval remains, but there is the same issue there as the one ...
6 years, 6 months ago (2014-06-09 15:24:31 UTC) #9
tkent
https://codereview.chromium.org/314333002/diff/100001/Source/web/WebDOMFileSystem.cpp File Source/web/WebDOMFileSystem.cpp (right): https://codereview.chromium.org/314333002/diff/100001/Source/web/WebDOMFileSystem.cpp#newcode142 Source/web/WebDOMFileSystem.cpp:142: WebDOMFileSystem::WebDOMFileSystem(const DOMFileSystem* domFileSystem) Please remove |const|. We should not ...
6 years, 6 months ago (2014-06-10 03:14:23 UTC) #10
sof
https://codereview.chromium.org/314333002/diff/100001/Source/web/WebDOMFileSystem.cpp File Source/web/WebDOMFileSystem.cpp (right): https://codereview.chromium.org/314333002/diff/100001/Source/web/WebDOMFileSystem.cpp#newcode142 Source/web/WebDOMFileSystem.cpp:142: WebDOMFileSystem::WebDOMFileSystem(const DOMFileSystem* domFileSystem) On 2014/06/10 03:14:22, tkent wrote: > ...
6 years, 6 months ago (2014-06-10 06:14:35 UTC) #11
tkent
lgtm
6 years, 6 months ago (2014-06-10 06:23:48 UTC) #12
sof
The CQ bit was checked by sigbjornf@opera.com
6 years, 6 months ago (2014-06-10 06:25:10 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sigbjornf@opera.com/314333002/120001
6 years, 6 months ago (2014-06-10 06:26:02 UTC) #14
commit-bot: I haz the power
6 years, 6 months ago (2014-06-10 07:22:45 UTC) #15
Message was sent while issue was closed.
Change committed as 175867

Powered by Google App Engine
This is Rietveld 408576698