199925 2019-07-18 17:46:55 -0700 Web Inspector: Page: mark any WebCore::Setting inspector override as UNLIKELY 2019-07-22 12:47:50 -0700 1 1 1 Unclassified WebKit Web Inspector WebKit Nightly Build All All RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=193813 InRadar P2 Normal --- 1 hi hi commit-queue hi inspector-bugzilla-changes joepeck webkit-bug-importer oldest_to_newest 1554095 0 hi 2019-07-18 17:46:55 -0700 This way, if the inspector crashes, we don't still use the overridden settings value. 1554097 1 374433 hi 2019-07-18 17:52:59 -0700 Created attachment 374433 Patch 1554115 2 374433 joepeck 2019-07-18 20:51:16 -0700 Comment on attachment 374433 Patch Ideally this is already always nullopt if there is no inspector frontend, right? Maybe we should just ASSERT(InspectorInstrumentation::hasFrontends()) inside the if. 1554116 3 joepeck 2019-07-18 20:52:17 -0700 (In reply to Devin Rousso from comment #0) > This way, if the inspector crashes, we don't still use the overridden > settings value. Shouldn't the InspectorPageAgent have cleaned up after itself in: void InspectorPageAgent::willDestroyFrontendAndBackend(Inspector::DisconnectReason) { ErrorString unused; disable(unused); } Which should set all override values to nullopt. 1554122 4 hi 2019-07-18 21:31:22 -0700 (In reply to Joseph Pecoraro from comment #3) > (In reply to Devin Rousso from comment #0) > > This way, if the inspector crashes, we don't still use the overridden settings value. > > Shouldn't the InspectorPageAgent have cleaned up after itself in: > > void > InspectorPageAgent::willDestroyFrontendAndBackend(Inspector::DisconnectReason) > { > ErrorString unused; > disable(unused); > } > > Which should set all override values to nullopt. Yes, but what about if the inspector crashes? We could potentially end up in a state where the override didn't get removed. Due to the potentially sensitive nature of some of these settings (e.g. CORS), this is an added precaution. 1554744 5 joepeck 2019-07-22 11:18:02 -0700 (In reply to Devin Rousso from comment #4) > (In reply to Joseph Pecoraro from comment #3) > > (In reply to Devin Rousso from comment #0) > > > This way, if the inspector crashes, we don't still use the overridden settings value. > > > > Shouldn't the InspectorPageAgent have cleaned up after itself in: > > > > void > > InspectorPageAgent::willDestroyFrontendAndBackend(Inspector::DisconnectReason) > > { > > ErrorString unused; > > disable(unused); > > } > > > > Which should set all override values to nullopt. > Yes, but what about if the inspector crashes? If the: • Inspected Page Web Content Process crashes - this state goes away and a new process will be spawned with its own WebCore::Settings • Inspector Web Content Process crashes - InspectorPageAgent::willDestroyFrontendAndBackend should run since a frontend went away • UIProcess Crashes - all of these processes go away Am I overlooking something? 1554763 6 hi 2019-07-22 11:37:33 -0700 (In reply to Joseph Pecoraro from comment #5) > • Inspector Web Content Process crashes > - InspectorPageAgent::willDestroyFrontendAndBackend should run since a frontend went away Oh, I didn't know we did that =D In that case, I agree that this isn't "necessary". I'll change it to just be an `ASSERT`. 1554766 7 374615 hi 2019-07-22 11:40:55 -0700 Created attachment 374615 Patch 1554789 8 374615 joepeck 2019-07-22 12:37:54 -0700 Comment on attachment 374615 Patch r=me 1554794 9 374615 commit-queue 2019-07-22 12:46:10 -0700 Comment on attachment 374615 Patch Clearing flags on attachment: 374615 Committed r247697: <https://trac.webkit.org/changeset/247697> 1554795 10 commit-queue 2019-07-22 12:46:12 -0700 All reviewed patches have been landed. Closing bug. 1554797 11 webkit-bug-importer 2019-07-22 12:47:50 -0700 <rdar://problem/53411226> 374433 2019-07-18 17:52:59 -0700 2019-07-22 11:40:38 -0700 Patch bug-199925-20190718175258.patch text/plain 2172 hi ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCA2NWYxODZkZDYyNGFlZWE4OTE2Yjg4NDJkZjc1OTlhMDgxZTZhMzhkLi4y NWFjNGEyMmZjZGM0OTQxZDhiMzA4ZTQ2MmJkMWIyZDI2ZDIyZTZiIDEwMDY0NAotLS0gYS9Tb3Vy Y2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0x LDMgKzEsMTMgQEAKKzIwMTktMDctMTggIERldmluIFJvdXNzbyAgPGRyb3Vzc29AYXBwbGUuY29t PgorCisgICAgICAgIFdlYiBJbnNwZWN0b3I6IFBhZ2U6IG9ubHkgYXBwbHkgb3ZlcnJpZGRlbiBz ZXR0aW5ncyBpZiBhbiBpbnNwZWN0b3IgaXMgY29ubmVjdGVkCisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xOTk5MjUKKworICAgICAgICBSZXZpZXdlZCBi eSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFNjcmlwdHMvU2V0dGluZ3NUZW1wbGF0ZXMv U2V0dGluZ3MuY3BwLmVyYjoKKyAgICAgICAgRG9uJ3QgdXNlIHRoZSBgbV8qSW5zcGVjdG9yT3Zl cnJpZGVgIHZhbHVlIGlmIHRoZXJlIGlzIG5vIGNvbm5lY3RlZCBpbnNwZWN0b3IuCisKIDIwMTkt MDctMTcgIENocmlzdG9waGVyIFJlaWQgIDxjaHJpcy5yZWlkQHNvbnkuY29tPgogCiAgICAgICAg IEJ5dGVjb2RlIGNhY2hlIHNob3VsZCB1c2UgRmlsZVN5c3RlbQpkaWZmIC0tZ2l0IGEvU291cmNl L1dlYkNvcmUvU2NyaXB0cy9TZXR0aW5nc1RlbXBsYXRlcy9TZXR0aW5ncy5jcHAuZXJiIGIvU291 cmNlL1dlYkNvcmUvU2NyaXB0cy9TZXR0aW5nc1RlbXBsYXRlcy9TZXR0aW5ncy5jcHAuZXJiCmlu ZGV4IDAzNjg2N2M1OWRmM2IzMGQzMmRkM2FmMjhhNTQyNzFkMGIwYjc0MjUuLmE3YjgzY2I5NThk NjI2YTRiZmY1M2QyOWYxMjBmZmRiYmVhNzMzZmYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3Jl L1NjcmlwdHMvU2V0dGluZ3NUZW1wbGF0ZXMvU2V0dGluZ3MuY3BwLmVyYgorKysgYi9Tb3VyY2Uv V2ViQ29yZS9TY3JpcHRzL1NldHRpbmdzVGVtcGxhdGVzL1NldHRpbmdzLmNwcC5lcmIKQEAgLTI4 LDYgKzI4LDcgQEAKICNpbmNsdWRlICJjb25maWcuaCIKICNpbmNsdWRlICJTZXR0aW5ncy5oIgog CisjaW5jbHVkZSAiSW5zcGVjdG9ySW5zdHJ1bWVudGF0aW9uLmgiCiAjaW5jbHVkZSAiUGFnZS5o IgogI2luY2x1ZGUgIlNldHRpbmdzRGVmYXVsdFZhbHVlcy5oIgogCkBAIC03NSw3ICs3Niw3IEBA IFNldHRpbmdzOjp+U2V0dGluZ3MoKQogPCU9IEBzZXR0aW5nLnBhcmFtZXRlclR5cGUgJT4gU2V0 dGluZ3M6OjwlPSBAc2V0dGluZy5nZXR0ZXJGdW5jdGlvbk5hbWUgJT4oKSBjb25zdAogewogPCUt IGlmIEBzZXR0aW5nLmhhc0luc3BlY3Rvck92ZXJyaWRlPyAtJT4KLSAgICBpZiAobV88JT0gQHNl dHRpbmcubmFtZSAlPkluc3BlY3Rvck92ZXJyaWRlKQorICAgIGlmIChVTkxJS0VMWShtXzwlPSBA c2V0dGluZy5uYW1lICU+SW5zcGVjdG9yT3ZlcnJpZGUgJiYgSW5zcGVjdG9ySW5zdHJ1bWVudGF0 aW9uOjpoYXNGcm9udGVuZHMoKSkpCiAgICAgICAgIHJldHVybiBtXzwlPSBAc2V0dGluZy5uYW1l ICU+SW5zcGVjdG9yT3ZlcnJpZGUudmFsdWUoKTsKIDwlLSBlbmQgLSU+CiAgICAgcmV0dXJuIG1f PCU9IEBzZXR0aW5nLm5hbWUgJT47CkBAIC05OSw3ICsxMDAsNyBAQCB2b2lkIFNldHRpbmdzOjo8 JT0gQHNldHRpbmcuc2V0dGVyRnVuY3Rpb25OYW1lICU+KDwlPSBAc2V0dGluZy5wYXJhbWV0ZXJU eXBlICU+CiA8JT0gQHNldHRpbmcucGFyYW1ldGVyVHlwZSAlPiBTZXR0aW5nczo6PCU9IEBzZXR0 aW5nLmdldHRlckZ1bmN0aW9uTmFtZSAlPigpIGNvbnN0CiB7CiA8JS0gaWYgQHNldHRpbmcuaGFz SW5zcGVjdG9yT3ZlcnJpZGU/IC0lPgotICAgIGlmIChtXzwlPSBAc2V0dGluZy5uYW1lICU+SW5z cGVjdG9yT3ZlcnJpZGUpCisgICAgaWYgKFVOTElLRUxZKG1fPCU9IEBzZXR0aW5nLm5hbWUgJT5J bnNwZWN0b3JPdmVycmlkZSAmJiBJbnNwZWN0b3JJbnN0cnVtZW50YXRpb246Omhhc0Zyb250ZW5k cygpKSkKICAgICAgICAgcmV0dXJuIG1fPCU9IEBzZXR0aW5nLm5hbWUgJT5JbnNwZWN0b3JPdmVy cmlkZS52YWx1ZSgpOwogPCUtIGVuZCAtJT4KICAgICByZXR1cm4gbV88JT0gQHNldHRpbmcubmFt ZSAlPjsK 374615 2019-07-22 11:40:55 -0700 2019-07-22 12:46:10 -0700 Patch bug-199925-20190722114055.patch text/plain 2150 hi ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCBlMTQ5YmNjZmUyZTY1NzU4MTg3YTlkZDJlYTZkZWQ3MGM5N2IwMmZmLi5i MDI0MzJkOGRjYzVjMzM5OTA2OWQyMDJmNjY5OTcyOGVjNmFmYjE5IDEwMDY0NAotLS0gYS9Tb3Vy Y2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0x LDMgKzEsMTIgQEAKKzIwMTktMDctMTggIERldmluIFJvdXNzbyAgPGRyb3Vzc29AYXBwbGUuY29t PgorCisgICAgICAgIFdlYiBJbnNwZWN0b3I6IFBhZ2U6IG1hcmsgYW55IFdlYkNvcmU6OlNldHRp bmcgaW5zcGVjdG9yIG92ZXJyaWRlIGFzIFVOTElLRUxZCisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xOTk5MjUKKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFNjcmlwdHMvU2V0dGluZ3NUZW1wbGF0ZXMvU2V0 dGluZ3MuY3BwLmVyYjoKKwogMjAxOS0wNy0xOCAgQW50b2luZSBRdWludCAgPGdyYW91dHNAYXBw bGUuY29tPgogCiAgICAgICAgIEVuYWJsZSBzaW11bGF0ZWQgbW91c2UgZXZlbnRzIHF1aXJrIGZv ciBzb3VuZGNsb3VkLmNvbQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvU2NyaXB0cy9TZXR0 aW5nc1RlbXBsYXRlcy9TZXR0aW5ncy5jcHAuZXJiIGIvU291cmNlL1dlYkNvcmUvU2NyaXB0cy9T ZXR0aW5nc1RlbXBsYXRlcy9TZXR0aW5ncy5jcHAuZXJiCmluZGV4IDAzNjg2N2M1OWRmM2IzMGQz MmRkM2FmMjhhNTQyNzFkMGIwYjc0MjUuLmZiZDMyNWIxMjcwYmE0OTFjNmNhOWJiYTYwMTA2Yzgw NmFkYmJlYzYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL1NjcmlwdHMvU2V0dGluZ3NUZW1w bGF0ZXMvU2V0dGluZ3MuY3BwLmVyYgorKysgYi9Tb3VyY2UvV2ViQ29yZS9TY3JpcHRzL1NldHRp bmdzVGVtcGxhdGVzL1NldHRpbmdzLmNwcC5lcmIKQEAgLTI4LDYgKzI4LDcgQEAKICNpbmNsdWRl ICJjb25maWcuaCIKICNpbmNsdWRlICJTZXR0aW5ncy5oIgogCisjaW5jbHVkZSAiSW5zcGVjdG9y SW5zdHJ1bWVudGF0aW9uLmgiCiAjaW5jbHVkZSAiUGFnZS5oIgogI2luY2x1ZGUgIlNldHRpbmdz RGVmYXVsdFZhbHVlcy5oIgogCkBAIC03NSw4ICs3NiwxMCBAQCBTZXR0aW5nczo6flNldHRpbmdz KCkKIDwlPSBAc2V0dGluZy5wYXJhbWV0ZXJUeXBlICU+IFNldHRpbmdzOjo8JT0gQHNldHRpbmcu Z2V0dGVyRnVuY3Rpb25OYW1lICU+KCkgY29uc3QKIHsKIDwlLSBpZiBAc2V0dGluZy5oYXNJbnNw ZWN0b3JPdmVycmlkZT8gLSU+Ci0gICAgaWYgKG1fPCU9IEBzZXR0aW5nLm5hbWUgJT5JbnNwZWN0 b3JPdmVycmlkZSkKKyAgICBpZiAoVU5MSUtFTFkobV88JT0gQHNldHRpbmcubmFtZSAlPkluc3Bl Y3Rvck92ZXJyaWRlKSkgeworICAgICAgICBBU1NFUlQoSW5zcGVjdG9ySW5zdHJ1bWVudGF0aW9u OjpoYXNGcm9udGVuZHMoKSk7CiAgICAgICAgIHJldHVybiBtXzwlPSBAc2V0dGluZy5uYW1lICU+ SW5zcGVjdG9yT3ZlcnJpZGUudmFsdWUoKTsKKyAgICB9CiA8JS0gZW5kIC0lPgogICAgIHJldHVy biBtXzwlPSBAc2V0dGluZy5uYW1lICU+OwogfQpAQCAtOTksOCArMTAyLDEwIEBAIHZvaWQgU2V0 dGluZ3M6OjwlPSBAc2V0dGluZy5zZXR0ZXJGdW5jdGlvbk5hbWUgJT4oPCU9IEBzZXR0aW5nLnBh cmFtZXRlclR5cGUgJT4KIDwlPSBAc2V0dGluZy5wYXJhbWV0ZXJUeXBlICU+IFNldHRpbmdzOjo8 JT0gQHNldHRpbmcuZ2V0dGVyRnVuY3Rpb25OYW1lICU+KCkgY29uc3QKIHsKIDwlLSBpZiBAc2V0 dGluZy5oYXNJbnNwZWN0b3JPdmVycmlkZT8gLSU+Ci0gICAgaWYgKG1fPCU9IEBzZXR0aW5nLm5h bWUgJT5JbnNwZWN0b3JPdmVycmlkZSkKKyAgICBpZiAoVU5MSUtFTFkobV88JT0gQHNldHRpbmcu bmFtZSAlPkluc3BlY3Rvck92ZXJyaWRlKSkgeworICAgICAgICBBU1NFUlQoSW5zcGVjdG9ySW5z dHJ1bWVudGF0aW9uOjpoYXNGcm9udGVuZHMoKSk7CiAgICAgICAgIHJldHVybiBtXzwlPSBAc2V0 dGluZy5uYW1lICU+SW5zcGVjdG9yT3ZlcnJpZGUudmFsdWUoKTsKKyAgICB9CiA8JS0gZW5kIC0l PgogICAgIHJldHVybiBtXzwlPSBAc2V0dGluZy5uYW1lICU+OwogfQo=