New issue
Advanced search Search tips

Issue 780474 link

Starred by 2 users

Issue metadata

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

Blocking:
issue 754695



Sign in to add a comment

Migrate components/offline_pages/core/background/connection_notifier.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/offline_pages/core/background/connection_notifier.h to use NetworkChangeObserver instead. 

See  Issue 754695  for the motivation.

 
I would like to work on this.
Thanks for the patch! I've kicked off a trybot dry run on your CL. Once everything looks good, you can select an owner from components/offline_pages/OWNERS and start the review process by clicking the "Start Review" button in gerrit.

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

Components: Internals>Network>Service

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

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.
xunjieli@, is this a mechanical change or are there behavior changes in the new API?
This particular one seems to be purely mechanical. 
In some cases, the observer implementation needs to be changed slightly (see last section on https://docs.google.com/document/d/1ch22_dRHPQJ9QArFMmWl_UyVpl-EDXA0PdljGh2kDF0/edit#heading=h.mlge0axnxo7)

In short, because offline_pages's ConnectionNotifier is a "constructive" observer, it needs to check type != CONNECTION_NONE. In this case, it already does so, so no additional changes needed besides the mechanical ones.

Project Member

Comment 9 by bugdroid1@chromium.org, Nov 9 2017

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

commit 8b5991731783550c806e178188f307e04235719d
Author: mallikarjun82 <vm.arjun@samsung.com>
Date: Thu Nov 09 08:23:59 2017

Change ConnectionTypeObserver to NetworkChangeObserver

This CL migrates connection_notifier.h to use NetworkChangeObserver.
More details are in the following bug

Bug:  780474 
Change-Id: Ib00250222ed704646fe0150b1aac420a8dd34e4d
Reviewed-on: https://chromium-review.googlesource.com/754245
Commit-Queue: srirama chandra sekhar <srirama.m@samsung.com>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515122}
[modify] https://crrev.com/8b5991731783550c806e178188f307e04235719d/AUTHORS
[modify] https://crrev.com/8b5991731783550c806e178188f307e04235719d/components/offline_pages/core/background/connection_notifier.cc
[modify] https://crrev.com/8b5991731783550c806e178188f307e04235719d/components/offline_pages/core/background/connection_notifier.h
[modify] https://crrev.com/8b5991731783550c806e178188f307e04235719d/components/offline_pages/core/background/request_coordinator_unittest.cc

The patch is merged. 
Can some one help to resolve this bug?
Status: Fixed (was: Available)

Sign in to add a comment