[NetInfo] Remove NetworkStateNotifier::setOnLine (issue 2546763004 by jkarlin@chromium.org)

[jka...@chromium.org]

Reviewers: jochen
CL: https://codereview.chromium.org/2546763004/

Message:
jochen@ PTAL at everything, thanks!

Description:
[NetInfo] Remove NetworkStateNotifier::setOnLine

The functionality of NetworkStateNotifier::setOnLine can be replaced by
NetworkStateNotifier::setWebConnection, which simplifies the
NetworkStateNotifier.

BUG=564732

Affected files (+11, -28 lines):
M content/renderer/render_thread_impl.cc
M third_party/WebKit/Source/core/page/NetworkStateNotifier.h
M third_party/WebKit/Source/core/page/NetworkStateNotifier.cpp
M third_party/WebKit/Source/web/WebNetworkStateNotifier.cpp
M third_party/WebKit/public/web/WebNetworkStateNotifier.h


Index: content/renderer/render_thread_impl.cc
diff --git a/content/renderer/render_thread_impl.cc b/content/renderer/render_thread_impl.cc
index 8616dbdfd1c22b389660eb92cf4314797a34a306..3976ad8b2e315e10c7eada8f1fea4728589a142c 100644
--- a/content/renderer/render_thread_impl.cc
+++ b/content/renderer/render_thread_impl.cc
@@ -2196,7 +2196,6 @@ void RenderThreadImpl::OnNetworkConnectionChanged(
net::NetworkChangeNotifier::ConnectionType type,
double max_bandwidth_mbps) {
bool online = type != net::NetworkChangeNotifier::CONNECTION_NONE;
- WebNetworkStateNotifier::setOnLine(online);
for (auto& observer : observers_)
observer.NetworkStateChanged(online);
WebNetworkStateNotifier::setWebConnection(
Index: third_party/WebKit/Source/core/page/NetworkStateNotifier.cpp
diff --git a/third_party/WebKit/Source/core/page/NetworkStateNotifier.cpp b/third_party/WebKit/Source/core/page/NetworkStateNotifier.cpp
index 26489b031e368d6adf6cea6918453f475386c821..3a5d70def5d3d2bf5885d4322d675cc2b38a0dd7 100644
--- a/third_party/WebKit/Source/core/page/NetworkStateNotifier.cpp
+++ b/third_party/WebKit/Source/core/page/NetworkStateNotifier.cpp
@@ -54,33 +54,25 @@ NetworkStateNotifier::ScopedNotifier::~ScopedNotifier() {
DCHECK(isMainThread());
const NetworkState& after =
m_notifier.m_hasOverride ? m_notifier.m_override : m_notifier.m_state;
+ if (!m_before.initialized)
+ return;
if ((after.type != m_before.type ||
- after.maxBandwidthMbps != m_before.maxBandwidthMbps) &&
- m_before.connectionInitialized)
+ after.maxBandwidthMbps != m_before.maxBandwidthMbps))
m_notifier.notifyObservers(after.type, after.maxBandwidthMbps);
- if (after.onLine != m_before.onLine && m_before.onLineInitialized)
+ if (after.onLine != m_before.onLine)
Page::networkStateChanged(after.onLine);
}

-void NetworkStateNotifier::setOnLine(bool onLine) {
- DCHECK(isMainThread());
- ScopedNotifier notifier(*this);
- {
- MutexLocker locker(m_mutex);
- m_state.onLineInitialized = true;
- m_state.onLine = onLine;
- }
-}
-
void NetworkStateNotifier::setWebConnection(WebConnectionType type,
double maxBandwidthMbps) {
DCHECK(isMainThread());
ScopedNotifier notifier(*this);
{
MutexLocker locker(m_mutex);
- m_state.connectionInitialized = true;
+ m_state.initialized = true;
m_state.type = type;
m_state.maxBandwidthMbps = maxBandwidthMbps;
+ m_state.onLine = type != WebConnectionType::WebConnectionTypeNone;
}
}

@@ -126,9 +118,8 @@ void NetworkStateNotifier::setOverride(bool onLine,
{
MutexLocker locker(m_mutex);
m_hasOverride = true;
- m_override.onLineInitialized = true;
+ m_override.initialized = true;
m_override.onLine = onLine;
- m_override.connectionInitialized = true;
m_override.type = type;
m_override.maxBandwidthMbps = maxBandwidthMbps;
}
Index: third_party/WebKit/Source/core/page/NetworkStateNotifier.h
diff --git a/third_party/WebKit/Source/core/page/NetworkStateNotifier.h b/third_party/WebKit/Source/core/page/NetworkStateNotifier.h
index dcb0abfd0a010fdaaa1160ce51781f4f2504b663..439e37ff021383b5996bb65f7a81c2acd1ca934e 100644
--- a/third_party/WebKit/Source/core/page/NetworkStateNotifier.h
+++ b/third_party/WebKit/Source/core/page/NetworkStateNotifier.h
@@ -56,17 +56,16 @@ class CORE_EXPORT NetworkStateNotifier {
bool onLine() const {
MutexLocker locker(m_mutex);
const NetworkState& state = m_hasOverride ? m_override : m_state;
- DCHECK(state.onLineInitialized);
+ DCHECK(state.initialized);
return state.onLine;
}

- void setOnLine(bool);

// Can be called on any thread.
WebConnectionType connectionType() const {
MutexLocker locker(m_mutex);
const NetworkState& state = m_hasOverride ? m_override : m_state;
- DCHECK(state.connectionInitialized);
+ DCHECK(state.initialized);
return state.type;
}

@@ -94,7 +93,7 @@ class CORE_EXPORT NetworkStateNotifier {
double maxBandwidth() const {
MutexLocker locker(m_mutex);
const NetworkState& state = m_hasOverride ? m_override : m_state;
- DCHECK(state.connectionInitialized);
+ DCHECK(state.initialized);
return state.maxBandwidthMbps;
}

@@ -127,9 +126,8 @@ class CORE_EXPORT NetworkStateNotifier {

struct NetworkState {
static const int kInvalidMaxBandwidth = -1;
- bool onLineInitialized = false;
+ bool initialized = false;
bool onLine = true;
- bool connectionInitialized = false;
WebConnectionType type = WebConnectionTypeOther;
double maxBandwidthMbps = kInvalidMaxBandwidth;
};
Index: third_party/WebKit/Source/web/WebNetworkStateNotifier.cpp
diff --git a/third_party/WebKit/Source/web/WebNetworkStateNotifier.cpp b/third_party/WebKit/Source/web/WebNetworkStateNotifier.cpp
index fce72d48378a759ac02879b8cdaa2a2086514581..00d1c58b1716012b84e35c080a082d50e53f88a2 100644
--- a/third_party/WebKit/Source/web/WebNetworkStateNotifier.cpp
+++ b/third_party/WebKit/Source/web/WebNetworkStateNotifier.cpp
@@ -34,10 +34,6 @@

namespace blink {

-void WebNetworkStateNotifier::setOnLine(bool onLine) {
- networkStateNotifier().setOnLine(onLine);
-}
-
void WebNetworkStateNotifier::setWebConnection(WebConnectionType type,
double maxBandwidthMbps) {
networkStateNotifier().setWebConnection(type, maxBandwidthMbps);
Index: third_party/WebKit/public/web/WebNetworkStateNotifier.h
diff --git a/third_party/WebKit/public/web/WebNetworkStateNotifier.h b/third_party/WebKit/public/web/WebNetworkStateNotifier.h
index 1853d25189533e30c712f8737f09d70a83dbee67..68ce0dc1b8a2e47142cce2a3076d90f981ca0e85 100644
--- a/third_party/WebKit/public/web/WebNetworkStateNotifier.h
+++ b/third_party/WebKit/public/web/WebNetworkStateNotifier.h
@@ -38,7 +38,6 @@ namespace blink {

class WebNetworkStateNotifier {
public:
- BLINK_EXPORT static void setOnLine(bool);
BLINK_EXPORT static void setWebConnection(WebConnectionType,
double maxBandwidthMbps);

[jka...@chromium.org]

tvolodin@ PTAL at android_webview/ Thanks!

https://codereview.chromium.org/2546763004/

[joc...@chromium.org]

lgtm



https://codereview.chromium.org/2546763004/

[timvo...@chromium.org]

corrected my email

https://codereview.chromium.org/2546763004/

[timvo...@chromium.org]

https://codereview.chromium.org/2546763004/diff/40001/android_webview/renderer/aw_render_thread_observer.cc
File android_webview/renderer/aw_render_thread_observer.cc (right):

https://codereview.chromium.org/2546763004/diff/40001/android_webview/renderer/aw_render_thread_observer.cc#newcode39
android_webview/renderer/aw_render_thread_observer.cc:39: // If this
method is being called on WebView then NetInfo isn't supported,
webview can still call setNetworkAvailable() even if netinfo is enabled
(and listening), in which case I think we stop listening (IIRC)

https://codereview.chromium.org/2546763004/diff/40001/android_webview/renderer/aw_render_thread_observer.cc#newcode47
android_webview/renderer/aw_render_thread_observer.cc:47:
blink::WebNetworkStateNotifier::setWebConnection(
should the behavior be the same here as before for consistency? actually
not sure if this will pass android CTS tests (?)

https://codereview.chromium.org/2546763004/