Bug 124367 – [ASAN] WebKit2: Remove calls to dead DownloadAuthenticationClient code

• Component: WebKit2
• Status: RESOLVED FIXED

[David Farler]

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.

[David Farler]

Created attachment 216954 [details]
Patch

[David Kilzer (:ddkilzer)]

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.

[David Farler]

(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.

[David Kilzer (:ddkilzer)]

(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.

[Alexey Proskuryakov]

Comment on attachment 216954 [details]
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.

[David Farler]

Committed r159441: <http://trac.webkit.org/changeset/159441>