Remove unnecessary calls to AddWakeOnPaketConnection and RemoveWakeOnPacketConnection

Remove unnecessary calls to AddWakeOnPaketConnection and RemoveWakeOnPacketConnection

    - Adds checks around shill method calls to AddWifiWakeOnPacketConnection and
      RemoveWifiWakeOnPacketConnection.
    - Renames WakeOnPacketConnectionObserver to WakeOnWifiConnectionObserver and moves its dec & imp to a separate file.
    - Implements unittests for WakeOnWifiConnectionObserver.
    - Implements MockNetworkDeviceHandler.

BUG=548298
COMPONENT: ChromeOS - WakeOnWifiManager, NetworkDeviceHandler.

Committed: https://crrev.com/77a1ea0183f82da064cb487b0a01ba5c7a95df8c
Cr-Commit-Position: refs/heads/master@{#388833}

[malaykeshav, 2016-04-13 23:40:31]

Description was changed from

==========
Syntax Fix


Pass NetworkDeviceHandler to Constructor


Remove unnecessary calls to AddWakeOnPaketConnection and
RemoveWakeOnPacketConnection.     - Adds checks around shill method calls to
AddWifiWakeOnPacketConnection and RemoveWifiWakeOnPacketConnection.     -
Renames WakeOnPacketConnectionObserver to ConnectionObserver.     - Implements
unittests for ConnectionObserver.     - Creates an implementation class for
NetworkHandler.     - Implements MockNetworkDeviceHandler.

BUG=548298
COMPONENT: ChromeOS - WakeOnWifiManager, NetworkDeviceHandler, Network
Handler
==========

to

==========
Remove unnecessary calls to AddWakeOnPaketConnection and
RemoveWakeOnPacketConnection
    
    - Adds checks around shill method calls to AddWifiWakeOnPacketConnection and
      RemoveWifiWakeOnPacketConnection.
    - Renames WakeOnPacketConnectionObserver to ConnectionObserver.
    - Implements unittests for ConnectionObserver.

BUG=548298
COMPONENT: ChromeOS - WakeOnWifiManager, NetworkDeviceHandler.
==========

[malaykeshav, 2016-04-13 23:47:07]

Patchset #1 (id:1) has been deleted

[malaykeshav, 2016-04-13 23:49:27]

Description was changed from

==========
Remove unnecessary calls to AddWakeOnPaketConnection and
RemoveWakeOnPacketConnection
    
    - Adds checks around shill method calls to AddWifiWakeOnPacketConnection and
      RemoveWifiWakeOnPacketConnection.
    - Renames WakeOnPacketConnectionObserver to ConnectionObserver.
    - Implements unittests for ConnectionObserver.

BUG=548298
COMPONENT: ChromeOS - WakeOnWifiManager, NetworkDeviceHandler.
==========

to

==========
Remove unnecessary calls to AddWakeOnPaketConnection and
RemoveWakeOnPacketConnection
    
    - Adds checks around shill method calls to AddWifiWakeOnPacketConnection and
      RemoveWifiWakeOnPacketConnection.
    - Renames WakeOnPacketConnectionObserver to ConnectionObserver.
    - Implements unittests for ConnectionObserver.
    - Implements MockNetworkDeviceHandler.

BUG=548298
COMPONENT: ChromeOS - WakeOnWifiManager, NetworkDeviceHandler.
==========

[malaykeshav, 2016-04-13 23:49:27]

malaykeshav@chromium.org changed reviewers:
+ stevenjb@chromium.org

[malaykeshav, 2016-04-13 23:49:32]

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

[commit-bot: I haz the power, 2016-04-13 23:49:52]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1891633002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1891633002/20001

[commit-bot: I haz the power, 2016-04-14 01:00:30]

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

[commit-bot: I haz the power, 2016-04-14 01:00:31]

Dry run: This issue passed the CQ dry run.

[malaykeshav, 2016-04-19 23:29:25]

On 2016/04/14 at 01:00:31, commit-bot wrote:
> Dry run: This issue passed the CQ dry run.

Ping

[stevenjb, 2016-04-19 23:46:15]

Sorry, thought I'd reviewed this but must not have published my comments.

Just one change request.

https://codereview.chromium.org/1891633002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/net/wake_on_wifi_manager.h (right):

https://codereview.chromium.org/1891633002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/net/wake_on_wifi_manager.h:14: #include
"chromeos/network/network_device_handler.h"
Forward declare instead

https://codereview.chromium.org/1891633002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/net/wake_on_wifi_manager.h:135: };
Since we need to expose this in the header for tests, we should move this and
its implementation to separate files. That will also reduce the header
dependencies we need to add to this header.

[malaykeshav, 2016-04-20 19:48:33]

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

[malaykeshav, 2016-04-20 19:48:42]

https://codereview.chromium.org/1891633002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/net/wake_on_wifi_manager.h (right):

https://codereview.chromium.org/1891633002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/net/wake_on_wifi_manager.h:14: #include
"chromeos/network/network_device_handler.h"
On 2016/04/19 at 23:46:15, stevenjb wrote:
> Forward declare instead

Done

https://codereview.chromium.org/1891633002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/net/wake_on_wifi_manager.h:135: };
On 2016/04/19 at 23:46:15, stevenjb wrote:
> Since we need to expose this in the header for tests, we should move this and
its implementation to separate files. That will also reduce the header
dependencies we need to add to this header.

Done

[commit-bot: I haz the power, 2016-04-20 19:49:03]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1891633002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1891633002/40001

[malaykeshav, 2016-04-20 19:49:59]

Description was changed from

==========
Remove unnecessary calls to AddWakeOnPaketConnection and
RemoveWakeOnPacketConnection
    
    - Adds checks around shill method calls to AddWifiWakeOnPacketConnection and
      RemoveWifiWakeOnPacketConnection.
    - Renames WakeOnPacketConnectionObserver to ConnectionObserver.
    - Implements unittests for ConnectionObserver.
    - Implements MockNetworkDeviceHandler.

BUG=548298
COMPONENT: ChromeOS - WakeOnWifiManager, NetworkDeviceHandler.
==========

to

==========
Remove unnecessary calls to AddWakeOnPaketConnection and
RemoveWakeOnPacketConnection
    
    - Adds checks around shill method calls to AddWifiWakeOnPacketConnection and
      RemoveWifiWakeOnPacketConnection.
    - Renames WakeOnPacketConnectionObserver to WakeOnWifiConnectionObserver and
moves its dec & imp to a separate file.
    - Implements unittests for WakeOnWifiConnectionObserver.
    - Implements MockNetworkDeviceHandler.

BUG=548298
COMPONENT: ChromeOS - WakeOnWifiManager, NetworkDeviceHandler.
==========

[stevenjb, 2016-04-20 20:16:28]

https://codereview.chromium.org/1891633002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/net/wake_on_wifi_connection_observer.h (right):

https://codereview.chromium.org/1891633002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/net/wake_on_wifi_connection_observer.h:23: // of this
class.
The last sentence doesn't really apply to this class. You could say 'Each
instance of this class is associated with a profile', but that really isn't
necessary, I would just remove the sentence.

https://codereview.chromium.org/1891633002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/net/wake_on_wifi_connection_observer.h:28: void
HandleWifiDevicePropertiesReady();
public methods should be documented (except trivial setters/getters)

https://codereview.chromium.org/1891633002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/net/wake_on_wifi_manager.cc (right):

https://codereview.chromium.org/1891633002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/net/wake_on_wifi_manager.cc:78: }
If this is just used in wake_on_wifi_connection_observer.cc, make it a file
local function there.

https://codereview.chromium.org/1891633002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/net/wake_on_wifi_manager.h (right):

https://codereview.chromium.org/1891633002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/net/wake_on_wifi_manager.h:17: #include
"net/base/ip_endpoint.h"
No longer needed

https://codereview.chromium.org/1891633002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/net/wake_on_wifi_manager.h:28: class
NetworkDeviceHandler;
Both of these should be forward declared in wake_on_wifi_connection_observer.h,
not here.

[commit-bot: I haz the power, 2016-04-20 21:51:57]

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

[commit-bot: I haz the power, 2016-04-20 21:51:58]

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

[malaykeshav, 2016-04-20 22:16:00]

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

[commit-bot: I haz the power, 2016-04-20 22:16:54]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1891633002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1891633002/60001

[malaykeshav, 2016-04-20 23:14:56]

https://codereview.chromium.org/1891633002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/net/wake_on_wifi_connection_observer.h (right):

https://codereview.chromium.org/1891633002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/net/wake_on_wifi_connection_observer.h:23: // of this
class.
On 2016/04/20 at 20:16:28, stevenjb wrote:
> The last sentence doesn't really apply to this class. You could say 'Each
instance of this class is associated with a profile', but that really isn't
necessary, I would just remove the sentence.

done

https://codereview.chromium.org/1891633002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/net/wake_on_wifi_connection_observer.h:28: void
HandleWifiDevicePropertiesReady();
On 2016/04/20 at 20:16:28, stevenjb wrote:
> public methods should be documented (except trivial setters/getters)

done

https://codereview.chromium.org/1891633002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/net/wake_on_wifi_manager.cc (right):

https://codereview.chromium.org/1891633002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/net/wake_on_wifi_manager.cc:78: }
On 2016/04/20 at 20:16:28, stevenjb wrote:
> If this is just used in wake_on_wifi_connection_observer.cc, make it a file
local function there.

Its being used by both wake_on_wifi_manager and *_connection_observer, hence
made it a private static.

https://codereview.chromium.org/1891633002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/net/wake_on_wifi_manager.h (right):

https://codereview.chromium.org/1891633002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/net/wake_on_wifi_manager.h:17: #include
"net/base/ip_endpoint.h"
On 2016/04/20 at 20:16:28, stevenjb wrote:
> No longer needed

Done

https://codereview.chromium.org/1891633002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/net/wake_on_wifi_manager.h:28: class
NetworkDeviceHandler;
On 2016/04/20 at 20:16:28, stevenjb wrote:
> Both of these should be forward declared in
wake_on_wifi_connection_observer.h, not here.

Done

[stevenjb, 2016-04-20 23:33:05]

lgtm

[commit-bot: I haz the power, 2016-04-21 00:07:34]

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

[commit-bot: I haz the power, 2016-04-21 00:07:35]

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

[malaykeshav, 2016-04-21 00:08:01]

The CQ bit was checked by malaykeshav@chromium.org

[commit-bot: I haz the power, 2016-04-21 00:08:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1891633002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1891633002/60001

[commit-bot: I haz the power, 2016-04-21 02:34:28]

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

[commit-bot: I haz the power, 2016-04-21 02:34:29]

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

[malaykeshav, 2016-04-21 17:47:02]

The CQ bit was checked by malaykeshav@chromium.org

[commit-bot: I haz the power, 2016-04-21 17:48:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1891633002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1891633002/60001

[commit-bot: I haz the power, 2016-04-21 18:56:02]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-04-22 19:37:00]

Description was changed from

==========
Remove unnecessary calls to AddWakeOnPaketConnection and
RemoveWakeOnPacketConnection
    
    - Adds checks around shill method calls to AddWifiWakeOnPacketConnection and
      RemoveWifiWakeOnPacketConnection.
    - Renames WakeOnPacketConnectionObserver to WakeOnWifiConnectionObserver and
moves its dec & imp to a separate file.
    - Implements unittests for WakeOnWifiConnectionObserver.
    - Implements MockNetworkDeviceHandler.

BUG=548298
COMPONENT: ChromeOS - WakeOnWifiManager, NetworkDeviceHandler.
==========

to

==========
Remove unnecessary calls to AddWakeOnPaketConnection and
RemoveWakeOnPacketConnection

    - Adds checks around shill method calls to AddWifiWakeOnPacketConnection and
      RemoveWifiWakeOnPacketConnection.
    - Renames WakeOnPacketConnectionObserver to WakeOnWifiConnectionObserver and
moves its dec & imp to a separate file.
    - Implements unittests for WakeOnWifiConnectionObserver.
    - Implements MockNetworkDeviceHandler.

BUG=548298
COMPONENT: ChromeOS - WakeOnWifiManager, NetworkDeviceHandler.

Committed: https://crrev.com/77a1ea0183f82da064cb487b0a01ba5c7a95df8c
Cr-Commit-Position: refs/heads/master@{#388833}
==========

[commit-bot: I haz the power, 2016-04-22 19:37:01]

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