Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
665703002:
Activate data reduction proxy after profile initialization is complete. (Closed)
|
Created:
6 years, 2 months ago by Not at Google. Contact bengr Modified:
6 years, 2 months ago CC:
chromium-reviews, cbentzel+watch_chromium.org, yoshiki+watch_chromium.org, rginda+watch_chromium.org, marq (ping after 24h) Base URL:
https://chromium.googlesource.com/chromium/src.git@master Project:
chromium Visibility:
Public. |
DescriptionActivate data reduction proxy after profile initialization is complete.
When we activate data reduction proxy, a request is made to the prober
URL. If this request is made before
ProfileManager::DoFinalInitForServices() completes, it causes a crash
because HostContentSettingsMap is not yet initialized.
BUG=424260
Committed: https://crrev.com/8d8aeb6b0d6b5bc33b4ff957177ce28904e57a1e
Cr-Commit-Position: refs/heads/master@{#300995}
Patch Set 1 #Patch Set 2 : Fix webview and tests. #Patch Set 3 : Fix tests. #
Total comments: 8
Patch Set 4 : Addressed bengr's comments #
Total comments: 8
Patch Set 5 : Move MaybeActivateDataReductionProxy to DoFinalInitForServices. #Patch Set 6 : Sync to head #
Total comments: 2
Messages
Total messages: 28 (5 generated)
kundaji@chromium.org changed reviewers: + bengr@chromium.org, mmenke@chromium.org, sgurun@chromium.org
sgurun: android_webview/browser/aw_browser_context.cc bengr: chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings_factory.h components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.cc components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h components/data_reduction_proxy/core/browser/data_reduction_proxy_settings_test_utils.cc mmenke: chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings_factory.h chrome/browser/profiles/profile_manager.cc
https://codereview.chromium.org/665703002/diff/40001/chrome/browser/profiles/... File chrome/browser/profiles/profile_manager.cc (right): https://codereview.chromium.org/665703002/diff/40001/chrome/browser/profiles/... chrome/browser/profiles/profile_manager.cc:986: DataReductionProxyChromeSettingsFactory::GetForBrowserContext(profile)-> What happens in incognito? https://codereview.chromium.org/665703002/diff/40001/components/data_reductio... File components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.cc (right): https://codereview.chromium.org/665703002/diff/40001/components/data_reductio... components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.cc:378: void DataReductionProxySettings::MaybeActivateDataReductionProxy( Can't tests just override this? https://codereview.chromium.org/665703002/diff/40001/components/data_reductio... File components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h (right): https://codereview.chromium.org/665703002/diff/40001/components/data_reductio... components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h:184: // Configures data reduction proxy and makes a request to the prober URL to probe https://codereview.chromium.org/665703002/diff/40001/components/data_reductio... components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h:185: // determine server availability. Describe what "at_startup" means.
https://codereview.chromium.org/665703002/diff/40001/chrome/browser/profiles/... File chrome/browser/profiles/profile_manager.cc (right): https://codereview.chromium.org/665703002/diff/40001/chrome/browser/profiles/... chrome/browser/profiles/profile_manager.cc:986: DataReductionProxyChromeSettingsFactory::GetForBrowserContext(profile)-> On 2014/10/22 00:46:43, bengr wrote: > What happens in incognito? This code does not run for incognito. https://codereview.chromium.org/665703002/diff/40001/components/data_reductio... File components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.cc (right): https://codereview.chromium.org/665703002/diff/40001/components/data_reductio... components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.cc:378: void DataReductionProxySettings::MaybeActivateDataReductionProxy( On 2014/10/22 00:46:43, bengr wrote: > Can't tests just override this? Every test which involves a profile will have to override this. Even ones which do not care or know about the data reduction proxy component. We will need to fix more than 200 hundred tests for this approach to work. https://codereview.chromium.org/665703002/diff/40001/components/data_reductio... File components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h (right): https://codereview.chromium.org/665703002/diff/40001/components/data_reductio... components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h:184: // Configures data reduction proxy and makes a request to the prober URL to On 2014/10/22 00:46:43, bengr wrote: > probe Done. https://codereview.chromium.org/665703002/diff/40001/components/data_reductio... components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h:185: // determine server availability. On 2014/10/22 00:46:43, bengr wrote: > Describe what "at_startup" means. Done.
lgtm
https://codereview.chromium.org/665703002/diff/60001/chrome/browser/profiles/... File chrome/browser/profiles/profile_manager.cc (right): https://codereview.chromium.org/665703002/diff/60001/chrome/browser/profiles/... chrome/browser/profiles/profile_manager.cc:987: MaybeActivateDataReductionProxy(true); I am not the owner of this code, but this custom code for DRP here just looks like a hack to me. is this the only way of achieving this goal?
https://codereview.chromium.org/665703002/diff/60001/chrome/browser/profiles/... File chrome/browser/profiles/profile_manager.cc (right): https://codereview.chromium.org/665703002/diff/60001/chrome/browser/profiles/... chrome/browser/profiles/profile_manager.cc:987: MaybeActivateDataReductionProxy(true); On 2014/10/22 20:25:59, sgurun wrote: > I am not the owner of this code, but this custom code for DRP here just looks > like a hack to me. is this the only way of achieving this goal? This code was simply moved from being called through ProfileImpl::DoFinalInit() to here(ProfileManger::DoFinalInit()), so it's about as good or bad as the previous location. Note that ProfileImpl::DoFinalInit() continues to have custom DRP code and the call is similar to this one. I don't know of a better location. We could see if mmenke has suggestions.
https://codereview.chromium.org/665703002/diff/60001/chrome/browser/profiles/... File chrome/browser/profiles/profile_manager.cc (right): https://codereview.chromium.org/665703002/diff/60001/chrome/browser/profiles/... chrome/browser/profiles/profile_manager.cc:987: MaybeActivateDataReductionProxy(true); On 2014/10/22 20:55:29, kundaji wrote: > On 2014/10/22 20:25:59, sgurun wrote: > > I am not the owner of this code, but this custom code for DRP here just looks > > like a hack to me. is this the only way of achieving this goal? > > This code was simply moved from being called through ProfileImpl::DoFinalInit() > to here(ProfileManger::DoFinalInit()), so it's about as good or bad as the > previous location. Note that ProfileImpl::DoFinalInit() continues to have custom > DRP code and the call is similar to this one. > > I don't know of a better location. We could see if mmenke has suggestions. Why can't we just do all this on the IO thread? (Stuff meaning doing the probe, figuring out what settings to use, etc)?
https://codereview.chromium.org/665703002/diff/60001/chrome/browser/profiles/... File chrome/browser/profiles/profile_manager.cc (right): https://codereview.chromium.org/665703002/diff/60001/chrome/browser/profiles/... chrome/browser/profiles/profile_manager.cc:987: MaybeActivateDataReductionProxy(true); On 2014/10/23 17:17:26, mmenke wrote: > On 2014/10/22 20:55:29, kundaji wrote: > > On 2014/10/22 20:25:59, sgurun wrote: > > > I am not the owner of this code, but this custom code for DRP here just > looks > > > like a hack to me. is this the only way of achieving this goal? > > > > This code was simply moved from being called through > ProfileImpl::DoFinalInit() > > to here(ProfileManger::DoFinalInit()), so it's about as good or bad as the > > previous location. Note that ProfileImpl::DoFinalInit() continues to have > custom > > DRP code and the call is similar to this one. > > > > I don't know of a better location. We could see if mmenke has suggestions. > > Why can't we just do all this on the IO thread? (Stuff meaning doing the probe, > figuring out what settings to use, etc)? The probe can only be done after profile initialization is complete. More specifically, only after DoFinalInitForServices(..) on line 982 above. If probe is done before DoFinalInitForServices(..), then it causes the crash mentioned in the bug. SetupDataReductionProxy(..) in io thread is called before the above method executes, so cannot do it there. Is there any location on io thread which works for this purpose?
https://codereview.chromium.org/665703002/diff/60001/chrome/browser/profiles/... File chrome/browser/profiles/profile_manager.cc (right): https://codereview.chromium.org/665703002/diff/60001/chrome/browser/profiles/... chrome/browser/profiles/profile_manager.cc:987: MaybeActivateDataReductionProxy(true); On 2014/10/23 17:29:33, kundaji wrote: > On 2014/10/23 17:17:26, mmenke wrote: > > On 2014/10/22 20:55:29, kundaji wrote: > > > On 2014/10/22 20:25:59, sgurun wrote: > > > > I am not the owner of this code, but this custom code for DRP here just > > looks > > > > like a hack to me. is this the only way of achieving this goal? > > > > > > This code was simply moved from being called through > > ProfileImpl::DoFinalInit() > > > to here(ProfileManger::DoFinalInit()), so it's about as good or bad as the > > > previous location. Note that ProfileImpl::DoFinalInit() continues to have > > custom > > > DRP code and the call is similar to this one. > > > > > > I don't know of a better location. We could see if mmenke has suggestions. > > > > Why can't we just do all this on the IO thread? (Stuff meaning doing the > probe, > > figuring out what settings to use, etc)? > > The probe can only be done after profile initialization is complete. More > specifically, only after DoFinalInitForServices(..) on line 982 above. If probe > is done before DoFinalInitForServices(..), then it causes the crash mentioned in > the bug. > > SetupDataReductionProxy(..) in io thread is called before the above method > executes, so cannot do it there. Is there any location on io thread which works > for this purpose? > I thought the issue was when you could first make a URLRequest? You should be able to them at the end of ProfileIOData::Init.
https://codereview.chromium.org/665703002/diff/60001/chrome/browser/profiles/... File chrome/browser/profiles/profile_manager.cc (right): https://codereview.chromium.org/665703002/diff/60001/chrome/browser/profiles/... chrome/browser/profiles/profile_manager.cc:987: MaybeActivateDataReductionProxy(true); On 2014/10/23 17:36:59, mmenke wrote: > On 2014/10/23 17:29:33, kundaji wrote: > > On 2014/10/23 17:17:26, mmenke wrote: > > > On 2014/10/22 20:55:29, kundaji wrote: > > > > On 2014/10/22 20:25:59, sgurun wrote: > > > > > I am not the owner of this code, but this custom code for DRP here just > > > looks > > > > > like a hack to me. is this the only way of achieving this goal? > > > > > > > > This code was simply moved from being called through > > > ProfileImpl::DoFinalInit() > > > > to here(ProfileManger::DoFinalInit()), so it's about as good or bad as the > > > > previous location. Note that ProfileImpl::DoFinalInit() continues to have > > > custom > > > > DRP code and the call is similar to this one. > > > > > > > > I don't know of a better location. We could see if mmenke has suggestions. > > > > > > Why can't we just do all this on the IO thread? (Stuff meaning doing the > > probe, > > > figuring out what settings to use, etc)? > > > > The probe can only be done after profile initialization is complete. More > > specifically, only after DoFinalInitForServices(..) on line 982 above. If > probe > > is done before DoFinalInitForServices(..), then it causes the crash mentioned > in > > the bug. > > > > SetupDataReductionProxy(..) in io thread is called before the above method > > executes, so cannot do it there. Is there any location on io thread which > works > > for this purpose? > > > > I thought the issue was when you could first make a URLRequest? You should be > able to them at the end of ProfileIOData::Init. Yes, we connect to the probe with a URLRequest.
https://codereview.chromium.org/665703002/diff/60001/chrome/browser/profiles/... File chrome/browser/profiles/profile_manager.cc (right): https://codereview.chromium.org/665703002/diff/60001/chrome/browser/profiles/... chrome/browser/profiles/profile_manager.cc:987: MaybeActivateDataReductionProxy(true); On 2014/10/23 17:36:59, mmenke wrote: > On 2014/10/23 17:29:33, kundaji wrote: > > On 2014/10/23 17:17:26, mmenke wrote: > > > On 2014/10/22 20:55:29, kundaji wrote: > > > > On 2014/10/22 20:25:59, sgurun wrote: > > > > > I am not the owner of this code, but this custom code for DRP here just > > > looks > > > > > like a hack to me. is this the only way of achieving this goal? > > > > > > > > This code was simply moved from being called through > > > ProfileImpl::DoFinalInit() > > > > to here(ProfileManger::DoFinalInit()), so it's about as good or bad as the > > > > previous location. Note that ProfileImpl::DoFinalInit() continues to have > > > custom > > > > DRP code and the call is similar to this one. > > > > > > > > I don't know of a better location. We could see if mmenke has suggestions. > > > > > > Why can't we just do all this on the IO thread? (Stuff meaning doing the > > probe, > > > figuring out what settings to use, etc)? > > > > The probe can only be done after profile initialization is complete. More > > specifically, only after DoFinalInitForServices(..) on line 982 above. If > probe > > is done before DoFinalInitForServices(..), then it causes the crash mentioned > in > > the bug. > > > > SetupDataReductionProxy(..) in io thread is called before the above method > > executes, so cannot do it there. Is there any location on io thread which > works > > for this purpose? > > > > I thought the issue was when you could first make a URLRequest? You should be > able to them at the end of ProfileIOData::Init. Trying to connect to the probe with a URLRequest at the end of ProfileIOData::Init causes the crash mentioned in the bug.
https://codereview.chromium.org/665703002/diff/60001/chrome/browser/profiles/... File chrome/browser/profiles/profile_manager.cc (right): https://codereview.chromium.org/665703002/diff/60001/chrome/browser/profiles/... chrome/browser/profiles/profile_manager.cc:987: MaybeActivateDataReductionProxy(true); On 2014/10/23 17:40:53, kundaji wrote: > On 2014/10/23 17:36:59, mmenke wrote: > > On 2014/10/23 17:29:33, kundaji wrote: > > > On 2014/10/23 17:17:26, mmenke wrote: > > > > On 2014/10/22 20:55:29, kundaji wrote: > > > > > On 2014/10/22 20:25:59, sgurun wrote: > > > > > > I am not the owner of this code, but this custom code for DRP here > just > > > > looks > > > > > > like a hack to me. is this the only way of achieving this goal? > > > > > > > > > > This code was simply moved from being called through > > > > ProfileImpl::DoFinalInit() > > > > > to here(ProfileManger::DoFinalInit()), so it's about as good or bad as > the > > > > > previous location. Note that ProfileImpl::DoFinalInit() continues to > have > > > > custom > > > > > DRP code and the call is similar to this one. > > > > > > > > > > I don't know of a better location. We could see if mmenke has > suggestions. > > > > > > > > Why can't we just do all this on the IO thread? (Stuff meaning doing the > > > probe, > > > > figuring out what settings to use, etc)? > > > > > > The probe can only be done after profile initialization is complete. More > > > specifically, only after DoFinalInitForServices(..) on line 982 above. If > > probe > > > is done before DoFinalInitForServices(..), then it causes the crash > mentioned > > in > > > the bug. > > > > > > SetupDataReductionProxy(..) in io thread is called before the above method > > > executes, so cannot do it there. Is there any location on io thread which > > works > > > for this purpose? > > > > > > > I thought the issue was when you could first make a URLRequest? You should be > > able to them at the end of ProfileIOData::Init. > > Trying to connect to the probe with a URLRequest at the end of > ProfileIOData::Init causes the crash mentioned in the bug. Skimming over the bug, I see no mention of this function, or even ProfileIOData. I do see mention of ProfileImpl::DoFinalInit.
Discussed in person. Moved MaybeActivateDataReductionProxy to DoFinalInitForServices.
kundaji@chromium.org changed reviewers: + rlp@chromium.org
sgurun: android_webview/browser/aw_browser_context.cc bengr: chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings_factory.h components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.cc components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h components/data_reduction_proxy/core/browser/data_reduction_proxy_settings_test_utils.cc mmenke: chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings_factory.h rpetterson: chrome/browser/profiles/profile_manager.cc
https://codereview.chromium.org/665703002/diff/100001/chrome/browser/profiles... File chrome/browser/profiles/profile_manager.cc (right): https://codereview.chromium.org/665703002/diff/100001/chrome/browser/profiles... chrome/browser/profiles/profile_manager.cc:1017: DataReductionProxyChromeSettingsFactory::GetForBrowserContext(profile)-> Is there any risk of this call slowing down startup since we create at least 1 profile pretty much immediately upon startup?
https://codereview.chromium.org/665703002/diff/100001/chrome/browser/profiles... File chrome/browser/profiles/profile_manager.cc (right): https://codereview.chromium.org/665703002/diff/100001/chrome/browser/profiles... chrome/browser/profiles/profile_manager.cc:1017: DataReductionProxyChromeSettingsFactory::GetForBrowserContext(profile)-> On 2014/10/23 19:57:04, rpetterson wrote: > Is there any risk of this call slowing down startup since we create at least 1 > profile pretty much immediately upon startup? This call is simply being moved from profile_impl.cc::DoFinalInit (which calls InitDataReductionProxySettings which used to call MaybeActivateDataReductionProxy) to here. So the code being executed does not change, just its order. Here is everything this call does: 1) Instantiate an instance of DataReductionProxyParams and DataReductionProxyChromeSettings which should be quick. 2) Set a pref and proxy configs. 3) Make a URL request. This is asynchronous. So I don't see any potential slowdown.
On 2014/10/23 20:08:51, kundaji wrote: > https://codereview.chromium.org/665703002/diff/100001/chrome/browser/profiles... > File chrome/browser/profiles/profile_manager.cc (right): > > https://codereview.chromium.org/665703002/diff/100001/chrome/browser/profiles... > chrome/browser/profiles/profile_manager.cc:1017: > DataReductionProxyChromeSettingsFactory::GetForBrowserContext(profile)-> > On 2014/10/23 19:57:04, rpetterson wrote: > > Is there any risk of this call slowing down startup since we create at least 1 > > profile pretty much immediately upon startup? > > This call is simply being moved from profile_impl.cc::DoFinalInit (which calls > InitDataReductionProxySettings which used to call > MaybeActivateDataReductionProxy) to here. So the code being executed does not > change, just its order. > > Here is everything this call does: > 1) Instantiate an instance of DataReductionProxyParams and > DataReductionProxyChromeSettings which should be quick. > 2) Set a pref and proxy configs. > 3) Make a URL request. This is asynchronous. > So I don't see any potential slowdown. That sounds fine. But if that's the case, why isn't profile_impl.cc included in this CL?
On 2014/10/23 21:28:45, rpetterson wrote: > On 2014/10/23 20:08:51, kundaji wrote: > > > https://codereview.chromium.org/665703002/diff/100001/chrome/browser/profiles... > > File chrome/browser/profiles/profile_manager.cc (right): > > > > > https://codereview.chromium.org/665703002/diff/100001/chrome/browser/profiles... > > chrome/browser/profiles/profile_manager.cc:1017: > > DataReductionProxyChromeSettingsFactory::GetForBrowserContext(profile)-> > > On 2014/10/23 19:57:04, rpetterson wrote: > > > Is there any risk of this call slowing down startup since we create at least > 1 > > > profile pretty much immediately upon startup? > > > > This call is simply being moved from profile_impl.cc::DoFinalInit (which calls > > InitDataReductionProxySettings which used to call > > MaybeActivateDataReductionProxy) to here. So the code being executed does not > > change, just its order. > > > > Here is everything this call does: > > 1) Instantiate an instance of DataReductionProxyParams and > > DataReductionProxyChromeSettings which should be quick. > > 2) Set a pref and proxy configs. > > 3) Make a URL request. This is asynchronous. > > So I don't see any potential slowdown. > > That sounds fine. But if that's the case, why isn't profile_impl.cc included in > this CL? The method was moved out from InitDataReductionProxySettings() in data_reduction_proxy_settings.cc, which is included in the cl. Here is the overall before and after picture: Before this cl: ProfileImpl::DoFinalInit() calls InitDataReductionProxySettings() which calls MaybeActivateDataReductionProxy(). After this cl: ProfileImpl::DoFinalInit() calls InitDataReductionProxySettings() and ProfileManager::DoFinalInit() calls MaybeActivateDataReductionProxy(). As for the relation between ProfileImpl::DoFinalInit() and ProfileManager::DoFinalInit(): ProfileImpl::DoFinalInit() calls ProfileManager::OnProfileCreated() which calls ProfileManager::DoFinalInit(). Feel free to ping me on IM if this is not clear.
On 2014/10/23 21:58:47, kundaji wrote: > On 2014/10/23 21:28:45, rpetterson wrote: > > On 2014/10/23 20:08:51, kundaji wrote: > > > > > > https://codereview.chromium.org/665703002/diff/100001/chrome/browser/profiles... > > > File chrome/browser/profiles/profile_manager.cc (right): > > > > > > > > > https://codereview.chromium.org/665703002/diff/100001/chrome/browser/profiles... > > > chrome/browser/profiles/profile_manager.cc:1017: > > > DataReductionProxyChromeSettingsFactory::GetForBrowserContext(profile)-> > > > On 2014/10/23 19:57:04, rpetterson wrote: > > > > Is there any risk of this call slowing down startup since we create at > least > > 1 > > > > profile pretty much immediately upon startup? > > > > > > This call is simply being moved from profile_impl.cc::DoFinalInit (which > calls > > > InitDataReductionProxySettings which used to call > > > MaybeActivateDataReductionProxy) to here. So the code being executed does > not > > > change, just its order. > > > > > > Here is everything this call does: > > > 1) Instantiate an instance of DataReductionProxyParams and > > > DataReductionProxyChromeSettings which should be quick. > > > 2) Set a pref and proxy configs. > > > 3) Make a URL request. This is asynchronous. > > > So I don't see any potential slowdown. > > > > That sounds fine. But if that's the case, why isn't profile_impl.cc included > in > > this CL? > > The method was moved out from InitDataReductionProxySettings() in > data_reduction_proxy_settings.cc, which is included in the cl. Here is the > overall before and after picture: > > Before this cl: > ProfileImpl::DoFinalInit() calls InitDataReductionProxySettings() which calls > MaybeActivateDataReductionProxy(). > > After this cl: > ProfileImpl::DoFinalInit() calls InitDataReductionProxySettings() > and > ProfileManager::DoFinalInit() calls MaybeActivateDataReductionProxy(). > > As for the relation between ProfileImpl::DoFinalInit() and > ProfileManager::DoFinalInit(): > ProfileImpl::DoFinalInit() calls ProfileManager::OnProfileCreated() which calls > ProfileManager::DoFinalInit(). > > Feel free to ping me on IM if this is not clear. Thanks for explaining! LGTM for profiles :)
On 2014/10/23 22:05:27, rpetterson wrote: > On 2014/10/23 21:58:47, kundaji wrote: > > On 2014/10/23 21:28:45, rpetterson wrote: > > > On 2014/10/23 20:08:51, kundaji wrote: > > > > > > > > > > https://codereview.chromium.org/665703002/diff/100001/chrome/browser/profiles... > > > > File chrome/browser/profiles/profile_manager.cc (right): > > > > > > > > > > > > > > https://codereview.chromium.org/665703002/diff/100001/chrome/browser/profiles... > > > > chrome/browser/profiles/profile_manager.cc:1017: > > > > DataReductionProxyChromeSettingsFactory::GetForBrowserContext(profile)-> > > > > On 2014/10/23 19:57:04, rpetterson wrote: > > > > > Is there any risk of this call slowing down startup since we create at > > least > > > 1 > > > > > profile pretty much immediately upon startup? > > > > > > > > This call is simply being moved from profile_impl.cc::DoFinalInit (which > > calls > > > > InitDataReductionProxySettings which used to call > > > > MaybeActivateDataReductionProxy) to here. So the code being executed does > > not > > > > change, just its order. > > > > > > > > Here is everything this call does: > > > > 1) Instantiate an instance of DataReductionProxyParams and > > > > DataReductionProxyChromeSettings which should be quick. > > > > 2) Set a pref and proxy configs. > > > > 3) Make a URL request. This is asynchronous. > > > > So I don't see any potential slowdown. > > > > > > That sounds fine. But if that's the case, why isn't profile_impl.cc included > > in > > > this CL? > > > > The method was moved out from InitDataReductionProxySettings() in > > data_reduction_proxy_settings.cc, which is included in the cl. Here is the > > overall before and after picture: > > > > Before this cl: > > ProfileImpl::DoFinalInit() calls InitDataReductionProxySettings() which calls > > MaybeActivateDataReductionProxy(). > > > > After this cl: > > ProfileImpl::DoFinalInit() calls InitDataReductionProxySettings() > > and > > ProfileManager::DoFinalInit() calls MaybeActivateDataReductionProxy(). > > > > As for the relation between ProfileImpl::DoFinalInit() and > > ProfileManager::DoFinalInit(): > > ProfileImpl::DoFinalInit() calls ProfileManager::OnProfileCreated() which > calls > > ProfileManager::DoFinalInit(). > > > > Feel free to ping me on IM if this is not clear. > > Thanks for explaining! LGTM for profiles :) aw lgtm
The CQ bit was checked by kundaji@chromium.org
The CQ bit was unchecked by kundaji@chromium.org
The CQ bit was checked by kundaji@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/665703002/100001
Message was sent while issue was closed.
Committed patchset #6 (id:100001)
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/8d8aeb6b0d6b5bc33b4ff957177ce28904e57a1e Cr-Commit-Position: refs/heads/master@{#300995} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
