Delete old NetworkQualityEstimator Cronet API

Delete old NetworkQualityEstimator Cronet API

Now that RequestFinishedListener doesn't depend on NQE's executor, the
rest of this API can go away.

BUG=618034
Committed: https://crrev.com/c1c13d3a04dad56eef598824921cdb5d78be5997
Cr-Commit-Position: refs/heads/master@{#408754}

[mgersh, 2016-07-26 20:17:58]

mgersh@chromium.org changed reviewers:
+ tbansal@chromium.org, xunjieli@chromium.org

[mgersh, 2016-07-26 20:17:58]

PTAL. As you saw in the design doc, I was deleting everything left that depended
on the old NQE executor, so I decided to finish removing it while I was there.

[tbansal1, 2016-07-26 20:20:36]

I think you will also need to remove some tests in
CronetUrlRequestContextTest.java

[xunjieli, 2016-07-26 20:20:42]

I am a bit confused. Why does this depend on the other CL (2178053002)?
Shouldn't we go the other way -- making the other CL based on top of this?

[mgersh, 2016-07-26 20:32:19]

On 2016/07/26 20:20:42, xunjieli wrote:
> I am a bit confused. Why does this depend on the other CL (2178053002)?
> Shouldn't we go the other way -- making the other CL based on top of this?

xunjieli: We can't delete enableNetworkQualityEstimator(Executor) and friends
until after RequestFinishedListener provides its own. The current (old) version
of RequestFinishedListener needs these to function.

tbansal: I believe what you're referring to was done in the patchset this
depends on. Sorry, the split might be a bit confusing, I originally wrote them
as a single CL.

[tbansal1, 2016-07-26 20:42:31]

On 2016/07/26 20:32:19, mgersh wrote:
> On 2016/07/26 20:20:42, xunjieli wrote:
> > I am a bit confused. Why does this depend on the other CL (2178053002)?
> > Shouldn't we go the other way -- making the other CL based on top of this?
> 
> xunjieli: We can't delete enableNetworkQualityEstimator(Executor) and friends
> until after RequestFinishedListener provides its own. The current (old)
version
> of RequestFinishedListener needs these to function.
> 
> tbansal: I believe what you're referring to was done in the patchset this
> depends on. Sorry, the split might be a bit confusing, I originally wrote them
> as a single CL.

IIUC, the other CL adds new API. This CL removes legacy API.
So, it makes sense for this CL to depend on the other one.

However, what happens to the embedders? Would they not break as soon as this
they use next Cronet version?

[mgersh, 2016-07-26 20:46:14]

On 2016/07/26 20:42:31, tbansal1 wrote:
> On 2016/07/26 20:32:19, mgersh wrote:
> > On 2016/07/26 20:20:42, xunjieli wrote:
> > > I am a bit confused. Why does this depend on the other CL (2178053002)?
> > > Shouldn't we go the other way -- making the other CL based on top of this?
> > 
> > xunjieli: We can't delete enableNetworkQualityEstimator(Executor) and
friends
> > until after RequestFinishedListener provides its own. The current (old)
> version
> > of RequestFinishedListener needs these to function.
> > 
> > tbansal: I believe what you're referring to was done in the patchset this
> > depends on. Sorry, the split might be a bit confusing, I originally wrote
them
> > as a single CL.
> 
> IIUC, the other CL adds new API. This CL removes legacy API.
> So, it makes sense for this CL to depend on the other one.
> 
> However, what happens to the embedders? Would they not break as soon as this
> they use next Cronet version?

Misha and I talked about this and decided that we'll update the embedders when
we update to the Cronet version with this in it, since there aren't very many
using RequestFinishedListener.

[tbansal1, 2016-07-26 20:51:40]

lgtm. I think with this CL all TODOs for bug 618034 are gone?

[xunjieli, 2016-07-29 18:23:03]

LGTM!

[mgersh, 2016-07-29 18:47:05]

The CQ bit was checked by mgersh@chromium.org

[mgersh, 2016-07-29 18:47:05]

The patchset sent to the CQ was uploaded after l-g-t-m from tbansal@chromium.org
Link to the patchset: https://codereview.chromium.org/2188553002/#ps60001
(title: "rebase")

[commit-bot: I haz the power, 2016-07-29 18:48:01]

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

[commit-bot: I haz the power, 2016-07-29 18:48:02]

This CL has an open dependency (Issue 2178053002 Patch 60001). Please resolve
the dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[mgersh, 2016-07-29 19:17:59]

The CQ bit was checked by mgersh@chromium.org

[commit-bot: I haz the power, 2016-07-29 19:18:59]

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 19:22:06]

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

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

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

[mgersh, 2016-07-29 19:33:08]

The CQ bit was checked by mgersh@chromium.org

[mgersh, 2016-07-29 19:33:08]

The patchset sent to the CQ was uploaded after l-g-t-m from
xunjieli@chromium.org, tbansal@chromium.org
Link to the patchset: https://codereview.chromium.org/2188553002/#ps80001
(title: "rebase again. ugh.")

[commit-bot: I haz the power, 2016-07-29 19:33:39]

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 21:16:17]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-07-29 21:18:10]

Description was changed from

==========
Delete old NetworkQualityEstimator Cronet API

Now that RequestFinishedListener doesn't depend on NQE's executor, the
rest of this API can go away.

BUG=618034
==========

to

==========
Delete old NetworkQualityEstimator Cronet API

Now that RequestFinishedListener doesn't depend on NQE's executor, the
rest of this API can go away.

BUG=618034

Committed: https://crrev.com/c1c13d3a04dad56eef598824921cdb5d78be5997
Cr-Commit-Position: refs/heads/master@{#408754}
==========

[commit-bot: I haz the power, 2016-07-29 21:18:11]

Patchset 5 (id:??) landed as
https://crrev.com/c1c13d3a04dad56eef598824921cdb5d78be5997
Cr-Commit-Position: refs/heads/master@{#408754}