Add release logging for CacheStorage::Engine disk related functions
Created attachment 334456 [details] Patch
Comment on attachment 334456 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334456&action=review > Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp:285 > + RELEASE_LOG_ERROR(CacheStorage, "CacheStorage::Engine::writeFile failed with error %d", error); An engine has an associated sessionID. You should check that the sessionID is not one for an ephemeral session before logging.
(In reply to Chris Dumez from comment #2) > Comment on attachment 334456 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=334456&action=review > > > Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp:285 > > + RELEASE_LOG_ERROR(CacheStorage, "CacheStorage::Engine::writeFile failed with error %d", error); > > An engine has an associated sessionID. You should check that the sessionID > is not one for an ephemeral session before logging. This is unnecessary as CacheStorage::Engine should not write any file for ephemeral sessions. We could add something like ASSERT(!sessionID.isEphemeral()) at the beginning of writeDisk/readDisk though.
(In reply to youenn fablet from comment #3) > (In reply to Chris Dumez from comment #2) > > Comment on attachment 334456 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=334456&action=review > > > > > Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp:285 > > > + RELEASE_LOG_ERROR(CacheStorage, "CacheStorage::Engine::writeFile failed with error %d", error); > > > > An engine has an associated sessionID. You should check that the sessionID > > is not one for an ephemeral session before logging. > > This is unnecessary as CacheStorage::Engine should not write any file for > ephemeral sessions. > We could add something like ASSERT(!sessionID.isEphemeral()) at the > beginning of writeDisk/readDisk though. Ok, please do that.
Comment on attachment 334456 [details] Patch r=me with the added assertions.
(In reply to youenn fablet from comment #3) > (In reply to Chris Dumez from comment #2) > > Comment on attachment 334456 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=334456&action=review > > > > > Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp:285 > > > + RELEASE_LOG_ERROR(CacheStorage, "CacheStorage::Engine::writeFile failed with error %d", error); > > > > An engine has an associated sessionID. You should check that the sessionID > > is not one for an ephemeral session before logging. > > This is unnecessary as CacheStorage::Engine should not write any file for > ephemeral sessions. > We could add something like ASSERT(!sessionID.isEphemeral()) at the > beginning of writeDisk/readDisk though. Actually, Engine does not store any sessionID. In case of ephemeral session, m_rootPath is null and we are checking whether it is null at the beginning of read/write methods.
Chris, I'll land the patch as is given the persistency check is close to the release logging and given the fact engine is not storing its sessionID. Let me know if that does not work for you.
Comment on attachment 334456 [details] Patch Rejecting attachment 334456 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 334456, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: rdparty/autoinstalled/mechanize/_urllib2_fork.py", line 332, in _call_chain result = func(*args) File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_urllib2_fork.py", line 1170, in https_open return self.do_open(conn_factory, req) File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_urllib2_fork.py", line 1118, in do_open raise URLError(err) urllib2.URLError: <urlopen error [Errno 60] Operation timed out> Full output: http://webkit-queues.webkit.org/results/6624429
Comment on attachment 334456 [details] Patch Clearing flags on attachment: 334456 Committed r228925: <https://trac.webkit.org/changeset/228925>
All reviewed patches have been landed. Closing bug.
<rdar://problem/37793473>