This CL adds ConnectionType to the MaxBandwidthObserver notification as a convenience for its only client (in the downstream CL), NetInfo.
This CL also ensures that MaxBandwidthObserver is called on every platform when the network type changes.
Upstream of https://codereview.chromium.org/1306423004/
BUG=412741
Committed: https://crrev.com/bce491860ced584a398c76dca134ed350e879259
Cr-Commit-Position: refs/heads/master@{#349673}
https://codereview.chromium.org/1306653003/diff/80001/net/base/network_change_notifier.h File net/base/network_change_notifier.h (right): https://codereview.chromium.org/1306653003/diff/80001/net/base/network_change_notifier.h#newcode240 net/base/network_change_notifier.h:240: // types). See the NetInfo spec for the mapping ...
5 years, 3 months ago
(2015-09-11 18:56:11 UTC)
#8
https://codereview.chromium.org/1306653003/diff/140001/net/base/network_change_notifier.h File net/base/network_change_notifier.h (right): https://codereview.chromium.org/1306653003/diff/140001/net/base/network_change_notifier.h#newcode245 net/base/network_change_notifier.h:245: ConnectionType* connection_type); Are you going to change this API ...
5 years, 3 months ago
(2015-09-11 18:59:02 UTC)
#9
https://codereview.chromium.org/1306653003/diff/140001/net/base/network_change_notifier.h File net/base/network_change_notifier.h (right): https://codereview.chromium.org/1306653003/diff/140001/net/base/network_change_notifier.h#newcode245 net/base/network_change_notifier.h:245: ConnectionType* connection_type); On 2015/09/11 19:04:26, jkarlin wrote: > On ...
5 years, 3 months ago
(2015-09-11 19:18:59 UTC)
#11
https://codereview.chromium.org/1306653003/diff/140001/net/base/network_chang...
File net/base/network_change_notifier.h (right):
https://codereview.chromium.org/1306653003/diff/140001/net/base/network_chang...
net/base/network_change_notifier.h:245: ConnectionType* connection_type);
On 2015/09/11 19:04:26, jkarlin wrote:
> On 2015/09/11 18:59:02, pauljensen wrote:
> > Are you going to change this API to take pointers to both items as per our
> char
> > earlier?
>
> Ah, I took our convo. to mean that you were fine with either arrangement. I
> chose this one form so that I could
> DCHECK(GetMaxBandwidthAndConnectionType(nullptr) != ...) which is needed in
one
> of the files.
Oh, no, I was trying to say I wanted it to be:
void GetMaxBandwidthAndConnectionType(double* max_bandwidth_mbps,
ConnectionType* connection_type);
Sorry. It seems more consistent to me.
jkarlin
https://codereview.chromium.org/1306653003/diff/140001/net/base/network_change_notifier.h File net/base/network_change_notifier.h (right): https://codereview.chromium.org/1306653003/diff/140001/net/base/network_change_notifier.h#newcode245 net/base/network_change_notifier.h:245: ConnectionType* connection_type); On 2015/09/11 19:18:59, pauljensen wrote: > On ...
5 years, 3 months ago
(2015-09-11 20:07:24 UTC)
#12
https://codereview.chromium.org/1306653003/diff/140001/net/base/network_chang...
File net/base/network_change_notifier.h (right):
https://codereview.chromium.org/1306653003/diff/140001/net/base/network_chang...
net/base/network_change_notifier.h:245: ConnectionType* connection_type);
On 2015/09/11 19:18:59, pauljensen wrote:
> On 2015/09/11 19:04:26, jkarlin wrote:
> > On 2015/09/11 18:59:02, pauljensen wrote:
> > > Are you going to change this API to take pointers to both items as per our
> > char
> > > earlier?
> >
> > Ah, I took our convo. to mean that you were fine with either arrangement. I
> > chose this one form so that I could
> > DCHECK(GetMaxBandwidthAndConnectionType(nullptr) != ...) which is needed in
> one
> > of the files.
>
> Oh, no, I was trying to say I wanted it to be:
> void GetMaxBandwidthAndConnectionType(double* max_bandwidth_mbps,
> ConnectionType* connection_type);
> Sorry. It seems more consistent to me.
Done.
https://codereview.chromium.org/1306653003/diff/140001/net/base/network_chang...
net/base/network_change_notifier.h:400: ConnectionType* connection_type) const;
On 2015/09/11 18:59:02, pauljensen wrote:
> ditto
Done.
jkarlin
PTAL, thanks!
5 years, 3 months ago
(2015-09-14 14:37:40 UTC)
#13
PTAL, thanks!
pneubeck (no reviews)
chromeos/network/network_change_notifier_chromeos.* still lgtm
5 years, 3 months ago
(2015-09-14 16:23:52 UTC)
#14
chromeos/network/network_change_notifier_chromeos.*
still lgtm
pauljensen
Can we give back more accurate max bandwidths given a particular connection type? e.g. NCNLinux ...
5 years, 3 months ago
(2015-09-15 18:17:40 UTC)
#15
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1306653003/260001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1306653003/260001
5 years, 3 months ago
(2015-09-18 14:06:08 UTC)
#21
Issue 1306653003: Add connection type to NCN::MaxBandwidthChanged
(Closed)
Created 5 years, 3 months ago by jkarlin
Modified 5 years, 3 months ago
Reviewers: pauljensen, pneubeck (no reviews)
Base URL: https://chromium.googlesource.com/chromium/src.git@master
Comments: 29