NQE: Move ECT to net:: namespace

NQE: Move ECT to net:: namespace

Move EffectiveConnectionType from NetworkQualityEstimator
class to net:: namespace

Also, move EffectiveConnectionType to a separate file in
//net/nqe/effective_connection_type.h.

TBR=nasko@chromium.org,dimich@chromium.org

BUG=631552, 631554
Committed: https://crrev.com/3eb690178fe3818530e84ee9bffffda40ca41edd
Cr-Commit-Position: refs/heads/master@{#408605}

[tbansal1, 2016-07-26 17:50:50]

The CQ bit was checked by tbansal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-26 17:51:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-26 17:59:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-26 17:59:55]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tbansal1, 2016-07-26 18:23:52]

The CQ bit was checked by tbansal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-26 18:24:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tbansal1, 2016-07-26 18:25:14]

Patchset #1 (id:1) has been deleted

[tbansal1, 2016-07-26 18:27:56]

Description was changed from

==========
Move effective connection type to a separate file

BUG=
==========

to

==========
Move NQE class to net::nqe namespace

With this CL, all public NQE code (one intended to be 
consumed from outside //net/nqe) is now in net::nqe 
namespace. All the internal code of NQE is in 
net::nqe::internal namespace.

Also, move EffectiveConnectionType to a separate file in
//net/nqe/effective_connection_type.h.

BUG=
==========

[commit-bot: I haz the power, 2016-07-26 18:33:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-26 18:33:01]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[tbansal1, 2016-07-26 18:39:43]

The CQ bit was checked by tbansal@chromium.org to run a CQ dry run

[tbansal1, 2016-07-26 18:39:46]

Patchset #1 (id:20001) has been deleted

[commit-bot: I haz the power, 2016-07-26 18:40:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tbansal1, 2016-07-26 18:41:53]

The CQ bit was checked by tbansal@chromium.org to run a CQ dry run

[tbansal1, 2016-07-26 18:41:57]

Patchset #1 (id:40001) has been deleted

[commit-bot: I haz the power, 2016-07-26 18:42:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tbansal1, 2016-07-26 18:50:53]

Description was changed from

==========
Move NQE class to net::nqe namespace

With this CL, all public NQE code (one intended to be 
consumed from outside //net/nqe) is now in net::nqe 
namespace. All the internal code of NQE is in 
net::nqe::internal namespace.

Also, move EffectiveConnectionType to a separate file in
//net/nqe/effective_connection_type.h.

BUG=
==========

to

==========
Move NQE class to net::nqe namespace

With this CL, all public NQE code (one intended to be 
consumed from outside //net/nqe) is now in net::nqe 
namespace. All the internal code of NQE is in 
net::nqe::internal namespace.

Also, move EffectiveConnectionType to a separate file in
//net/nqe/effective_connection_type.h.

BUG=631552,631554
==========

[tbansal1, 2016-07-26 18:54:43]

tbansal@chromium.org changed reviewers:
+ mmenke@chromium.org

[tbansal1, 2016-07-26 18:54:44]

mmenke: Can you please take a quick look at
//net/nqe/effective_connection_type.h and //net/nqe/network_quality_estimator.h.
I am primarily looking for a feedback on whether this is okay from //net general
style.

[commit-bot: I haz the power, 2016-07-26 18:55:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-26 18:55:14]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tbansal1, 2016-07-26 20:17:44]

The CQ bit was checked by tbansal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-26 20:18:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-26 20:34:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-26 20:34:15]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[tbansal1, 2016-07-26 20:46:37]

The CQ bit was checked by tbansal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-26 20:47:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tbansal1, 2016-07-26 20:54:12]

tbansal@chromium.org changed reviewers:
+ bengr@chromium.org

[tbansal1, 2016-07-26 20:54:13]

bengr: PTAL. Most significant changes are in
\\net\nqe\network_quality_estimator.h and
\\net\nqe\effective_connection_type.h.

Rest of the changes are mechanical: Simply updating the call sites to NQE.

Thanks.

[commit-bot: I haz the power, 2016-07-26 21:02:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-26 21:03:00]

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...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_gyp_rel on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gyp_...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[mmenke, 2016-07-26 21:37:13]

On 2016/07/26 20:54:13, tbansal1 wrote:
> bengr: PTAL. Most significant changes are in
> \\net\nqe\network_quality_estimator.h and
> \\net\nqe\effective_connection_type.h.
> 
> Rest of the changes are mechanical: Simply updating the call sites to NQE.
> 
> Thanks.

I believe the nested namespaces like this are frowned upon (At least that was
the result of a couple discussions on the subject, years ago)

[tbansal1, 2016-07-26 21:43:45]

On 2016/07/26 21:37:13, mmenke wrote:
> On 2016/07/26 20:54:13, tbansal1 wrote:
> > bengr: PTAL. Most significant changes are in
> > \\net\nqe\network_quality_estimator.h and
> > \\net\nqe\effective_connection_type.h.
> > 
> > Rest of the changes are mechanical: Simply updating the call sites to NQE.
> > 
> > Thanks.
> 
> I believe the nested namespaces like this are frowned upon (At least that was
> the result of a couple discussions on the subject, years ago)

Do you remember the reason? I do not see any related discussion
on net-dev or other groups. Also, there are some examples of this
(besides net::android)
https://cs.chromium.org/search/?q=-f:net+f:cc$+net::%5Bb-z%5D*::+case:yes&p=2...

[tbansal1, 2016-07-26 22:00:58]

On 2016/07/26 21:43:45, tbansal1 wrote:
> On 2016/07/26 21:37:13, mmenke wrote:
> > On 2016/07/26 20:54:13, tbansal1 wrote:
> > > bengr: PTAL. Most significant changes are in
> > > \\net\nqe\network_quality_estimator.h and
> > > \\net\nqe\effective_connection_type.h.
> > > 
> > > Rest of the changes are mechanical: Simply updating the call sites to NQE.
> > > 
> > > Thanks.
> > 
> > I believe the nested namespaces like this are frowned upon (At least that
was
> > the result of a couple discussions on the subject, years ago)
> 
> Do you remember the reason? I do not see any related discussion
> on net-dev or other groups. Also, there are some examples of this
> (besides net::android)
>
https://cs.chromium.org/search/?q=-f:net+f:cc$+net::%5Bb-z%5D*::+case:yes&p=2...

A simpler option might be to move enum EffectiveConnectionType
to a separate file
(//net/nqe/effective_connection_type.h) but in net namespace.
Is that acceptable? That way, the code outside of //net only
deals with namespace net (and not
nested namespaces). Is that acceptable?

[mmenke, 2016-07-27 15:01:41]

On 2016/07/26 22:00:58, tbansal1 wrote:
> On 2016/07/26 21:43:45, tbansal1 wrote:
> > On 2016/07/26 21:37:13, mmenke wrote:
> > > On 2016/07/26 20:54:13, tbansal1 wrote:
> > > > bengr: PTAL. Most significant changes are in
> > > > \\net\nqe\network_quality_estimator.h and
> > > > \\net\nqe\effective_connection_type.h.
> > > > 
> > > > Rest of the changes are mechanical: Simply updating the call sites to
NQE.
> > > > 
> > > > Thanks.
> > > 
> > > I believe the nested namespaces like this are frowned upon (At least that
> was
> > > the result of a couple discussions on the subject, years ago)
> > 
> > Do you remember the reason? I do not see any related discussion
> > on net-dev or other groups. Also, there are some examples of this
> > (besides net::android)
> >
>
https://cs.chromium.org/search/?q=-f:net+f:cc$+net::%5Bb-z%5D*::+case:yes&p=2...
> 
> A simpler option might be to move enum EffectiveConnectionType
> to a separate file
> (//net/nqe/effective_connection_type.h) but in net namespace.
> Is that acceptable? That way, the code outside of //net only
> deals with namespace net (and not
> nested namespaces). Is that acceptable?

I think that sounds reasonable (Bonus points for making it an enum class, too).

Hrm...I'm having trouble finding the conversation, or the reasoning for it,
unfortunately.  net::ct seems pretty problematic to me, as it doesn't match the
directory structure.

[tbansal1, 2016-07-28 18:10:20]

Patchset #4 (id:120001) has been deleted

[tbansal1, 2016-07-28 18:19:49]

Patchset #4 (id:140001) has been deleted

[tbansal1, 2016-07-28 18:23:56]

Description was changed from

==========
Move NQE class to net::nqe namespace

With this CL, all public NQE code (one intended to be 
consumed from outside //net/nqe) is now in net::nqe 
namespace. All the internal code of NQE is in 
net::nqe::internal namespace.

Also, move EffectiveConnectionType to a separate file in
//net/nqe/effective_connection_type.h.

BUG=631552,631554
==========

to

==========
Move ECT to net:: namespace

Also, move EffectiveConnectionType to a separate file in
//net/nqe/effective_connection_type.h.

BUG=631552,631554
==========

[tbansal1, 2016-07-28 18:24:37]

Description was changed from

==========
Move ECT to net:: namespace

Also, move EffectiveConnectionType to a separate file in
//net/nqe/effective_connection_type.h.

BUG=631552,631554
==========

to

==========
NQE: Move ECT to net:: namespace

Move EffectiveConnectionType from NetworkQualityEstimator 
class to net:: namespace

Also, move EffectiveConnectionType to a separate file in
//net/nqe/effective_connection_type.h.

BUG=631552,631554
==========

[tbansal1, 2016-07-28 18:24:49]

The CQ bit was checked by tbansal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-28 18:25:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tbansal1, 2016-07-28 18:27:31]

mmneke: PTAL. The most important change is in
//net/nqe/network_quality_estimator.h and
//net/nqe/effective_connection_type.h

Rest are all mechanical changes.

I did not change from enum to enum class yet since the variables in
EffectiveConnectionType are ordered. e.g.,
if(current_ect <= E_C_T_2G) { // do something }
If I change to enum class, I would need to add static casting everywhere. I am
not sure if that's cleaner (I can still do it, if  you think that's the right
way).

[commit-bot: I haz the power, 2016-07-28 18:35:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-28 18:35:20]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mmenke, 2016-07-28 18:39:35]

On 2016/07/28 18:27:31, tbansal1 wrote:
> mmneke: PTAL. The most important change is in
> //net/nqe/network_quality_estimator.h and
> //net/nqe/effective_connection_type.h
> 
> Rest are all mechanical changes.
> 
> I did not change from enum to enum class yet since the variables in
> EffectiveConnectionType are ordered. e.g.,
> if(current_ect <= E_C_T_2G) { // do something }
> If I change to enum class, I would need to add static casting everywhere. I am
> not sure if that's cleaner (I can still do it, if  you think that's the right
> way).

Static casting is ugly, so yea, best not to use enum classes if you need to do
that sort of comparison.

[mmenke, 2016-07-28 18:46:21]

On 2016/07/28 18:39:35, mmenke wrote:
> On 2016/07/28 18:27:31, tbansal1 wrote:
> > mmneke: PTAL. The most important change is in
> > //net/nqe/network_quality_estimator.h and
> > //net/nqe/effective_connection_type.h
> > 
> > Rest are all mechanical changes.
> > 
> > I did not change from enum to enum class yet since the variables in
> > EffectiveConnectionType are ordered. e.g.,
> > if(current_ect <= E_C_T_2G) { // do something }
> > If I change to enum class, I would need to add static casting everywhere. I
am
> > not sure if that's cleaner (I can still do it, if  you think that's the
right
> > way).
> 
> Static casting is ugly, so yea, best not to use enum classes if you need to do
> that sort of comparison.

Looks like there's  bunch of stuff to clean up in this CL
(EffectiveConnectionType.<enum value> -> <enum value>, stuff left over from
trying to use an enum class, etc).  Enough minor bugs that signing off seems
premature, though once fixed, I'll just stamp it.

[tbansal1, 2016-07-28 19:00:12]

The CQ bit was checked by tbansal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-28 19:00:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mmenke, 2016-07-28 19:44:12]

LGTM (And a tiny nit I forgot to publish).  Think this CL is TBRable.  I guess
it's just most of the content/ stuff that none of us owners.

https://codereview.chromium.org/2183153002/diff/180001/net/nqe/effective_conn...
File net/nqe/effective_connection_type.h (right):

https://codereview.chromium.org/2183153002/diff/180001/net/nqe/effective_conn...
net/nqe/effective_connection_type.h:14: // different, usually worse, from its
expected capabilities.
grammar nit:  different -> differently (It's an adverb meaning how it performs,
not the direct object of performs)

[commit-bot: I haz the power, 2016-07-28 20:33:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-28 20:33:16]

Dry run: This issue passed the CQ dry run.

[tbansal1, 2016-07-28 23:58:15]

Description was changed from

==========
NQE: Move ECT to net:: namespace

Move EffectiveConnectionType from NetworkQualityEstimator 
class to net:: namespace

Also, move EffectiveConnectionType to a separate file in
//net/nqe/effective_connection_type.h.

BUG=631552,631554
==========

to

==========
NQE: Move ECT to net:: namespace

Move EffectiveConnectionType from NetworkQualityEstimator 
class to net:: namespace

Also, move EffectiveConnectionType to a separate file in
//net/nqe/effective_connection_type.h.

TBR=nasko@chromium.org

BUG=631552,631554
==========

[tbansal1, 2016-07-28 23:58:22]

The CQ bit was checked by tbansal@chromium.org

[tbansal1, 2016-07-28 23:58:23]

The patchset sent to the CQ was uploaded after l-g-t-m from mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/2183153002/#ps200001
(title: "Rebased, addressed mmenke's comments")

[tbansal1, 2016-07-28 23:58:29]

https://codereview.chromium.org/2183153002/diff/180001/net/nqe/effective_conn...
File net/nqe/effective_connection_type.h (right):

https://codereview.chromium.org/2183153002/diff/180001/net/nqe/effective_conn...
net/nqe/effective_connection_type.h:14: // different, usually worse, from its
expected capabilities.
On 2016/07/28 19:44:12, mmenke wrote:
> grammar nit:  different -> differently (It's an adverb meaning how it
performs,
> not the direct object of performs)

Done.

[commit-bot: I haz the power, 2016-07-28 23:58:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tbansal1, 2016-07-28 23:59:01]

The CQ bit was unchecked by tbansal@chromium.org

[tbansal1, 2016-07-29 00:00:35]

Patchset #7 (id:220001) has been deleted

[tbansal1, 2016-07-29 00:12:12]

The CQ bit was checked by tbansal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-29 00:12:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tbansal1, 2016-07-29 00:12:47]

Description was changed from

==========
NQE: Move ECT to net:: namespace

Move EffectiveConnectionType from NetworkQualityEstimator 
class to net:: namespace

Also, move EffectiveConnectionType to a separate file in
//net/nqe/effective_connection_type.h.

TBR=nasko@chromium.org

BUG=631552,631554
==========

to

==========
NQE: Move ECT to net:: namespace

Move EffectiveConnectionType from NetworkQualityEstimator 
class to net:: namespace

Also, move EffectiveConnectionType to a separate file in
//net/nqe/effective_connection_type.h.

TBR=nasko@chromium.org,dimich@chromium.org

BUG=631552,631554
==========

[tbansal1, 2016-07-29 00:37:25]

The CQ bit was unchecked by tbansal@chromium.org

[tbansal1, 2016-07-29 00:37:36]

The CQ bit was checked by tbansal@chromium.org

[tbansal1, 2016-07-29 00:37:37]

The patchset sent to the CQ was uploaded after l-g-t-m from mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/2183153002/#ps240001
(title: "Moar rebasing")

[commit-bot: I haz the power, 2016-07-29 00:37:58]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-29 00:44:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-29 00:44:55]

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...)

[tbansal1, 2016-07-29 07:24:27]

The CQ bit was checked by tbansal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-29 07:24:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-29 08:22:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-29 08:22:25]

Dry run: This issue passed the CQ dry run.

[tbansal1, 2016-07-29 08:52:17]

The CQ bit was checked by tbansal@chromium.org

[tbansal1, 2016-07-29 08:52:17]

The patchset sent to the CQ was uploaded after l-g-t-m from mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/2183153002/#ps260001
(title: "Rebased, also fixed comnpilation error")

[commit-bot: I haz the power, 2016-07-29 08:52:28]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-29 08:56:11]

Description was changed from

==========
NQE: Move ECT to net:: namespace

Move EffectiveConnectionType from NetworkQualityEstimator 
class to net:: namespace

Also, move EffectiveConnectionType to a separate file in
//net/nqe/effective_connection_type.h.

TBR=nasko@chromium.org,dimich@chromium.org

BUG=631552,631554
==========

to

==========
NQE: Move ECT to net:: namespace

Move EffectiveConnectionType from NetworkQualityEstimator 
class to net:: namespace

Also, move EffectiveConnectionType to a separate file in
//net/nqe/effective_connection_type.h.

TBR=nasko@chromium.org,dimich@chromium.org

BUG=631552,631554
==========

[commit-bot: I haz the power, 2016-07-29 08:56:13]

Committed patchset #8 (id:260001)

[commit-bot: I haz the power, 2016-07-29 08:58:00]

Description was changed from

==========
NQE: Move ECT to net:: namespace

Move EffectiveConnectionType from NetworkQualityEstimator 
class to net:: namespace

Also, move EffectiveConnectionType to a separate file in
//net/nqe/effective_connection_type.h.

TBR=nasko@chromium.org,dimich@chromium.org

BUG=631552,631554
==========

to

==========
NQE: Move ECT to net:: namespace

Move EffectiveConnectionType from NetworkQualityEstimator
class to net:: namespace

Also, move EffectiveConnectionType to a separate file in
//net/nqe/effective_connection_type.h.

TBR=nasko@chromium.org,dimich@chromium.org

BUG=631552,631554

Committed: https://crrev.com/3eb690178fe3818530e84ee9bffffda40ca41edd
Cr-Commit-Position: refs/heads/master@{#408605}
==========

[commit-bot: I haz the power, 2016-07-29 08:58:02]

Patchset 8 (id:??) landed as
https://crrev.com/3eb690178fe3818530e84ee9bffffda40ca41edd
Cr-Commit-Position: refs/heads/master@{#408605}