124367 2013-11-14 09:29:50 -0800 [ASAN] WebKit2: Remove calls to dead DownloadAuthenticationClient code 2013-11-18 11:42:10 -0800 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Mac All RESOLVED FIXED P2 Normal --- 1 dfarler dfarler andersca beidson ddkilzer sam oldest_to_newest 950718 0 dfarler 2013-11-14 09:29:50 -0800 To unblock ASAN builds, remove the following dead calls: Undefined symbols for architecture x86_64: "WebKit::Download::cancelAuthenticationChallenge(WebCore::AuthenticationChallenge const&)", referenced from: WebKit::DownloadAuthenticationClient::receivedCancellation(WebCore::AuthenticationChallenge const&) in DownloadAuthenticationClient.o "WebKit::Download::continueWithoutCredential(WebCore::AuthenticationChallenge const&)", referenced from: WebKit::DownloadAuthenticationClient::receivedRequestToContinueWithoutCredential(WebCore::AuthenticationChallenge const&) in DownloadAuthenticationClient.o "WebKit::Download::useCredential(WebCore::AuthenticationChallenge const&, WebCore::Credential const&)", referenced from: WebKit::DownloadAuthenticationClient::receivedCredential(WebCore::AuthenticationChallenge const&, WebCore::Credential const&) in DownloadAuthenticationClient.o I believe we’ve seen this before but the solution was to just remove the dead code altogether. To speed things along, I’d like to just remove the calls to nowhere. 950732 1 216954 dfarler 2013-11-14 09:50:18 -0800 Created attachment 216954 Patch 951012 2 ddkilzer 2013-11-15 02:53:45 -0800 Why aren't these methods implemented in Source/WebKit2/Shared/Downloads/mac/DownloadMac.mm? Odd, all three are only implemented in Source/WebKit2/Shared/Downloads/soup/DownloadSoup.cpp currently…as notImplemented(). LOL! Seems like we need to pull this thread a little more to see what other code can simply be removed. 951066 3 dfarler 2013-11-15 07:41:40 -0800 (In reply to comment #2) > Why aren't these methods implemented in Source/WebKit2/Shared/Downloads/mac/DownloadMac.mm? > > Odd, all three are only implemented in Source/WebKit2/Shared/Downloads/soup/DownloadSoup.cpp currently…as notImplemented(). LOL! > > Seems like we need to pull this thread a little more to see what other code can simply be removed. I had filed this to originally unblock the builds: https://bugs.webkit.org/show_bug.cgi?id=119667 But since the stale code is more widespread, we can use that to explore removing more. I just wanted to clean up the patched codebase on the ASAN builder to keep builds going. 951239 4 ddkilzer 2013-11-15 15:35:36 -0800 (In reply to comment #3) > (In reply to comment #2) > > Why aren't these methods implemented in Source/WebKit2/Shared/Downloads/mac/DownloadMac.mm? > > > > Odd, all three are only implemented in Source/WebKit2/Shared/Downloads/soup/DownloadSoup.cpp currently…as notImplemented(). LOL! > > > > Seems like we need to pull this thread a little more to see what other code can simply be removed. > > I had filed this to originally unblock the builds: > https://bugs.webkit.org/show_bug.cgi?id=119667 > > But since the stale code is more widespread, we can use that to explore removing more. I just wanted to clean up the patched codebase on the ASAN builder to keep builds going. Okay, makes sense to me. Need a WebKit2 reviewer to stamp this, though, even though it's not an architectural change. 951284 5 216954 ap 2013-11-15 17:01:22 -0800 Comment on attachment 216954 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=216954&action=review Anders may know more about the history of this code. It would be very much worth it to delete as much as possible. > Source/WebKit2/Shared/Downloads/DownloadAuthenticationClient.cpp:-45 > - if (!m_download) > - return; > - m_download->useCredential(challenge, credential); Please add a fixme referencing https://bugs.webkit.org/show_bug.cgi?id=119667 // FIXME (119667): This can probably be just removed. > Source/WebKit2/Shared/Downloads/DownloadAuthenticationClient.cpp:-52 > - if (!m_download) > - return; > - m_download->continueWithoutCredential(challenge); Ditto. > Source/WebKit2/Shared/Downloads/DownloadAuthenticationClient.cpp:-59 > - if (!m_download) > - return; > - m_download->cancelAuthenticationChallenge(challenge); Ditto. 951721 6 dfarler 2013-11-18 11:42:10 -0800 Committed r159441: <http://trac.webkit.org/changeset/159441> 216954 2013-11-14 09:50:18 -0800 2013-11-15 17:01:22 -0800 Patch bug-124367-20131114095017.patch text/plain 2421 dfarler U3VidmVyc2lvbiBSZXZpc2lvbjogMTU5Mjg4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggYjAzNGJhYWE3NDQxODM0 ODAzM2Q2MWU0ZWM4ZTM4ZTU2NGQ0NzA0OC4uODgzZGJlODZlOGMyNjA2MmFjZTAxMTFlNDI2NzFj ODE4ZDg2ZmIxMyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDEzLTExLTE0ICBEYXZp ZCBGYXJsZXIgIDxkZmFybGVyQGFwcGxlLmNvbT4KKworICAgICAgICBbQVNBTl0gV2ViS2l0Mjog UmVtb3ZlIGNhbGxzIHRvIGRlYWQgRG93bmxvYWRBdXRoZW50aWNhdGlvbkNsaWVudCBjb2RlCisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMjQzNjcKKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFNoYXJlZC9E b3dubG9hZHMvRG93bmxvYWRBdXRoZW50aWNhdGlvbkNsaWVudC5jcHA6CisgICAgICAgIChXZWJL aXQ6OkRvd25sb2FkQXV0aGVudGljYXRpb25DbGllbnQ6OnJlY2VpdmVkQ3JlZGVudGlhbCk6Cisg ICAgICAgIChXZWJLaXQ6OkRvd25sb2FkQXV0aGVudGljYXRpb25DbGllbnQ6OnJlY2VpdmVkUmVx dWVzdFRvQ29udGludWVXaXRob3V0Q3JlZGVudGlhbCk6CisgICAgICAgIChXZWJLaXQ6OkRvd25s b2FkQXV0aGVudGljYXRpb25DbGllbnQ6OnJlY2VpdmVkQ2FuY2VsbGF0aW9uKToKKyAgICAgICAg UmVtb3ZlIGNhbGxzIHRvIG5vd2hlcmU6CisgICAgICAgIFdlYktpdDo6RG93bmxvYWQ6OmNhbmNl bEF1dGhlbnRpY2F0aW9uQ2hhbGxlbmdlKFdlYkNvcmU6OkF1dGhlbnRpY2F0aW9uQ2hhbGxlbmdl IGNvbnN0JikKKyAgICAgICAgV2ViS2l0OjpEb3dubG9hZDo6Y29udGludWVXaXRob3V0Q3JlZGVu dGlhbChXZWJDb3JlOjpBdXRoZW50aWNhdGlvbkNoYWxsZW5nZSBjb25zdCYpCisgICAgICAgIFdl YktpdDo6RG93bmxvYWQ6OnVzZUNyZWRlbnRpYWwoV2ViQ29yZTo6QXV0aGVudGljYXRpb25DaGFs bGVuZ2UgY29uc3QmLCBXZWJDb3JlOjpDcmVkZW50aWFsIGNvbnN0JikKKwogMjAxMy0xMS0xNCAg Unl1YW4gQ2hvaSAgPHJ5dWFuLmNob2lAc2Ftc3VuZy5jb20+CiAKICAgICAgICAgW1dLMl0gTW92 ZSBDb29yZGluYXRlZCBHcmFwaGljcyByZWxhdGVkIGNvZGUgb3V0IG9mIERyYXdpbmdBcmVhUHJv eHkKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL1NoYXJlZC9Eb3dubG9hZHMvRG93bmxvYWRB dXRoZW50aWNhdGlvbkNsaWVudC5jcHAgYi9Tb3VyY2UvV2ViS2l0Mi9TaGFyZWQvRG93bmxvYWRz L0Rvd25sb2FkQXV0aGVudGljYXRpb25DbGllbnQuY3BwCmluZGV4IDJmMTU4MGZmNzBkOGI1YzMy ZGMxNTllNDMyMDlhNWMxMGJmNDMyNjkuLjg2NjBiYjAwOTJmZWU2N2FmMDEyNjgxNmUyMzkwN2Ez ZWIwYzk2NzggMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1NoYXJlZC9Eb3dubG9hZHMvRG93 bmxvYWRBdXRoZW50aWNhdGlvbkNsaWVudC5jcHAKKysrIGIvU291cmNlL1dlYktpdDIvU2hhcmVk L0Rvd25sb2Fkcy9Eb3dubG9hZEF1dGhlbnRpY2F0aW9uQ2xpZW50LmNwcApAQCAtNDAsMjMgKzQw LDE0IEBAIERvd25sb2FkQXV0aGVudGljYXRpb25DbGllbnQ6OkRvd25sb2FkQXV0aGVudGljYXRp b25DbGllbnQoRG93bmxvYWQqIGRvd25sb2FkKQogCiB2b2lkIERvd25sb2FkQXV0aGVudGljYXRp b25DbGllbnQ6OnJlY2VpdmVkQ3JlZGVudGlhbChjb25zdCBBdXRoZW50aWNhdGlvbkNoYWxsZW5n ZSYgY2hhbGxlbmdlLCBjb25zdCBDcmVkZW50aWFsJiBjcmVkZW50aWFsKQogewotICAgIGlmICgh bV9kb3dubG9hZCkKLSAgICAgICAgcmV0dXJuOwotICAgIG1fZG93bmxvYWQtPnVzZUNyZWRlbnRp YWwoY2hhbGxlbmdlLCBjcmVkZW50aWFsKTsKIH0KIAogdm9pZCBEb3dubG9hZEF1dGhlbnRpY2F0 aW9uQ2xpZW50OjpyZWNlaXZlZFJlcXVlc3RUb0NvbnRpbnVlV2l0aG91dENyZWRlbnRpYWwoY29u c3QgQXV0aGVudGljYXRpb25DaGFsbGVuZ2UmIGNoYWxsZW5nZSkKIHsKLSAgICBpZiAoIW1fZG93 bmxvYWQpCi0gICAgICAgIHJldHVybjsKLSAgICBtX2Rvd25sb2FkLT5jb250aW51ZVdpdGhvdXRD cmVkZW50aWFsKGNoYWxsZW5nZSk7CiB9CiAKIHZvaWQgRG93bmxvYWRBdXRoZW50aWNhdGlvbkNs aWVudDo6cmVjZWl2ZWRDYW5jZWxsYXRpb24oY29uc3QgQXV0aGVudGljYXRpb25DaGFsbGVuZ2Um IGNoYWxsZW5nZSkKIHsKLSAgICBpZiAoIW1fZG93bmxvYWQpCi0gICAgICAgIHJldHVybjsKLSAg ICBtX2Rvd25sb2FkLT5jYW5jZWxBdXRoZW50aWNhdGlvbkNoYWxsZW5nZShjaGFsbGVuZ2UpOwog fQogCiB9IC8vIG5hbWVzcGFjZSBXZWJLaXQK