[HttpNetworkSession] Flush socket pools on low memory notification.

[HttpNetworkSession] Flush socket pools on low memory notification.

This cl aims to reduce the number of times chrome is killed
when there is not enough memory left (it mostly concerns
Android).

When there is a low memory notification sent by the memory
pressure monitor (either MEMORY_PRESSURE_LEVEL_MODERATE or
MEMORY_PRESSURE_LEVEL_CRITICAL), OnMemoryPressure callback
is called and idle sockets are flushed.

At first I thought to close idle sockets on moderate
notifications and use CloseAllConnections() on critical
notifications, but after some tests decided to flush idle
sockets only, because CloseAllConnections() terminates a
transaction and causes failure.

+ added unit tests.

BUG=617719
Committed: https://crrev.com/0adf859e4a1312c56d324503a59959fd85dc8c44
Cr-Commit-Position: refs/heads/master@{#405708}

[maksims (do not use this acc), 2016-07-11 07:29:25]

Description was changed from

==========
flush sockets

BUG=
==========

to

==========
[HttpNetworkSession] Flush socket pools on low memory notification.

This cl aims to reduce the number of times chrome is killed
when there is not enough memory left (it mostly concerns
Android).

When there is a low memory notification sent by the memory
pressure monitor (either MEMORY_PRESSURE_LEVEL_MODERATE or
MEMORY_PRESSURE_LEVEL_CRITICAL), OnMemoryPressure callback
is called and idle sockets are flushed.

At first I thought to close idle sockets on moderate
notifications and use CloseAllConnections() on critical
notifications, but after some tests decided to flush idle
sockets only, because CloseAllConnections() terminates a
transaction and causes failure.

BUG=617719
==========

[maksims (do not use this acc), 2016-07-11 07:29:25]

maksim.sisov@intel.com changed reviewers:
+ asanka@chromium.org, davidben@chromium.org, mmenke@chromium.org

[maksims (do not use this acc), 2016-07-11 11:24:48]

Description was changed from

==========
[HttpNetworkSession] Flush socket pools on low memory notification.

This cl aims to reduce the number of times chrome is killed
when there is not enough memory left (it mostly concerns
Android).

When there is a low memory notification sent by the memory
pressure monitor (either MEMORY_PRESSURE_LEVEL_MODERATE or
MEMORY_PRESSURE_LEVEL_CRITICAL), OnMemoryPressure callback
is called and idle sockets are flushed.

At first I thought to close idle sockets on moderate
notifications and use CloseAllConnections() on critical
notifications, but after some tests decided to flush idle
sockets only, because CloseAllConnections() terminates a
transaction and causes failure.

BUG=617719
==========

to

==========
[HttpNetworkSession] Flush socket pools on low memory notification.

This cl aims to reduce the number of times chrome is killed
when there is not enough memory left (it mostly concerns
Android).

When there is a low memory notification sent by the memory
pressure monitor (either MEMORY_PRESSURE_LEVEL_MODERATE or
MEMORY_PRESSURE_LEVEL_CRITICAL), OnMemoryPressure callback
is called and idle sockets are flushed.

At first I thought to close idle sockets on moderate
notifications and use CloseAllConnections() on critical
notifications, but after some tests decided to flush idle
sockets only, because CloseAllConnections() terminates a
transaction and causes failure.

+ added unit tests.

BUG=617719
==========

[maksims (do not use this acc), 2016-07-11 11:52:33]

Would somebody take a look into that, please?

[mmenke, 2016-07-12 18:03:17]

https://codereview.chromium.org/2132313002/diff/20001/net/http/http_network_t...
File net/http/http_network_transaction_unittest.cc (right):

https://codereview.chromium.org/2132313002/diff/20001/net/http/http_network_t...
net/http/http_network_transaction_unittest.cc:6446: EXPECT_EQ(0,
GetIdleSocketCountInTransportSocketPool(session.get()));
Should be checking the SSL socket pool, not the transport one.

https://codereview.chromium.org/2132313002/diff/20001/net/http/http_network_t...
net/http/http_network_transaction_unittest.cc:6456:
EXPECT_THAT(callback.WaitForResult(), IsOk());
It seems weird to have these lines below the memory pressure notification.  It
doesn't really matter what order things occur in here, but seems like we want
the test to be more deterministic, and we want to be sure we actually create a
socket before we get the memory pressure notification.

So I suggest moving these lines before the notification, along with the
response->headers check just below this, and the idle socket count check.

You can also merge the lines to be:

EXPECT_THAT(callback.GetResult(rv), IsOk());

https://codereview.chromium.org/2132313002/diff/20001/net/http/http_network_t...
net/http/http_network_transaction_unittest.cc:6463: EXPECT_EQ(0,
GetIdleSocketCountInTransportSocketPool(session.get()));
GetIdleSocketCountInSSLSocketPool

https://codereview.chromium.org/2132313002/diff/20001/net/http/http_network_t...
net/http/http_network_transaction_unittest.cc:6487:
TEST_P(HttpNetworkTransactionTest, FlushSocketPoolOnLowMemoryNotifications) {
This is the more basic test, so should be before the other one.

[maksims (do not use this acc), 2016-07-13 07:45:53]

https://codereview.chromium.org/2132313002/diff/20001/net/http/http_network_t...
File net/http/http_network_transaction_unittest.cc (right):

https://codereview.chromium.org/2132313002/diff/20001/net/http/http_network_t...
net/http/http_network_transaction_unittest.cc:6446: EXPECT_EQ(0,
GetIdleSocketCountInTransportSocketPool(session.get()));
On 2016/07/12 18:03:17, mmenke wrote:
> Should be checking the SSL socket pool, not the transport one.

Done.

https://codereview.chromium.org/2132313002/diff/20001/net/http/http_network_t...
net/http/http_network_transaction_unittest.cc:6456:
EXPECT_THAT(callback.WaitForResult(), IsOk());
On 2016/07/12 18:03:17, mmenke wrote:
> It seems weird to have these lines below the memory pressure notification.  It
> doesn't really matter what order things occur in here, but seems like we want
> the test to be more deterministic, and we want to be sure we actually create a
> socket before we get the memory pressure notification.
> 
> So I suggest moving these lines before the notification, along with the
> response->headers check just below this, and the idle socket count check.
> 
> You can also merge the lines to be:
> 
> EXPECT_THAT(callback.GetResult(rv), IsOk());

Done.

https://codereview.chromium.org/2132313002/diff/20001/net/http/http_network_t...
net/http/http_network_transaction_unittest.cc:6463: EXPECT_EQ(0,
GetIdleSocketCountInTransportSocketPool(session.get()));
On 2016/07/12 18:03:17, mmenke wrote:
> GetIdleSocketCountInSSLSocketPool

Done.

https://codereview.chromium.org/2132313002/diff/20001/net/http/http_network_t...
net/http/http_network_transaction_unittest.cc:6487:
TEST_P(HttpNetworkTransactionTest, FlushSocketPoolOnLowMemoryNotifications) {
On 2016/07/12 18:03:17, mmenke wrote:
> This is the more basic test, so should be before the other one.

Done.

[maksims (do not use this acc), 2016-07-13 07:57:03]

Patchset #3 (id:40001) has been deleted

[maksims (do not use this acc), 2016-07-13 11:14:58]

The CQ bit was checked by maksim.sisov@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-07-13 11:15:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-13 12:22:16]

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

[commit-bot: I haz the power, 2016-07-13 12:22:17]

Dry run: This issue passed the CQ dry run.

[mmenke, 2016-07-14 19:06:50]

LGTM!

[maksims (do not use this acc), 2016-07-15 05:17:43]

The CQ bit was checked by maksim.sisov@intel.com

[commit-bot: I haz the power, 2016-07-15 05:17:58]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-15 06:26:11]

Description was changed from

==========
[HttpNetworkSession] Flush socket pools on low memory notification.

This cl aims to reduce the number of times chrome is killed
when there is not enough memory left (it mostly concerns
Android).

When there is a low memory notification sent by the memory
pressure monitor (either MEMORY_PRESSURE_LEVEL_MODERATE or
MEMORY_PRESSURE_LEVEL_CRITICAL), OnMemoryPressure callback
is called and idle sockets are flushed.

At first I thought to close idle sockets on moderate
notifications and use CloseAllConnections() on critical
notifications, but after some tests decided to flush idle
sockets only, because CloseAllConnections() terminates a
transaction and causes failure.

+ added unit tests.

BUG=617719
==========

to

==========
[HttpNetworkSession] Flush socket pools on low memory notification.

This cl aims to reduce the number of times chrome is killed
when there is not enough memory left (it mostly concerns
Android).

When there is a low memory notification sent by the memory
pressure monitor (either MEMORY_PRESSURE_LEVEL_MODERATE or
MEMORY_PRESSURE_LEVEL_CRITICAL), OnMemoryPressure callback
is called and idle sockets are flushed.

At first I thought to close idle sockets on moderate
notifications and use CloseAllConnections() on critical
notifications, but after some tests decided to flush idle
sockets only, because CloseAllConnections() terminates a
transaction and causes failure.

+ added unit tests.

BUG=617719
==========

[commit-bot: I haz the power, 2016-07-15 06:26:12]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-07-15 06:26:19]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-15 06:27:51]

Description was changed from

==========
[HttpNetworkSession] Flush socket pools on low memory notification.

This cl aims to reduce the number of times chrome is killed
when there is not enough memory left (it mostly concerns
Android).

When there is a low memory notification sent by the memory
pressure monitor (either MEMORY_PRESSURE_LEVEL_MODERATE or
MEMORY_PRESSURE_LEVEL_CRITICAL), OnMemoryPressure callback
is called and idle sockets are flushed.

At first I thought to close idle sockets on moderate
notifications and use CloseAllConnections() on critical
notifications, but after some tests decided to flush idle
sockets only, because CloseAllConnections() terminates a
transaction and causes failure.

+ added unit tests.

BUG=617719
==========

to

==========
[HttpNetworkSession] Flush socket pools on low memory notification.

This cl aims to reduce the number of times chrome is killed
when there is not enough memory left (it mostly concerns
Android).

When there is a low memory notification sent by the memory
pressure monitor (either MEMORY_PRESSURE_LEVEL_MODERATE or
MEMORY_PRESSURE_LEVEL_CRITICAL), OnMemoryPressure callback
is called and idle sockets are flushed.

At first I thought to close idle sockets on moderate
notifications and use CloseAllConnections() on critical
notifications, but after some tests decided to flush idle
sockets only, because CloseAllConnections() terminates a
transaction and causes failure.

+ added unit tests.

BUG=617719

Committed: https://crrev.com/0adf859e4a1312c56d324503a59959fd85dc8c44
Cr-Commit-Position: refs/heads/master@{#405708}
==========

[commit-bot: I haz the power, 2016-07-15 06:27:52]

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