Clear the HTTP auth state when clearing cookies or cache.

chrome/browser/profiles/profile_impl_io_data.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/profiles/profile_impl_io_data.h"
 
 #include <set>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 760 matching lines @@
 ProfileImplIOData::AcquireIsolatedMediaRequestContext(
     net::URLRequestContext* app_context,
     const StoragePartitionDescriptor& partition_descriptor) const {
   // We create per-app media contexts on demand, unlike the others above.
   net::URLRequestContext* media_request_context =
       InitializeMediaRequestContext(app_context, partition_descriptor);
   DCHECK(media_request_context);
   return media_request_context;
 }
 
 void ProfileImplIOData::ClearNetworkingHistorySinceOnIOThread(

[mmenke, 2015/12/01 15:59:48]

Hrm...Is this really part of the network history?  I notice we aren't clearing
the HTTP cache here, for instance, which has a clearer relationship to the
network history.

I certainly agree that we should probably auth data when we clear cookies.

[msramek, 2015/12/01 16:26:04]

The HTTP cache is deleted straight from BrowsingDataRemover (through
StoragePartitionHttpCacheDataRemover) when the user chooses to delete cache. I
assume that there two reasons why it isn't included here:
1. It's a several-step process jumping between the UI and IO thread, complicated
enough to deserve its own class.
2. This method is called even when user chooses cookies for deletion, and we
didn't want to delete cache in that case.

If you think this doesn't semantically fit here, we can perhaps extract this
part and call it directly from BrowsingDataRemover. It's just that this method
is called from (and only from) BrowsingDataRemover, so it seemed like the right
place. Well, or we can rename the method.

[asanka, 2015/12/01 17:51:10]

Putting this in ClearNetworkHistorySinceOnIOThread() means that HttpAuthCache
will be cleared if any option is selected in the "Clear browsing data" dialog
(even when the user is just trying to clear downloads history). Although that's
probably symptomatic of another problem.

Each time this has come up, the user expectation is that the HTTP auth cache
would be cleared when the "Passwords" option is selected on the "Clear browsing
data" dialog. This makes sense since the HttpAuthCache currently stores
passwords.

As far as the network stack goes, it makes sense also clear the auth cache if
the user chooses 'Cookies and other site and plugin data'. This option maps to
REMOVE_SITE_DATA | REMOVE_CACHE which includes REMOVE_COOKIES but not
REMOVE_PASSWORDS.

Overall, I think either REMOVE_COOKIES and REMOVE_PASSWORDS should imply
clearing of HttpAuthCache for the prescribed time window. And clearing the
HttpAuthCache should imply flushing the socket pools since connection based auth
schemes might used an authenticated connection (as mmenke pointed out). WDYT?

[mmenke, 2015/12/01 17:59:32]

That sounds reasonable to me, at least.

     base::Time time,
     const base::Closure& completion) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(initialized());
 
+  // Synchronous deletion.
   DCHECK(transport_security_state());
-  // Completes synchronously.
   transport_security_state()->DeleteAllDynamicDataSince(time);
+  DCHECK(http_network_session_.get());
+  http_network_session_->http_auth_cache()->Clear();

[mmenke, 2015/12/01 15:59:48]

I don't think it makes much sense to clear auth information without also
flushing the socket pools.  Otherwise, we may still have reusable connections
around that actually use that information.

[msramek, 2015/12/01 16:26:04]

Ok, I'll look into it! (I'm not very familiar with our network stack)

[asanka, 2015/12/01 17:51:10]

Also, this is meant to delete state since |time|. HttpAuthCache already keeps
creation timestamps around for cache entries which we should use to only remove
entries that were created after |time|.

+
+  // Asynchronous deletion, calls the |completion| callback when finished.
   DCHECK(http_server_properties_manager_);
   http_server_properties_manager_->Clear(completion);
 }