183042 2018-02-22 09:25:46 -0800 Add release logging for CacheStorage::Engine disk related functions 2018-02-22 10:53:11 -0800 1 1 1 Unclassified WebKit Service Workers WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 youennf youennf cdumez commit-queue webkit-bug-importer oldest_to_newest 1401435 0 youennf 2018-02-22 09:25:46 -0800 Add release logging for CacheStorage::Engine disk related functions 1401436 1 334456 youennf 2018-02-22 09:26:48 -0800 Created attachment 334456 Patch 1401437 2 334456 cdumez 2018-02-22 09:28:42 -0800 Comment on attachment 334456 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. 1401438 3 youennf 2018-02-22 09:36:41 -0800 (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. 1401439 4 cdumez 2018-02-22 09:37:43 -0800 (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. 1401440 5 334456 cdumez 2018-02-22 09:38:14 -0800 Comment on attachment 334456 Patch r=me with the added assertions. 1401442 6 youennf 2018-02-22 09:40:39 -0800 (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. 1401451 7 youennf 2018-02-22 10:32:25 -0800 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. 1401456 8 334456 commit-queue 2018-02-22 10:36:24 -0800 Comment on attachment 334456 Patch Rejecting attachment 334456 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 1401466 9 334456 commit-queue 2018-02-22 10:51:58 -0800 Comment on attachment 334456 Patch Clearing flags on attachment: 334456 Committed r228925: <https://trac.webkit.org/changeset/228925> 1401467 10 commit-queue 2018-02-22 10:51:59 -0800 All reviewed patches have been landed. Closing bug. 1401468 11 webkit-bug-importer 2018-02-22 10:53:11 -0800 <rdar://problem/37793473> 334456 2018-02-22 09:26:48 -0800 2018-02-22 10:51:58 -0800 Patch bug-183042-20180222092629.patch text/plain 2314 youennf U3VidmVyc2lvbiBSZXZpc2lvbjogMjI4ODU4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDBiZThhYmNiNmM2NDhjMjA5 MTYzMDVhYzY4MGMwOGRkNmMyNzI5MWYuLjE0NWZhZWM0OWQzNjQzZDEyOTEwNGJkMTMxNWI1Zjk1 ZWI4Mjk2NDQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTQgQEAKKzIwMTgtMDItMjIgIFlvdWVubiBG YWJsZXQgIDx5b3Vlbm5AYXBwbGUuY29tPgorCisgICAgICAgIEFkZCByZWxlYXNlIGxvZ2dpbmcg Zm9yIENhY2hlU3RvcmFnZTo6RW5naW5lIGRpc2sgcmVsYXRlZCBmdW5jdGlvbnMKKyAgICAgICAg aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE4MzA0MgorCisgICAgICAg IFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogTmV0d29ya1Byb2Nlc3Mv Y2FjaGUvQ2FjaGVTdG9yYWdlRW5naW5lLmNwcDoKKyAgICAgICAgKFdlYktpdDo6Q2FjaGVTdG9y YWdlOjpFbmdpbmU6OndyaXRlRmlsZSk6CisgICAgICAgIChXZWJLaXQ6OkNhY2hlU3RvcmFnZTo6 RW5naW5lOjpyZWFkRmlsZSk6CisKIDIwMTgtMDItMjEgIFlvdWVubiBGYWJsZXQgIDx5b3Vlbm5A YXBwbGUuY29tPgogCiAgICAgICAgIEFkZCByZWxlYXNlIGFzc2VydHMgZm9yIHNlcnZpY2Ugd29y a2VyIGZldGNoIGFuZCBwb3N0TWVzc2FnZSBldmVudHMKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJL aXQvTmV0d29ya1Byb2Nlc3MvY2FjaGUvQ2FjaGVTdG9yYWdlRW5naW5lLmNwcCBiL1NvdXJjZS9X ZWJLaXQvTmV0d29ya1Byb2Nlc3MvY2FjaGUvQ2FjaGVTdG9yYWdlRW5naW5lLmNwcAppbmRleCA5 YmM4ZmE4MzM1MGIxY2Y3OWYxNWFiZWYxMTE4ZDhiZjg3ZDcwOWRjLi4xYTkxOTY2NDljNmQ4MTg0 OWY2OGIxY2UzMDEzN2U2NjhjNGMyNzU2IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0L05ldHdv cmtQcm9jZXNzL2NhY2hlL0NhY2hlU3RvcmFnZUVuZ2luZS5jcHAKKysrIGIvU291cmNlL1dlYktp dC9OZXR3b3JrUHJvY2Vzcy9jYWNoZS9DYWNoZVN0b3JhZ2VFbmdpbmUuY3BwCkBAIC0yNiw2ICsy Niw3IEBACiAjaW5jbHVkZSAiY29uZmlnLmgiCiAjaW5jbHVkZSAiQ2FjaGVTdG9yYWdlRW5naW5l LmgiCiAKKyNpbmNsdWRlICJMb2dnaW5nLmgiCiAjaW5jbHVkZSAiTmV0d29ya0NhY2hlRmlsZVN5 c3RlbS5oIgogI2luY2x1ZGUgIk5ldHdvcmtDYWNoZUlPQ2hhbm5lbC5oIgogI2luY2x1ZGUgIk5l dHdvcmtQcm9jZXNzLmgiCkBAIC0yODEsNiArMjgyLDggQEAgdm9pZCBFbmdpbmU6OndyaXRlRmls ZShjb25zdCBTdHJpbmcmIGZpbGVuYW1lLCBOZXR3b3JrQ2FjaGU6OkRhdGEmJiBkYXRhLCBXZWJD b3IKICAgICAgICAgY2hhbm5lbC0+d3JpdGUoMCwgZGF0YSwgbnVsbHB0ciwgW2NhbGxiYWNrID0g V1RGTW92ZShjYWxsYmFjayldKGludCBlcnJvcikgbXV0YWJsZSB7CiAgICAgICAgICAgICBBU1NF UlQoUnVuTG9vcDo6aXNNYWluKCkpOwogICAgICAgICAgICAgaWYgKGVycm9yKSB7CisgICAgICAg ICAgICAgICAgUkVMRUFTRV9MT0dfRVJST1IoQ2FjaGVTdG9yYWdlLCAiQ2FjaGVTdG9yYWdlOjpF bmdpbmU6OndyaXRlRmlsZSBmYWlsZWQgd2l0aCBlcnJvciAlZCIsIGVycm9yKTsKKwogICAgICAg ICAgICAgICAgIGNhbGxiYWNrKEVycm9yOjpXcml0ZURpc2spOwogICAgICAgICAgICAgICAgIHJl dHVybjsKICAgICAgICAgICAgIH0KQEAgLTMwNiw2ICszMDksOCBAQCB2b2lkIEVuZ2luZTo6cmVh ZEZpbGUoY29uc3QgU3RyaW5nJiBmaWxlbmFtZSwgV1RGOjpGdW5jdGlvbjx2b2lkKGNvbnN0IE5l dHdvcmtDYQogICAgICAgICB9CiAKICAgICAgICAgY2hhbm5lbC0+cmVhZCgwLCBzdGQ6Om51bWVy aWNfbGltaXRzPHNpemVfdD46Om1heCgpLCBudWxscHRyLCBbY2FsbGJhY2sgPSBXVEZNb3ZlKGNh bGxiYWNrKV0oY29uc3QgRGF0YSYgZGF0YSwgaW50IGVycm9yKSBtdXRhYmxlIHsKKyAgICAgICAg ICAgIFJFTEVBU0VfTE9HX0VSUk9SX0lGKGVycm9yLCBDYWNoZVN0b3JhZ2UsICJDYWNoZVN0b3Jh Z2U6OkVuZ2luZTo6cmVhZEZpbGUgZmFpbGVkIHdpdGggZXJyb3IgJWQiLCBlcnJvcik7CisKICAg ICAgICAgICAgIC8vIEZJWE1FOiBXZSBzaG91bGQgZG8gdGhlIGRlY29kaW5nIGluIHRoZSBiYWNr Z3JvdW5kIHRocmVhZC4KICAgICAgICAgICAgIEFTU0VSVChSdW5Mb29wOjppc01haW4oKSk7CiAg ICAgICAgICAgICBjYWxsYmFjayhkYXRhLCBlcnJvcik7Cg==