New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 651157 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 25
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocked on:
issue 748910

Blocking:
issue 665064
issue 665179
issue 647412



Sign in to add a comment

Eliminate ash::NetworkingConfigDelegate

Project Member Reported by jamescook@chromium.org, Sep 28 2016

Issue description

// This delegate allows the UI code in ash, e.g. |NetworkStateListDetailedView|,
// to access the |NetworkingConfigService| in order to determine whether the
// configuration of a network identified by its |service_path| is controlled by
// an extension.

Chrome could probably push this information to ash on startup and on extension update.

 
Blockedon: 665179
Blocking: 665179
Blockedon: -665179
Cc: abodenha@chromium.org
@James, who should be the owner for this? You?
Status: Available (was: Untriaged)
Anyone on mustash. Will status=Available get it out of your triage queue?

Owner: jamescook@chromium.org
Status: Started (was: Available)
Steven, it looks like this code is related to identifying networks controlled by captive portal extensions. Is there a good extension to test this with?

Cc: emaxx@chromium.org cschuet@chromium.org
That's correct. Unfortunately I don't know of a good extension to test this with. The code was added by the enterprise team (cschuet@ and pneubeck@).

+cschuet@, +emaxx@ - Can you point James to an extension to test this code with?
Cc: -steve...@chromium.org jamescook@chromium.org
Owner: steve...@chromium.org
Status: Assigned (was: Started)
Summary: Eliminate ash::NetworkingConfigDelegate and use mojo NetworkState service (was: mustash: Need ash::NetworkingConfigDelegate implementation)
To stevenjb per offline discussion. This delegate should go away entirely and be replaced by both ash and chrome talking to a NetworkConfig / NetworkState mojo service.

I've written a browser test that verifies the correct icon shows in the system tray, so we'll have test coverage during the eventual switchover.

Attached is a very simple test extension I wrote.
networking_config.tar.gz
659 bytes Download
Project Member

Comment 10 by bugdroid1@chromium.org, Dec 8 2016

Project Member

Comment 11 by bugdroid1@chromium.org, Dec 9 2016

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

commit ab5d67b88547ab4b54c2d0fee2783191cb556c6d
Author: yhirano <yhirano@chromium.org>
Date: Fri Dec 09 05:49:50 2016

Revert of Add test for system tray network menu icon for extension-controlled networks (patchset #3 id:40001 of https://codereview.chromium.org/2558083003/ )

Reason for revert:
NetworkingConfigDelegateChromeosTest.SystemTrayItem is failing on Linux Chromium OS ASan LSan Tests: see http://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=browser_tests&tests=SystemTrayItem.

Original issue's description:
> Add test for system tray network menu icon for extension-controlled networks
>
> Eventually extension-controlled network information will come out of a mojo
> service. This provides test coverage to make sure the icon shows up in the
> system tray menu.
>
> BUG= 651157 
> TEST=chrome browser_tests
>
> Committed: https://crrev.com/af700b84d792456061162fc8fdcd08065ad8773a
> Cr-Commit-Position: refs/heads/master@{#437341}

TBR=stevenjb@chromium.org,jamescook@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 651157 

Review-Url: https://codereview.chromium.org/2563943002
Cr-Commit-Position: refs/heads/master@{#437480}

[modify] https://crrev.com/ab5d67b88547ab4b54c2d0fee2783191cb556c6d/ash/common/system/networking_config_delegate.h
[modify] https://crrev.com/ab5d67b88547ab4b54c2d0fee2783191cb556c6d/ash/common/system/tray/system_tray.cc
[modify] https://crrev.com/ab5d67b88547ab4b54c2d0fee2783191cb556c6d/ash/common/system/tray/system_tray.h
[delete] https://crrev.com/e535009c983cfe65f82ac47b624c37ab81bc98e0/chrome/browser/ui/ash/networking_config_delegate_chromeos_browsertest.cc
[modify] https://crrev.com/ab5d67b88547ab4b54c2d0fee2783191cb556c6d/chrome/test/BUILD.gn
[delete] https://crrev.com/e535009c983cfe65f82ac47b624c37ab81bc98e0/chrome/test/data/extensions/networking_config_delegate/background.js
[delete] https://crrev.com/e535009c983cfe65f82ac47b624c37ab81bc98e0/chrome/test/data/extensions/networking_config_delegate/manifest.json

Project Member

Comment 12 by bugdroid1@chromium.org, Dec 9 2016

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

commit 4b7224833d7a739afb2cb1758a2acbe6509d12cc
Author: jamescook <jamescook@chromium.org>
Date: Fri Dec 09 20:35:10 2016

Reland: Add test for system tray network menu icon for extension-controlled networks

Original CL http://crrev.com/2558083003 was reverted for use-after-free
caused by my test network detailed view registering observers that were
not properly cleaned up. Switched to using the real system menu network
detailed view.

Eventually extension-controlled network information will come out of a mojo
service. This provides test coverage to make sure the icon shows up in the
system tray menu.

BUG= 651157 
TEST=chrome browser_tests

Review-Url: https://codereview.chromium.org/2565823002
Cr-Commit-Position: refs/heads/master@{#437627}

[modify] https://crrev.com/4b7224833d7a739afb2cb1758a2acbe6509d12cc/ash/common/system/networking_config_delegate.h
[modify] https://crrev.com/4b7224833d7a739afb2cb1758a2acbe6509d12cc/ash/common/system/tray/system_tray.cc
[modify] https://crrev.com/4b7224833d7a739afb2cb1758a2acbe6509d12cc/ash/common/system/tray/system_tray.h
[add] https://crrev.com/4b7224833d7a739afb2cb1758a2acbe6509d12cc/chrome/browser/ui/ash/networking_config_delegate_chromeos_browsertest.cc
[modify] https://crrev.com/4b7224833d7a739afb2cb1758a2acbe6509d12cc/chrome/test/BUILD.gn
[add] https://crrev.com/4b7224833d7a739afb2cb1758a2acbe6509d12cc/chrome/test/data/extensions/networking_config_delegate/background.js
[add] https://crrev.com/4b7224833d7a739afb2cb1758a2acbe6509d12cc/chrome/test/data/extensions/networking_config_delegate/manifest.json

Blocking: 665064
Labels: M-64
Status: Started (was: Assigned)
Ah, yes, I remember this now. This is going to be fun.

I am realizing that for a lot of reasons (e.g. interdependencies between network configuraitons and extensions, also certs), network configuration is going to be tied to the browser process for a long time to come.

That should be fine, and should simplify a lot of things, it just means that when we create a network configuration mojo service it will live in the browser process.

As long as that is true, then we can just have NetworkState cache the extension information and have NetworkingConfigService notify NetworkStateHandler of any changes, and make retrieving tha tinformation part of the network configuraiton API.

Blockedon: 748910
Cc: lgcheng@google.com
I am going to wait for https://chromium-review.googlesource.com/c/chromium/src/+/703974 to land and build on the generalized "vpn_proivder_id", converting it to an even more generic "provider_id" and add a "provider_name" property as well.

Then NetworkingConfigService can fill in these properties for WiFi and we can use them directly in the Network UI without the need for NetworkingConfigDelegate.

Labels: -M-64 M-65
Status: Assigned (was: Started)
I was incorrect as to what these are used for. This is actually used for extensions that provide captive portal configuration for whitelisted wifi networks.

I will need to track down an author of this feature to get a test extension (or write my own) so that I can properly test the feature and the changes.

Because of the added complexity I am putting this on the backburner for now. 

Labels: -M-65 M-66
Labels: -M-66 M-67
Components: Internals>Services>Ash
Labels: -Proj-Mustash-Mash
Labels: -M-67 M-68
Labels: -M-68 M-69
Blockedon: 862420
Labels: -M-69 Target-70
Status: Started (was: Assigned)
Summary: Eliminate ash::NetworkingConfigDelegate (was: Eliminate ash::NetworkingConfigDelegate and use mojo NetworkState service)
The implementation doesn't actually require a mojo service. Instead we can just update NetworkState directly from NetworkingConfigService, since extensions and NetworkState will both be part of the Chrome process for the forseeable future.

(Eventually we can use a mojo service if NetworkHandler is extracted from the browser/extension code, but that will be a significant undertaking regardless).
Project Member

Comment 25 by bugdroid1@chromium.org, Jul 19

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

commit 87bfdeebdf0f8a470805dc5cbe962ef5686cc1e9
Author: Steven Bennetts <stevenjb@chromium.org>
Date: Thu Jul 19 21:37:55 2018

Convert NetworkUIData to base::Value (cleanup)

This change uses base::Value instead of base::DictionaryValue in
NetworkUIData. This migrates us towards the eventualy deprecation
of DictionaryValue and will simplify some other re-factoring.

Bug:  651157 
Change-Id: I2d4b9e61607b77852ece44de9990021d9b912b93
Reviewed-on: https://chromium-review.googlesource.com/1143598
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576642}
[modify] https://crrev.com/87bfdeebdf0f8a470805dc5cbe962ef5686cc1e9/chromeos/network/managed_network_configuration_handler_impl.cc
[modify] https://crrev.com/87bfdeebdf0f8a470805dc5cbe962ef5686cc1e9/chromeos/network/network_ui_data.cc
[modify] https://crrev.com/87bfdeebdf0f8a470805dc5cbe962ef5686cc1e9/chromeos/network/network_ui_data.h
[modify] https://crrev.com/87bfdeebdf0f8a470805dc5cbe962ef5686cc1e9/chromeos/network/network_ui_data_unittest.cc
[modify] https://crrev.com/87bfdeebdf0f8a470805dc5cbe962ef5686cc1e9/chromeos/network/onc/onc_utils.cc
[modify] https://crrev.com/87bfdeebdf0f8a470805dc5cbe962ef5686cc1e9/chromeos/network/policy_applicator.cc
[modify] https://crrev.com/87bfdeebdf0f8a470805dc5cbe962ef5686cc1e9/chromeos/network/policy_util.cc
[modify] https://crrev.com/87bfdeebdf0f8a470805dc5cbe962ef5686cc1e9/chromeos/network/shill_property_util.cc

Project Member

Comment 26 by bugdroid1@chromium.org, Jul 25

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

commit 8572b0675cd615ec64c4486a4a308e3eabac8bca
Author: Steven Bennetts <stevenjb@chromium.org>
Date: Wed Jul 25 16:44:33 2018

Use NetworkState to provide captive portal provider info

This CL eliminates NetworkingConfigDelegate by providing the
captive portal provider info in NetworkState directly.

This CL also:
* Adds NetworkState::ProviderInfo struct for both VPN and captive portal
* Renames NetworkingConfigDelegateChromeosTest ->
  NetworkingConfigChromeosTest since it wasn't testing the delegate
  per-se, but the functionality it was providing.

Bug:  651157 

For trivial rename in arc_net_host_impl.cc:
TBR=hidehiko@chromium.org

Change-Id: Iab872220aaedbcf27b4228581931e39c43dbab97
Reviewed-on: https://chromium-review.googlesource.com/1141210
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577938}
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/ash/BUILD.gn
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/ash/shell/shell_delegate_impl.cc
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/ash/shell/shell_delegate_impl.h
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/ash/shell_delegate.h
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/ash/shell_delegate_mash.cc
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/ash/shell_delegate_mash.h
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/ash/system/network/network_list.cc
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/ash/system/network/vpn_list_view.cc
[delete] https://crrev.com/41fe91f97ef8455cbb7b1673954545050cb8e86c/ash/system/networking_config_delegate.cc
[delete] https://crrev.com/41fe91f97ef8455cbb7b1673954545050cb8e86c/ash/system/networking_config_delegate.h
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/ash/test_shell_delegate.cc
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/ash/test_shell_delegate.h
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/chrome/browser/ui/ash/chrome_shell_delegate.cc
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/chrome/browser/ui/ash/chrome_shell_delegate.h
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/chrome/browser/ui/ash/network/DEPS
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/chrome/browser/ui/ash/network/network_state_notifier.cc
[rename] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/chrome/browser/ui/ash/network/networking_config_chromeos_browsertest.cc
[delete] https://crrev.com/41fe91f97ef8455cbb7b1673954545050cb8e86c/chrome/browser/ui/ash/network/networking_config_delegate_chromeos.cc
[delete] https://crrev.com/41fe91f97ef8455cbb7b1673954545050cb8e86c/chrome/browser/ui/ash/network/networking_config_delegate_chromeos.h
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/chrome/browser/ui/ash/system_tray_client.cc
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/chrome/browser/ui/webui/settings/chromeos/internet_handler.cc
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/chrome/test/BUILD.gn
[rename] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/chrome/test/data/extensions/networking_config/background.js
[rename] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/chrome/test/data/extensions/networking_config/manifest.json
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/chromeos/network/network_connect.cc
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/chromeos/network/network_state.cc
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/chromeos/network/network_state.h
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/chromeos/network/network_state_handler.cc
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/chromeos/network/network_state_handler.h
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/chromeos/network/network_state_handler_unittest.cc
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/chromeos/network/network_state_unittest.cc
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/components/arc/net/arc_net_host_impl.cc
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/extensions/browser/api/networking_config/networking_config_service.cc
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/extensions/browser/api/networking_config/networking_config_service_chromeos_unittest.cc
[modify] https://crrev.com/8572b0675cd615ec64c4486a4a308e3eabac8bca/testing/buildbot/filters/mash.browser_tests.filter

Labels: -Target-70 M-70
Status: Fixed (was: Started)
Project Member

Comment 28 by bugdroid1@chromium.org, Jul 25

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

commit 5fabb2c2ad0bcfce811c93db532d3f97d9ffef51
Author: Steven Bennetts <stevenjb@chromium.org>
Date: Wed Jul 25 18:53:32 2018

Convert onc::ReadDictionaryFromJson to use base::Value

This includes some partial conversion to base::Value in onc_utils.cc,
as part of a long term effort to convert network config and ONC code
to non deprecated base::Value patterns.

Bug:  651157 
Change-Id: Ic0456f9221fedaf95a78ce670af9f4be4bc95ad1
Reviewed-on: https://chromium-review.googlesource.com/1144196
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577997}
[modify] https://crrev.com/5fabb2c2ad0bcfce811c93db532d3f97d9ffef51/chrome/browser/chromeos/policy/configuration_policy_handler_chromeos.cc
[modify] https://crrev.com/5fabb2c2ad0bcfce811c93db532d3f97d9ffef51/chrome/browser/chromeos/policy/network_configuration_updater_unittest.cc
[modify] https://crrev.com/5fabb2c2ad0bcfce811c93db532d3f97d9ffef51/chrome/browser/chromeos/proxy_config_service_impl_unittest.cc
[modify] https://crrev.com/5fabb2c2ad0bcfce811c93db532d3f97d9ffef51/chromeos/network/managed_network_configuration_handler_unittest.cc
[modify] https://crrev.com/5fabb2c2ad0bcfce811c93db532d3f97d9ffef51/chromeos/network/network_state.cc
[modify] https://crrev.com/5fabb2c2ad0bcfce811c93db532d3f97d9ffef51/chromeos/network/network_state_test.cc
[modify] https://crrev.com/5fabb2c2ad0bcfce811c93db532d3f97d9ffef51/chromeos/network/onc/onc_translator_shill_to_onc.cc
[modify] https://crrev.com/5fabb2c2ad0bcfce811c93db532d3f97d9ffef51/chromeos/network/onc/onc_utils.cc
[modify] https://crrev.com/5fabb2c2ad0bcfce811c93db532d3f97d9ffef51/chromeos/network/onc/onc_utils.h
[modify] https://crrev.com/5fabb2c2ad0bcfce811c93db532d3f97d9ffef51/chromeos/network/onc/onc_utils_unittest.cc
[modify] https://crrev.com/5fabb2c2ad0bcfce811c93db532d3f97d9ffef51/chromeos/network/onc/onc_validator_unittest.cc
[modify] https://crrev.com/5fabb2c2ad0bcfce811c93db532d3f97d9ffef51/chromeos/network/shill_property_util.cc

Project Member

Comment 29 by bugdroid1@chromium.org, Jul 27

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

commit 3e470ac65be5d44632140b10eb02ca25e4a9de69
Author: Steven Bennetts <stevenjb@chromium.org>
Date: Fri Jul 27 18:29:27 2018

ProxyConfigDictionary: Use base::Value

Additional base::Value cleanup in preparation for some NetworkState
cleanup. This causes ProxyConfigDictionary to conform to the new
base::Value patterns.

Specifically ProxyConfigDictionary now uses base::Value instead of
std::unique_ptr<base::DictionaryValue> and pass by value semantics.

This CL changes ProxyConfigDictionary::GetPacMandatory to return false
if kProxyPacMandatory is not present, making the method consistent with
other similar methods and consistent with the usage in the one call
site that checks the result (proxy_api_helpers::CreatePacScriptDict).
If that logic is incorrect and not covered by any tests (unlikely,
test coverage for this code is quite thorough), then we will see the
following error:
"Invalid proxy configuration. Missing PAC mandatory field."

For trivial changes to c/b/extensions and c/b/net:
TBR=mattm@chromium.org, benwells@chromium.org

Bug:  651157 
Change-Id: I6fa01f2be11c8484d8783c3bf94445c3c4addbc7
Reviewed-on: https://chromium-review.googlesource.com/1150846
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578718}
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/chrome/browser/chromeos/arc/intent_helper/arc_settings_service_browsertest.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/chrome/browser/chromeos/proxy_config_service_impl_unittest.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/chrome/browser/extensions/api/declarative_net_request/declarative_net_request_browsertest.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/chrome/browser/extensions/api/proxy/proxy_api.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/chrome/browser/extensions/api/proxy/proxy_api_helpers.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/chrome/browser/extensions/api/proxy/proxy_api_helpers.h
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/chrome/browser/extensions/api/proxy/proxy_api_helpers_unittest.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/chrome/browser/extensions/api/proxy/proxy_apitest.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/chrome/browser/extensions/api/web_request/web_request_apitest.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/chrome/browser/net/load_timing_browsertest.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/chrome/browser/net/network_context_configuration_browsertest.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/chrome/browser/prefs/chrome_command_line_pref_store.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/chrome/browser/prefs/chrome_command_line_pref_store_unittest.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/chrome/browser/prefs/proxy_policy_unittest.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/chromeos/network/onc/onc_translator_onc_to_shill.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/chromeos/network/onc/onc_translator_shill_to_onc.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/chromeos/network/onc/onc_utils.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/chromeos/network/onc/onc_utils.h
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/chromeos/network/onc/onc_utils_unittest.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/chromeos/network/proxy/proxy_config_handler.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/chromeos/network/proxy/proxy_config_service_impl.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/chromeos/network/proxy/ui_proxy_config.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/chromeos/network/proxy/ui_proxy_config.h
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/chromeos/network/proxy/ui_proxy_config_service.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/components/policy/core/browser/proxy_policy_handler.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/components/policy/core/browser/proxy_policy_handler_unittest.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/components/prefs/pref_registry_simple.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/components/prefs/pref_registry_simple.h
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/components/proxy_config/pref_proxy_config_tracker_impl.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/components/proxy_config/pref_proxy_config_tracker_impl_unittest.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/components/proxy_config/proxy_config_dictionary.cc
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/components/proxy_config/proxy_config_dictionary.h
[modify] https://crrev.com/3e470ac65be5d44632140b10eb02ca25e4a9de69/components/proxy_config/proxy_config_dictionary_unittest.cc

Project Member

Comment 30 by bugdroid1@chromium.org, Jul 27

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

commit c0130c96ff7022547997b91dbbfbade20d985b0f
Author: Steven Bennetts <stevenjb@chromium.org>
Date: Fri Jul 27 21:05:15 2018

Use unique_ptr for NetworkState IP and Proxy configs.

IP config only exists for connected networks.
Proxy config only exists for networks with a proxy configured.

Storing these as a DictionaryValue takes 32 bytes, whereas storing these
as a unique_ptr<> only takes 8 bytes. It also makes the symantics of
whether or not the property as set a bit more clear.

Bug:  651157 
Change-Id: I4ab04ada996406bcace85c9038ac35f32b77054d
Reviewed-on: https://chromium-review.googlesource.com/1152148
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578787}
[modify] https://crrev.com/c0130c96ff7022547997b91dbbfbade20d985b0f/chrome/browser/chromeos/net/network_portal_detector_impl.cc
[modify] https://crrev.com/c0130c96ff7022547997b91dbbfbade20d985b0f/chrome/browser/ui/webui/chromeos/login/network_state_informer.cc
[modify] https://crrev.com/c0130c96ff7022547997b91dbbfbade20d985b0f/chromeos/network/managed_network_configuration_handler_impl.cc
[modify] https://crrev.com/c0130c96ff7022547997b91dbbfbade20d985b0f/chromeos/network/network_change_notifier_chromeos_unittest.cc
[modify] https://crrev.com/c0130c96ff7022547997b91dbbfbade20d985b0f/chromeos/network/network_state.cc
[modify] https://crrev.com/c0130c96ff7022547997b91dbbfbade20d985b0f/chromeos/network/network_state.h
[modify] https://crrev.com/c0130c96ff7022547997b91dbbfbade20d985b0f/chromeos/network/proxy/proxy_config_handler.cc

Comment 31 by steve...@chromium.org, Jan 18 (5 days ago)

Blockedon: -862420

Sign in to add a comment