Added event to networkingPrivate extension that portal detection is completed.

Added event to networkingPrivate extension that portal detection is completed.

BUG=354946
TEST=ExtensionNetworkingPrivateApiTest.*
R=yoz@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=262736

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=262855

[Yoyo Zhou, 2014-04-04 19:28:41]

https://chromiumcodereview.appspot.com/225413005/diff/1/chrome/browser/chrome...
File chrome/browser/chromeos/net/network_portal_detector_test_impl.cc (right):

https://chromiumcodereview.appspot.com/225413005/diff/1/chrome/browser/chrome...
chrome/browser/chromeos/net/network_portal_detector_test_impl.cc:42: void
NetworkPortalDetectorTestImpl::AlwaysFireObserverForTesting() {
I'm not sure what the purpose of this is. Can it be folded into
NotifyObserersForTesting above?

https://chromiumcodereview.appspot.com/225413005/diff/1/chrome/browser/chrome...
File chrome/browser/chromeos/net/network_portal_detector_test_impl.h (right):

https://chromiumcodereview.appspot.com/225413005/diff/1/chrome/browser/chrome...
chrome/browser/chromeos/net/network_portal_detector_test_impl.h:48: bool
always_fire_observer_;
This should be documented.

[ygorshenin1, 2014-04-07 09:30:15]

https://chromiumcodereview.appspot.com/225413005/diff/1/chrome/browser/chrome...
File chrome/browser/chromeos/net/network_portal_detector_test_impl.cc (right):

https://chromiumcodereview.appspot.com/225413005/diff/1/chrome/browser/chrome...
chrome/browser/chromeos/net/network_portal_detector_test_impl.cc:42: void
NetworkPortalDetectorTestImpl::AlwaysFireObserverForTesting() {
The purpose of AlwaysFireObserverForTesting() is to fire listeners just after
chrome.networkingPrivate.onPortalDetectionCompleted.addListener is called. Thus,
following happens during execution of JS test:
1. AlwaysFireObserverForTesting() is called.
2. JS test starts.
3. chrome.networkingPrivate.onPortalDetectionCompleted.addListener is called, as
a result an observer is added
to NetworkPortalDetectorTestImpl.
4. Observer is fired, JS side is notified about completion of (fake) portal
detection.
5. JS test ends.

Thus, this method is used only for networkingPrivate JS test. I've tried other
approaches, but for me it looks impossible to call explicitly
NotifyObserversForTesting() during execution of JS test. What do you think?

On 2014/04/04 19:28:41, Yoyo Zhou wrote:
> I'm not sure what the purpose of this is. Can it be folded into
> NotifyObserersForTesting above?

[ygorshenin1, 2014-04-07 09:34:16]

https://chromiumcodereview.appspot.com/225413005/diff/1/chrome/browser/chrome...
File chrome/browser/chromeos/net/network_portal_detector_test_impl.h (right):

https://chromiumcodereview.appspot.com/225413005/diff/1/chrome/browser/chrome...
chrome/browser/chromeos/net/network_portal_detector_test_impl.h:48: bool
always_fire_observer_;
On 2014/04/04 19:28:41, Yoyo Zhou wrote:
> This should be documented.

Done.

[Yoyo Zhou, 2014-04-07 23:26:20]

chrome.test.sendMessage is a good way to do this, I think. You set up a test
message listener in the C++ code and wait for it to receive a message from the
JS.

Here's one example where we wait for a message, and then open an incognito tab
from the browser:
https://code.google.com/p/chromium/codesearch#search/&q=createIncognitoTab&sq...

[ygorshenin1, 2014-04-08 08:09:15]

Thanks for good advice! PTAL

[Yoyo Zhou, 2014-04-08 17:57:26]

Great, LGTM

[ygorshenin1, 2014-04-08 18:04:20]

The CQ bit was checked by ygorshenin@chromium.org

[commit-bot: I haz the power, 2014-04-08 18:05:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ygorshenin@chromium.org/225413005/30001

[commit-bot: I haz the power, 2014-04-08 20:19:24]

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

[commit-bot: I haz the power, 2014-04-08 20:19:24]

Retried try job too often on win_x64_rel for step(s) base_unittests,
chrome_elf_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...

[ygorshenin1, 2014-04-09 10:11:32]

The CQ bit was checked by ygorshenin@chromium.org

[commit-bot: I haz the power, 2014-04-09 10:11:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ygorshenin@chromium.org/225413005/30001

[commit-bot: I haz the power, 2014-04-09 11:17:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ygorshenin@chromium.org/225413005/30001

[commit-bot: I haz the power, 2014-04-09 11:27:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ygorshenin@chromium.org/225413005/30001

[commit-bot: I haz the power, 2014-04-09 12:01:32]

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

[commit-bot: I haz the power, 2014-04-09 12:01:33]

Try jobs failed on following builders:
  tryserver.chromium on mac_chromium_rel

[ygorshenin1, 2014-04-09 12:26:54]

The CQ bit was checked by ygorshenin@chromium.org

[commit-bot: I haz the power, 2014-04-09 12:27:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ygorshenin@chromium.org/225413005/30001

[commit-bot: I haz the power, 2014-04-09 13:34:42]

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

[commit-bot: I haz the power, 2014-04-09 13:34:43]

Try jobs failed on following builders:
  tryserver.chromium on mac_chromium_compile_dbg

[ygorshenin1, 2014-04-09 14:16:03]

The CQ bit was checked by ygorshenin@chromium.org

[commit-bot: I haz the power, 2014-04-09 14:16:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ygorshenin@chromium.org/225413005/30001

[commit-bot: I haz the power, 2014-04-09 15:00:11]

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

[commit-bot: I haz the power, 2014-04-09 15:00:11]

Try jobs failed on following builders:
  tryserver.chromium on win_chromium_compile_dbg

[ygorshenin1, 2014-04-09 17:47:55]

Committed patchset #3 manually as r262736 (presubmit successful).

[tapted, 2014-04-09 18:00:08]

A revert of this CL has been created in
https://codereview.chromium.org/231173005/ by tapted@chromium.org.

The reason for reverting is: Fails compile on Chromium Mac.

errors like

../../chrome/browser/extensions/api/networking_private/networking_private_apitest.cc:139:3:error:
expected '{' or ','
  bool RunNetworkingSubtest(const std::string& subtest) {

link:
http://build.chromium.org/p/chromium.mac/builders/Mac%20Builder/builds/1840/.

[ygorshenin1, 2014-04-09 19:34:32]

The CQ bit was checked by ygorshenin@chromium.org

[commit-bot: I haz the power, 2014-04-09 19:34:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ygorshenin@chromium.org/225413005/50001

[commit-bot: I haz the power, 2014-04-09 23:36:30]

Change committed as 262855