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

Issue 660769 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Feb 2017
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

NetworkThrottlingObserverTest.ThrottlingChangeCallsShill has been failing on Chrome OS bot

Project Member Reported by tyoshino@chromium.org, Oct 31 2016

Issue description

Added a fix here: https://codereview.chromium.org/2465743003/
Could you help LGTM?

Alas, your fix landed but there's still memory leaks: https://uberchromegw.corp.google.com/i/chromium.memory/builders/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/builds/17328

I'll revert both your changes now.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 31 2016

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

commit c90183bb2178b4778b80d4705361b838577dc300
Author: phoglund <phoglund@chromium.org>
Date: Mon Oct 31 12:43:18 2016

Revert of Fix memory leak in NetworkThrottlingObserverTest (patchset #2 id:20001 of https://codereview.chromium.org/2465743003/ )

Reason for revert:
Fix doesn't appear help. Reverting to be able to revert base CL.

BUG= 660769 

Original issue's description:
> Fix memory leak in NetworkThrottlingObserverTest
>
> DBusThreadManager is initialized but not shut down in
> NetworkThrottlingObserverTest. This causes one of the trybots
> to fail.
> While there, fix a couple of style issues.
>
> BUG=chromium:634529, chromium:642811
>
> Committed: https://crrev.com/441d475906634f882776abee5847f04162226a73
> Cr-Commit-Position: refs/heads/master@{#428675}

TBR=thestig@chromium.org,stevenjb@chromium.org,kirtika@google.com,satorux@chromium.org,kirtika@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:634529, chromium:642811

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

[modify] https://crrev.com/c90183bb2178b4778b80d4705361b838577dc300/chrome/browser/chromeos/net/network_throttling_observer.cc
[modify] https://crrev.com/c90183bb2178b4778b80d4705361b838577dc300/chrome/browser/chromeos/net/network_throttling_observer_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 31 2016

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

commit 02a8965352416f5c892f511b56bf4c37c778f507
Author: phoglund <phoglund@chromium.org>
Date: Mon Oct 31 14:20:06 2016

Revert of Add network throttling as an enterprise policy (patchset #17 id:320001 of https://codereview.chromium.org/2364703002/ )

Reason for revert:
Introduces memory leak; see discussion in https://bugs.chromium.org/p/chromium/issues/detail?id=660769.

BUG= 660769 

Original issue's description:
> Add support for an enterprise policy that lets the admin
> turn on network throttling, and specify the upload and download rates in kbps to throttle to.
>
> BUG=642811
> TEST=(1) Unit-tests
> (2) Added network throttling policy to YAPS server,
> enabled throttling with various values, disabled,
> and checked that changes propagate to client device.
>
> Signed-off-by: Kirtika Ruchandani <kirtika@google.com>
> Committed: https://crrev.com/7db3c2b899d448dc0235e03a248df83aa5778ebf
> Cr-Commit-Position: refs/heads/master@{#428565}

TBR=bartfab@chromium.org,stevenjb@chromium.org,kirtika@google.com,michaelpg@chromium.org,mnissler@chromium.org,atwilson@chromium.org,gab@chromium.org,brettw@chromium.org,isherman@chromium.org,asvitkine@chromium.org,thestig@chromium.org,kirtika@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=642811

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

[modify] https://crrev.com/02a8965352416f5c892f511b56bf4c37c778f507/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/02a8965352416f5c892f511b56bf4c37c778f507/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
[modify] https://crrev.com/02a8965352416f5c892f511b56bf4c37c778f507/chrome/browser/chromeos/chrome_browser_main_chromeos.h
[delete] https://crrev.com/fc1aaf6d1cc0a4191e0ca50487598a9b6ea7622a/chrome/browser/chromeos/net/network_throttling_observer.cc
[delete] https://crrev.com/fc1aaf6d1cc0a4191e0ca50487598a9b6ea7622a/chrome/browser/chromeos/net/network_throttling_observer.h
[delete] https://crrev.com/fc1aaf6d1cc0a4191e0ca50487598a9b6ea7622a/chrome/browser/chromeos/net/network_throttling_observer_unittest.cc
[modify] https://crrev.com/02a8965352416f5c892f511b56bf4c37c778f507/chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc
[modify] https://crrev.com/02a8965352416f5c892f511b56bf4c37c778f507/chrome/browser/chromeos/policy/proto/chrome_device_policy.proto
[modify] https://crrev.com/02a8965352416f5c892f511b56bf4c37c778f507/chrome/browser/policy/configuration_policy_handler_list_factory.cc
[modify] https://crrev.com/02a8965352416f5c892f511b56bf4c37c778f507/chrome/browser/prefs/browser_prefs.cc
[modify] https://crrev.com/02a8965352416f5c892f511b56bf4c37c778f507/chrome/common/pref_names.cc
[modify] https://crrev.com/02a8965352416f5c892f511b56bf4c37c778f507/chrome/common/pref_names.h
[modify] https://crrev.com/02a8965352416f5c892f511b56bf4c37c778f507/chrome/test/data/policy/policy_test_cases.json
[modify] https://crrev.com/02a8965352416f5c892f511b56bf4c37c778f507/chromeos/dbus/fake_shill_manager_client.cc
[modify] https://crrev.com/02a8965352416f5c892f511b56bf4c37c778f507/chromeos/dbus/fake_shill_manager_client.h
[modify] https://crrev.com/02a8965352416f5c892f511b56bf4c37c778f507/chromeos/dbus/mock_shill_manager_client.h
[modify] https://crrev.com/02a8965352416f5c892f511b56bf4c37c778f507/chromeos/dbus/shill_manager_client.cc
[modify] https://crrev.com/02a8965352416f5c892f511b56bf4c37c778f507/chromeos/dbus/shill_manager_client.h
[modify] https://crrev.com/02a8965352416f5c892f511b56bf4c37c778f507/chromeos/dbus/shill_manager_client_unittest.cc
[modify] https://crrev.com/02a8965352416f5c892f511b56bf4c37c778f507/chromeos/network/network_state_handler.cc
[modify] https://crrev.com/02a8965352416f5c892f511b56bf4c37c778f507/chromeos/network/network_state_handler.h
[modify] https://crrev.com/02a8965352416f5c892f511b56bf4c37c778f507/chromeos/network/shill_property_handler.cc
[modify] https://crrev.com/02a8965352416f5c892f511b56bf4c37c778f507/chromeos/network/shill_property_handler.h
[modify] https://crrev.com/02a8965352416f5c892f511b56bf4c37c778f507/components/policy/resources/policy_templates.json
[modify] https://crrev.com/02a8965352416f5c892f511b56bf4c37c778f507/tools/metrics/histograms/histograms.xml

Comment 5 by kirtika@google.com, Feb 1 2017

Status: Fixed (was: Assigned)
Was fixed with this re-land: https://codereview.chromium.org/2466693002

Sign in to add a comment