New issue
Advanced search Search tips

Issue 848435 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Split HttpAuthPreferences

Project Member Reported by mmenke@chromium.org, May 31 2018

Issue description

HttpAuthPreferences has two types of data:

Data that's statically set on creation, and is only access by HttpAuthHandlerRegistryFactory on creation, with big scary warnings not to access the prefs after construction, because they my be null (The warnings are because when HttpAuthHandlerRegistryFactory::CreateDefault is used, the prefs_ value will be null after construction - they're justed used to create the factory)

And data that is pulled dynamically from the prefs (when non-null) and is pulled from the prefs by the HttpAuthHandlerRegistryFactory as needed, so is dynamically updated.

The former type of data isn't really needed in the prefs object, since it's not dynamically updated, and the static pref values aren't even shared between different factories.  So I propose making the supported schemes and gssapi library arguments to HttpAuthHandlerRegistryFactory's factory, instead of pref members.

Working on hooking these up through mojo, and it really highlights how unrelated these two usage patterns are.
 

Comment 1 by mmenke@chromium.org, May 31 2018

There's actually a TODO about allowing supported schemes to change, but when/if that's hooked up, it can move back to prefs.
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 7 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/03976eedc5e0a6ec882cd2b64dc8515f53d2eab0

commit 03976eedc5e0a6ec882cd2b64dc8515f53d2eab0
Author: Matt Menke <mmenke@chromium.org>
Date: Thu Jun 07 12:37:30 2018

Separate out HttpAuthPreferences that can't change.

Turn them into constructor arguments for HttpAuthFactory instead.
This makes for a cleaner API, with a bit less awkwardness. This also
makes it possible to use HttpAuthFactory::CreateDefault with an
HttpAuthPreferences object, something that was once impossible.

Bug:  848435 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.linux:linux_mojo;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I09ed42374299fe6f2dce7d3e8d1b380ac759e35b
Reviewed-on: https://chromium-review.googlesource.com/1081368
Reviewed-by: Asanka Herath <asanka@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565241}
[modify] https://crrev.com/03976eedc5e0a6ec882cd2b64dc8515f53d2eab0/android_webview/browser/net/aw_url_request_context_getter.cc
[modify] https://crrev.com/03976eedc5e0a6ec882cd2b64dc8515f53d2eab0/chrome/browser/io_thread.cc
[modify] https://crrev.com/03976eedc5e0a6ec882cd2b64dc8515f53d2eab0/google_apis/gcm/tools/mcs_probe.cc
[modify] https://crrev.com/03976eedc5e0a6ec882cd2b64dc8515f53d2eab0/ios/components/io_thread/ios_io_thread.mm
[modify] https://crrev.com/03976eedc5e0a6ec882cd2b64dc8515f53d2eab0/net/http/http_auth_gssapi_posix.cc
[modify] https://crrev.com/03976eedc5e0a6ec882cd2b64dc8515f53d2eab0/net/http/http_auth_gssapi_posix.h
[modify] https://crrev.com/03976eedc5e0a6ec882cd2b64dc8515f53d2eab0/net/http/http_auth_handler_factory.cc
[modify] https://crrev.com/03976eedc5e0a6ec882cd2b64dc8515f53d2eab0/net/http/http_auth_handler_factory.h
[modify] https://crrev.com/03976eedc5e0a6ec882cd2b64dc8515f53d2eab0/net/http/http_auth_handler_negotiate.cc
[modify] https://crrev.com/03976eedc5e0a6ec882cd2b64dc8515f53d2eab0/net/http/http_auth_handler_negotiate.h
[modify] https://crrev.com/03976eedc5e0a6ec882cd2b64dc8515f53d2eab0/net/http/http_auth_preferences.cc
[modify] https://crrev.com/03976eedc5e0a6ec882cd2b64dc8515f53d2eab0/net/http/http_auth_preferences.h
[modify] https://crrev.com/03976eedc5e0a6ec882cd2b64dc8515f53d2eab0/net/http/http_auth_preferences_unittest.cc
[modify] https://crrev.com/03976eedc5e0a6ec882cd2b64dc8515f53d2eab0/net/http/mock_gssapi_library_posix.cc
[modify] https://crrev.com/03976eedc5e0a6ec882cd2b64dc8515f53d2eab0/net/http/mock_gssapi_library_posix.h
[modify] https://crrev.com/03976eedc5e0a6ec882cd2b64dc8515f53d2eab0/services/network/network_context.cc
[modify] https://crrev.com/03976eedc5e0a6ec882cd2b64dc8515f53d2eab0/services/network/network_context_unittest.cc
[modify] https://crrev.com/03976eedc5e0a6ec882cd2b64dc8515f53d2eab0/services/network/network_service.cc

Status: Fixed (was: Started)

Sign in to add a comment