New issue
Advanced search Search tips

Issue 780475 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Dec 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug
Proj-Servicification

Blocking:
issue 754695



Sign in to add a comment

Migrate components/drive/job_scheduler.h to use net::NetworkChangeNotifier::NetworkChangeObserver

Project Member Reported by xunji...@chromium.org, Nov 1 2017

Issue description

net::NetworkChangeNotifier's IPAddressObserver and ConnectionTypeObserver are deprecated and will be hidden soon. Please migrate components/drive/job_scheduler.h to use NetworkChangeObserver instead. 

See  Issue 754695  for the motivation.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 6 2017

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

commit 817fd56ec3e2dfec7f0f0b1eacd79b5455bf7e3c
Author: SexyTreeTrunks <apenr1234@gmail.com>
Date: Mon Nov 06 10:38:04 2017

Migrate ConnectionTypeObserver to NetworkChangeObserver in job_scheduler.h

Bug:  780475 
Change-Id: Ie6250fd2ea9221fe0bcd3578be0012527efaf11f
Reviewed-on: https://chromium-review.googlesource.com/752941
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Commit-Queue: Ryo Hashimoto <hashimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514108}
[modify] https://crrev.com/817fd56ec3e2dfec7f0f0b1eacd79b5455bf7e3c/AUTHORS
[modify] https://crrev.com/817fd56ec3e2dfec7f0f0b1eacd79b5455bf7e3c/components/drive/job_scheduler.cc
[modify] https://crrev.com/817fd56ec3e2dfec7f0f0b1eacd79b5455bf7e3c/components/drive/job_scheduler.h

Status: Fixed (was: Available)
Closing this. This is fixed by apenr1234@gmail.com
Status: Available (was: Fixed)
I think the submitted change may break drive jobs, see my comment: https://chromium-review.googlesource.com/c/chromium/src/+/752941/4/components/drive/job_scheduler.cc#1148
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 6 2017

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

commit 71ae178b28935c6cb845473c72d201a61988c9aa
Author: Helen Li <xunjieli@chromium.org>
Date: Mon Nov 06 15:18:05 2017

Revert "Migrate ConnectionTypeObserver to NetworkChangeObserver in job_scheduler.h"

This reverts commit 817fd56ec3e2dfec7f0f0b1eacd79b5455bf7e3c.

Reason for revert: This change will cause scheduler to run even when internet is disconnected

Original change's description:
> Migrate ConnectionTypeObserver to NetworkChangeObserver in job_scheduler.h
> 
> Bug:  780475 
> Change-Id: Ie6250fd2ea9221fe0bcd3578be0012527efaf11f
> Reviewed-on: https://chromium-review.googlesource.com/752941
> Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
> Commit-Queue: Ryo Hashimoto <hashimoto@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#514108}

TBR=hashimoto@chromium.org,hirono@chromium.org,fukino@chromium.org,hidehiko@chromium.org,apenr1234@gmail.com

Change-Id: I3484058e3078193aa8651bd8f9c8e08671741d60
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  780475 
Reviewed-on: https://chromium-review.googlesource.com/753710
Reviewed-by: Helen Li <xunjieli@chromium.org>
Commit-Queue: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514131}
[modify] https://crrev.com/71ae178b28935c6cb845473c72d201a61988c9aa/AUTHORS
[modify] https://crrev.com/71ae178b28935c6cb845473c72d201a61988c9aa/components/drive/job_scheduler.cc
[modify] https://crrev.com/71ae178b28935c6cb845473c72d201a61988c9aa/components/drive/job_scheduler.h

Thanks. I have scheduled a revert. 
apenr1234@gmail.com: If you'd like to reland the change, can you apply the suggested edit per comment #3?
If you do so, could you also use your real name in "Author" field in Gerrit? Previous names that you've used ("SexyTreeTrunks" and "DdoDdoGirl") aren't usually seen in Chromium. 

void JobScheduler::OnNetworkChanged(net::NetworkChangeNotifier::ConnectionType type) {
  if (type == net::NetworkChangeNotifier::CONNECTION_NONE)
    return;
  ...
}


Comment 6 by laforge@google.com, Nov 7 2017

Components: Internals>Network>Service

Comment 7 by laforge@google.com, Nov 7 2017

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.
Thanks for your comment. I will reflect your comment in code and also in my username in Gerrit! 
After this issue is resolved, I want to process https://bugs.chromium.org/p/chromium/issues/detail?id=780480
Will this issue need code for handling CONNECTION_NONE signal?
Project Member

Comment 9 by bugdroid1@chromium.org, Dec 6 2017

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

commit edaf7f063eab7315cb215f0717be59fbfa8c4d69
Author: Minjeong Lee <apenr1234@gmail.com>
Date: Wed Dec 06 07:03:46 2017

Add handling code when ConnectionType is CONNECTION_NONE in JobScheduler::OnNetworkChanged

Migrate ConnectionTypeObserver to NetworkChangeObserver in job_scheduler.h

Bug:  780475 
Change-Id: I4b5699a703550bd61b11bdbed66cfee712090ff4
Reviewed-on: https://chromium-review.googlesource.com/760242
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Commit-Queue: Ryo Hashimoto <hashimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522032}
[modify] https://crrev.com/edaf7f063eab7315cb215f0717be59fbfa8c4d69/components/drive/job_scheduler.cc
[modify] https://crrev.com/edaf7f063eab7315cb215f0717be59fbfa8c4d69/components/drive/job_scheduler.h

Status: Fixed (was: Available)

Sign in to add a comment