Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2866613003:
Solve ProfileIOData, Extension, HostContentSettingsMap ordering issue. (Closed)
|
Created:
3 years, 7 months ago by mmenke Modified:
3 years, 7 months ago CC:
chromium-reviews, extensions-reviews_chromium.org, msramek+watch_chromium.org, raymes+watch_chromium.org, rginda+watch_chromium.org, chromium-apps-reviews_chromium.org, markusheintz_ Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionSolve ProfileIOData, Extension, HostContentSettingsMap ordering issue.
There's a chicken/egg/omelette(?) issue during set up:
* ProfilIOData needs to grab HostContentSettingsMap on creation, and it
can be used at any point after creation to create a URLRequestContext,
which can use the HostContentSettingsMap.
* ExtensionSystem needs to modify the HostContentSettingsMap before the
map is used.
* ExtensionSystem needs access to the URLRequestContextGetter, which
ProfileIOData creates, but needs the HostContentSettingsMap before it
can safely create it.
This CL just makes the ExtensionSystem method to hook into
HostContentSettingsMap static, and the map sets up the hook on
construction. Since the method to set up hooks wasn't depending on any
members of the ExtensionSystem, this should just "work"
This bug has existed a while, but we never ran into DCHECKs as a
result because while the URLRequestContext was potentially being set
up with the HostContentSettingsMap was fully initialized, we generally
weren't issuing network requests until after the ExtensionSystem set
up the map.
https://codereview.chromium.org/2841163002/ changed that, since the
ProxyService may figure out it needs a PAC file and fetch it any time
after the URLRequestContext is created.
BUG=718835
Review-Url: https://codereview.chromium.org/2866613003
Cr-Commit-Position: refs/heads/master@{#471853}
Committed: https://chromium.googlesource.com/chromium/src/+/ad5094aba23fb6260d618c3223bb22d2deb85d6f
Patch Set 1 #Patch Set 2 : Fix? #
Total comments: 6
Patch Set 3 : Response to comments #
Total comments: 2
Patch Set 4 : Fix build #
Total comments: 2
Patch Set 5 : Uninteresting merge, add TODO #
Messages
Total messages: 39 (26 generated)
The CQ bit was checked by mmenke@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by mmenke@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Solve ProfileIOData, Extension, HostContentSettingsMap chicken/egg issue * ProfilIOData needs to grab HostContentSettingsMap on creation, and it can be used at any point after creation to create a URLRequestContext, which can use the HostContentSettingsMap. * ExtensionSystem needs to modify the HostContentSettingsMap before the map is used. * ExtensionSystem needs access to the URLRequestContextGetter, which ProfileIOData creates, but needs the HostContentSettingsMap before it can safely create it. This CL just makes the ExtensionSystem method to hook into HostContentSettingsMap static, and th emap sets up the hook on construction. Since the method to set up hooks wasn't depending on any members of the ExtensionSystem, this should just "work" This bug has existed a while, but we never ran into DCHECKs as a result because while the URLRequestContext was potentially being set up with the HostContentSettingsMap was fully initialized, we generally weren't issuing network requests until after the ExtensionSystem set up the map. https://codereview.chromium.org/2841163002/ changed that, since the ProxyService may figure out it needs a PAC file and fetch it any time after the URLRequestContext is created. BUG=718835 ========== to ========== Solve ProfileIOData, Extension, HostContentSettingsMap ordering issue. There's a chicken/egg/omelette(?) during set up: * ProfilIOData needs to grab HostContentSettingsMap on creation, and it can be used at any point after creation to create a URLRequestContext, which can use the HostContentSettingsMap. * ExtensionSystem needs to modify the HostContentSettingsMap before the map is used. * ExtensionSystem needs access to the URLRequestContextGetter, which ProfileIOData creates, but needs the HostContentSettingsMap before it can safely create it. This CL just makes the ExtensionSystem method to hook into HostContentSettingsMap static, and th emap sets up the hook on construction. Since the method to set up hooks wasn't depending on any members of the ExtensionSystem, this should just "work" This bug has existed a while, but we never ran into DCHECKs as a result because while the URLRequestContext was potentially being set up with the HostContentSettingsMap was fully initialized, we generally weren't issuing network requests until after the ExtensionSystem set up the map. https://codereview.chromium.org/2841163002/ changed that, since the ProxyService may figure out it needs a PAC file and fetch it any time after the URLRequestContext is created. BUG=718835 ==========
Description was changed from ========== Solve ProfileIOData, Extension, HostContentSettingsMap ordering issue. There's a chicken/egg/omelette(?) during set up: * ProfilIOData needs to grab HostContentSettingsMap on creation, and it can be used at any point after creation to create a URLRequestContext, which can use the HostContentSettingsMap. * ExtensionSystem needs to modify the HostContentSettingsMap before the map is used. * ExtensionSystem needs access to the URLRequestContextGetter, which ProfileIOData creates, but needs the HostContentSettingsMap before it can safely create it. This CL just makes the ExtensionSystem method to hook into HostContentSettingsMap static, and th emap sets up the hook on construction. Since the method to set up hooks wasn't depending on any members of the ExtensionSystem, this should just "work" This bug has existed a while, but we never ran into DCHECKs as a result because while the URLRequestContext was potentially being set up with the HostContentSettingsMap was fully initialized, we generally weren't issuing network requests until after the ExtensionSystem set up the map. https://codereview.chromium.org/2841163002/ changed that, since the ProxyService may figure out it needs a PAC file and fetch it any time after the URLRequestContext is created. BUG=718835 ========== to ========== Solve ProfileIOData, Extension, HostContentSettingsMap ordering issue. There's a chicken/egg/omelette(?) issue during set up: * ProfilIOData needs to grab HostContentSettingsMap on creation, and it can be used at any point after creation to create a URLRequestContext, which can use the HostContentSettingsMap. * ExtensionSystem needs to modify the HostContentSettingsMap before the map is used. * ExtensionSystem needs access to the URLRequestContextGetter, which ProfileIOData creates, but needs the HostContentSettingsMap before it can safely create it. This CL just makes the ExtensionSystem method to hook into HostContentSettingsMap static, and th emap sets up the hook on construction. Since the method to set up hooks wasn't depending on any members of the ExtensionSystem, this should just "work" This bug has existed a while, but we never ran into DCHECKs as a result because while the URLRequestContext was potentially being set up with the HostContentSettingsMap was fully initialized, we generally weren't issuing network requests until after the ExtensionSystem set up the map. https://codereview.chromium.org/2841163002/ changed that, since the ProxyService may figure out it needs a PAC file and fetch it any time after the URLRequestContext is created. BUG=718835 ==========
mmenke@chromium.org changed reviewers: + bauerb@chromium.org, rdevlin.cronin@chromium.org
Is this sane? It looks to me like content_settings::InternalExtensionProvider and content_settings::CustomExtensionProvider are safe to create before the ExtensionsSystem is set up. I'm not sure about the interaction with the "switches::kEnableLoginScreenApps" check in ProfileManager and the "extensions::ContentSettingsService::Get(profile)" check I added, though - does the former imply the latter is true? If initializing the extension system doesn't have a side effect of initializing the ContentSettingsService (Which is normally initialized much earlier in the process), then I think this is fine.
Description was changed from ========== Solve ProfileIOData, Extension, HostContentSettingsMap ordering issue. There's a chicken/egg/omelette(?) issue during set up: * ProfilIOData needs to grab HostContentSettingsMap on creation, and it can be used at any point after creation to create a URLRequestContext, which can use the HostContentSettingsMap. * ExtensionSystem needs to modify the HostContentSettingsMap before the map is used. * ExtensionSystem needs access to the URLRequestContextGetter, which ProfileIOData creates, but needs the HostContentSettingsMap before it can safely create it. This CL just makes the ExtensionSystem method to hook into HostContentSettingsMap static, and th emap sets up the hook on construction. Since the method to set up hooks wasn't depending on any members of the ExtensionSystem, this should just "work" This bug has existed a while, but we never ran into DCHECKs as a result because while the URLRequestContext was potentially being set up with the HostContentSettingsMap was fully initialized, we generally weren't issuing network requests until after the ExtensionSystem set up the map. https://codereview.chromium.org/2841163002/ changed that, since the ProxyService may figure out it needs a PAC file and fetch it any time after the URLRequestContext is created. BUG=718835 ========== to ========== Solve ProfileIOData, Extension, HostContentSettingsMap ordering issue. There's a chicken/egg/omelette(?) issue during set up: * ProfilIOData needs to grab HostContentSettingsMap on creation, and it can be used at any point after creation to create a URLRequestContext, which can use the HostContentSettingsMap. * ExtensionSystem needs to modify the HostContentSettingsMap before the map is used. * ExtensionSystem needs access to the URLRequestContextGetter, which ProfileIOData creates, but needs the HostContentSettingsMap before it can safely create it. This CL just makes the ExtensionSystem method to hook into HostContentSettingsMap static, and the map sets up the hook on construction. Since the method to set up hooks wasn't depending on any members of the ExtensionSystem, this should just "work" This bug has existed a while, but we never ran into DCHECKs as a result because while the URLRequestContext was potentially being set up with the HostContentSettingsMap was fully initialized, we generally weren't issuing network requests until after the ExtensionSystem set up the map. https://codereview.chromium.org/2841163002/ changed that, since the ProxyService may figure out it needs a PAC file and fetch it any time after the URLRequestContext is created. BUG=718835 ==========
https://codereview.chromium.org/2866613003/diff/20001/chrome/browser/extensio... File chrome/browser/extensions/extension_service.cc (right): https://codereview.chromium.org/2866613003/diff/20001/chrome/browser/extensio... chrome/browser/extensions/extension_service.cc:2053: if (!extensions::ContentSettingsService::Get(profile)) The ContentSettingsService is just a BrowserContextKeyedService that's created with the profile. Additionally, it's set to be created if it doesn't exist with the first Get() call. So this probably isn't the right check. What exactly are you trying to determine here? If it's just whether extensions are enabled... hmm... we store that data on the ExtensionService, but the whole point is that that may not exist yet... https://codereview.chromium.org/2866613003/diff/20001/chrome/browser/extensio... chrome/browser/extensions/extension_service.cc:2060: new content_settings::InternalExtensionProvider(profile))); Note: this relies on the ExtensionRegistry, which should be okay (since that can exist before the extension system).
https://codereview.chromium.org/2866613003/diff/20001/chrome/browser/extensio... File chrome/browser/extensions/extension_service.cc (right): https://codereview.chromium.org/2866613003/diff/20001/chrome/browser/extensio... chrome/browser/extensions/extension_service.cc:2053: if (!extensions::ContentSettingsService::Get(profile)) On 2017/05/05 22:27:56, Devlin wrote: > The ContentSettingsService is just a BrowserContextKeyedService that's created > with the profile. Additionally, it's set to be created if it doesn't exist with > the first Get() call. So this probably isn't the right check. What exactly are > you trying to determine here? If it's just whether extensions are enabled... > hmm... we store that data on the ExtensionService, but the whole point is that > that may not exist yet... It's null for incognito, causing this method to crash. Except apparently for signing profiles when switches::kEnableLoginScreenApps is true. All the test failures for PS1 were due to this being nullptr.
https://codereview.chromium.org/2866613003/diff/20001/chrome/browser/extensio... File chrome/browser/extensions/extension_service.cc (right): https://codereview.chromium.org/2866613003/diff/20001/chrome/browser/extensio... chrome/browser/extensions/extension_service.cc:2053: if (!extensions::ContentSettingsService::Get(profile)) On 2017/05/05 22:54:34, mmenke wrote: > On 2017/05/05 22:27:56, Devlin wrote: > > The ContentSettingsService is just a BrowserContextKeyedService that's created > > with the profile. Additionally, it's set to be created if it doesn't exist > with > > the first Get() call. So this probably isn't the right check. What exactly > are > > you trying to determine here? If it's just whether extensions are enabled... > > hmm... we store that data on the ExtensionService, but the whole point is that > > that may not exist yet... > > It's null for incognito, causing this method to crash. Except apparently for > signing profiles when switches::kEnableLoginScreenApps is true. All the test > failures for PS1 were due to this being nullptr. Ah, okay. ExtensionService only has a single shared instance across incognito and regular profiles, so the implementation here before *always* used the original profile (the one in |profile_| for extension service. To maintain that same behavior, we would want to use profile->GetOriginalProfile() in all cases here. That would fix the crashing.
On 2017/05/05 23:00:08, Devlin wrote: > https://codereview.chromium.org/2866613003/diff/20001/chrome/browser/extensio... > File chrome/browser/extensions/extension_service.cc (right): > > https://codereview.chromium.org/2866613003/diff/20001/chrome/browser/extensio... > chrome/browser/extensions/extension_service.cc:2053: if > (!extensions::ContentSettingsService::Get(profile)) > On 2017/05/05 22:54:34, mmenke wrote: > > On 2017/05/05 22:27:56, Devlin wrote: > > > The ContentSettingsService is just a BrowserContextKeyedService that's > created > > > with the profile. Additionally, it's set to be created if it doesn't exist > > with > > > the first Get() call. So this probably isn't the right check. What exactly > > are > > > you trying to determine here? If it's just whether extensions are > enabled... > > > hmm... we store that data on the ExtensionService, but the whole point is > that > > > that may not exist yet... > > > > It's null for incognito, causing this method to crash. Except apparently for > > signing profiles when switches::kEnableLoginScreenApps is true. All the test > > failures for PS1 were due to this being nullptr. > > Ah, okay. ExtensionService only has a single shared instance across incognito > and regular profiles, so the implementation here before *always* used the > original profile (the one in |profile_| for extension service. To maintain that > same behavior, we would want to use profile->GetOriginalProfile() in all cases > here. That would fix the crashing. The old code always checked if ext_service was nullptr. I assumed that (In ProfileManager) this only happened when extensions_enabled was false. If it's not null for incognito mode, when is it null?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
LGTM, thanks!
The CQ bit was checked by mmenke@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Thanks, Devlin! PTAL. https://codereview.chromium.org/2866613003/diff/20001/chrome/browser/extensio... File chrome/browser/extensions/extension_service.cc (right): https://codereview.chromium.org/2866613003/diff/20001/chrome/browser/extensio... chrome/browser/extensions/extension_service.cc:2053: if (!extensions::ContentSettingsService::Get(profile)) On 2017/05/05 23:00:08, Devlin (OOO until May 12) wrote: > On 2017/05/05 22:54:34, mmenke wrote: > > On 2017/05/05 22:27:56, Devlin wrote: > > > The ContentSettingsService is just a BrowserContextKeyedService that's > created > > > with the profile. Additionally, it's set to be created if it doesn't exist > > with > > > the first Get() call. So this probably isn't the right check. What exactly > > are > > > you trying to determine here? If it's just whether extensions are > enabled... > > > hmm... we store that data on the ExtensionService, but the whole point is > that > > > that may not exist yet... > > > > It's null for incognito, causing this method to crash. Except apparently for > > signing profiles when switches::kEnableLoginScreenApps is true. All the test > > failures for PS1 were due to this being nullptr. > > Ah, okay. ExtensionService only has a single shared instance across incognito > and regular profiles, so the implementation here before *always* used the > original profile (the one in |profile_| for extension service. To maintain that > same behavior, we would want to use profile->GetOriginalProfile() in all cases > here. That would fix the crashing. I've confirmed that in the original code, we called this for incognito profiles. And that while we seem to have a separate ExtensionService for OTR profiles, the profile_ variable of those extensions services is the original profile, so I've made the suggested change. PTAL. https://codereview.chromium.org/2866613003/diff/40001/chrome/browser/extensio... File chrome/browser/extensions/extension_service.cc (right): https://codereview.chromium.org/2866613003/diff/40001/chrome/browser/extensio... chrome/browser/extensions/extension_service.cc:2068: profile->GetOriginalProfile() != profile))); Is the comparison in line 2068 always true?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by mmenke@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM https://codereview.chromium.org/2866613003/diff/20001/chrome/browser/extensio... File chrome/browser/extensions/extension_service.cc (right): https://codereview.chromium.org/2866613003/diff/20001/chrome/browser/extensio... chrome/browser/extensions/extension_service.cc:2053: if (!extensions::ContentSettingsService::Get(profile)) On 2017/05/08 17:50:35, mmenke wrote: > On 2017/05/05 23:00:08, Devlin (OOO until May 12) wrote: > > On 2017/05/05 22:54:34, mmenke wrote: > > > On 2017/05/05 22:27:56, Devlin wrote: > > > > The ContentSettingsService is just a BrowserContextKeyedService that's > > created > > > > with the profile. Additionally, it's set to be created if it doesn't > exist > > > with > > > > the first Get() call. So this probably isn't the right check. What > exactly > > > are > > > > you trying to determine here? If it's just whether extensions are > > enabled... > > > > hmm... we store that data on the ExtensionService, but the whole point is > > that > > > > that may not exist yet... > > > > > > It's null for incognito, causing this method to crash. Except apparently > for > > > signing profiles when switches::kEnableLoginScreenApps is true. All the > test > > > failures for PS1 were due to this being nullptr. > > > > Ah, okay. ExtensionService only has a single shared instance across incognito > > and regular profiles, so the implementation here before *always* used the > > original profile (the one in |profile_| for extension service. To maintain > that > > same behavior, we would want to use profile->GetOriginalProfile() in all cases > > here. That would fix the crashing. > > I've confirmed that in the original code, we called this for incognito profiles. > And that while we seem to have a separate ExtensionService for OTR profiles, > the profile_ variable of those extensions services is the original profile, so > I've made the suggested change. PTAL. Note: We don't have a separate ExtensionService for OTR profiles. The exact ownership is that we have a ExtensionSystem::Shared object which (as the name implies) is shared between the OTR and regular profiles; the ExtensionService is owned by this object. In other words: Profile* regular_profile = <some profile>; Profile* otr_profile = regular_profile->GetOffTheRecordProfile(); // This won't crash. CHECK_EQ(ExtensionSystem::Get(regular_profile)->extension_service(), ExtensionSystem::Get(otr_profile)->extension_service()); https://codereview.chromium.org/2866613003/diff/40001/chrome/browser/extensio... File chrome/browser/extensions/extension_service.cc (right): https://codereview.chromium.org/2866613003/diff/40001/chrome/browser/extensio... chrome/browser/extensions/extension_service.cc:2068: profile->GetOriginalProfile() != profile))); On 2017/05/08 17:50:35, mmenke wrote: > Is the comparison in line 2068 always true? Yes, it will be. See also other comment. https://codereview.chromium.org/2866613003/diff/60001/chrome/browser/extensio... File chrome/browser/extensions/extension_service.cc (right): https://codereview.chromium.org/2866613003/diff/60001/chrome/browser/extensio... chrome/browser/extensions/extension_service.cc:2068: false))); This is the only place the bool was specified, even though it's guaranteed to have always been false (in the original code as well). But it looks like the incognito bool is actually used in the code for the CustomExtensionProvider... can we add a TODO to address that? Maybe bauerb@ knows more about it?
Thanks for helping me understand this code! https://codereview.chromium.org/2866613003/diff/60001/chrome/browser/extensio... File chrome/browser/extensions/extension_service.cc (right): https://codereview.chromium.org/2866613003/diff/60001/chrome/browser/extensio... chrome/browser/extensions/extension_service.cc:2068: false))); On 2017/05/15 16:53:21, Devlin (catching up) wrote: > This is the only place the bool was specified, even though it's guaranteed to > have always been false (in the original code as well). But it looks like the > incognito bool is actually used in the code for the CustomExtensionProvider... > can we add a TODO to address that? Maybe bauerb@ knows more about it? TODO added. I'd offer to address it myself (Since I assume this fix is just to use profile != original_profile, in the new code), except that it probably needs a browser test, which is more effort than I'm willing to invest.
The CQ bit was checked by mmenke@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org, rdevlin.cronin@chromium.org Link to the patchset: https://codereview.chromium.org/2866613003/#ps80001 (title: "Uninteresting merge, add TODO")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by mmenke@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1494869801627330,
"parent_rev": "1fd895c9370191b2026fd558cd65b821dc1bc070", "commit_rev":
"ad5094aba23fb6260d618c3223bb22d2deb85d6f"}
Message was sent while issue was closed.
Description was changed from ========== Solve ProfileIOData, Extension, HostContentSettingsMap ordering issue. There's a chicken/egg/omelette(?) issue during set up: * ProfilIOData needs to grab HostContentSettingsMap on creation, and it can be used at any point after creation to create a URLRequestContext, which can use the HostContentSettingsMap. * ExtensionSystem needs to modify the HostContentSettingsMap before the map is used. * ExtensionSystem needs access to the URLRequestContextGetter, which ProfileIOData creates, but needs the HostContentSettingsMap before it can safely create it. This CL just makes the ExtensionSystem method to hook into HostContentSettingsMap static, and the map sets up the hook on construction. Since the method to set up hooks wasn't depending on any members of the ExtensionSystem, this should just "work" This bug has existed a while, but we never ran into DCHECKs as a result because while the URLRequestContext was potentially being set up with the HostContentSettingsMap was fully initialized, we generally weren't issuing network requests until after the ExtensionSystem set up the map. https://codereview.chromium.org/2841163002/ changed that, since the ProxyService may figure out it needs a PAC file and fetch it any time after the URLRequestContext is created. BUG=718835 ========== to ========== Solve ProfileIOData, Extension, HostContentSettingsMap ordering issue. There's a chicken/egg/omelette(?) issue during set up: * ProfilIOData needs to grab HostContentSettingsMap on creation, and it can be used at any point after creation to create a URLRequestContext, which can use the HostContentSettingsMap. * ExtensionSystem needs to modify the HostContentSettingsMap before the map is used. * ExtensionSystem needs access to the URLRequestContextGetter, which ProfileIOData creates, but needs the HostContentSettingsMap before it can safely create it. This CL just makes the ExtensionSystem method to hook into HostContentSettingsMap static, and the map sets up the hook on construction. Since the method to set up hooks wasn't depending on any members of the ExtensionSystem, this should just "work" This bug has existed a while, but we never ran into DCHECKs as a result because while the URLRequestContext was potentially being set up with the HostContentSettingsMap was fully initialized, we generally weren't issuing network requests until after the ExtensionSystem set up the map. https://codereview.chromium.org/2841163002/ changed that, since the ProxyService may figure out it needs a PAC file and fetch it any time after the URLRequestContext is created. BUG=718835 Review-Url: https://codereview.chromium.org/2866613003 Cr-Commit-Position: refs/heads/master@{#471853} Committed: https://chromium.googlesource.com/chromium/src/+/ad5094aba23fb6260d618c3223bb... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/ad5094aba23fb6260d618c3223bb... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
