Remove usage of WriteCheckSessionHandler

+++ b/core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php
@@ -182,7 +182,6 @@ public static function getSkippedDeprecations() {
       'The "session_handler.write_check" service relies on the deprecated "Symfony\Component\HttpFoundation\Session\Storage\Handler\WriteCheckSessionHandler" class. It should either be deprecated or its implementation upgraded.',

Should also be able to remove this deprecation here too.

I think it is reasonable to merge this functionality into \Drupal\Core\Session\WriteSafeSessionHandler

+++ b/core/lib/Drupal/Core/Session/WriteSafeSessionHandler.php
@@ -19,6 +19,13 @@ class WriteSafeSessionHandler implements \SessionHandlerInterface, WriteSafeSess
+  private $readSessions;

I kept this as private because it makes sense that this info should never leak out of this object. It's private in Symfony's implementation.

I think once we support only PHP 7.0 then we can probably remove this completely and use http://php.net/manual/en/session.configuration.php#ini.session.lazy-write

Outdated duplicate: #2937540: Remove usages of Symfony\Component\HttpFoundation\Session\Storage\Handler\WriteCheckSessionHandler

Related: #2937541: [PP-1] The "session_handler.write_check" service relies on the deprecated WriteCheckSessionHandler class

Needs follow-up for the work in #9, with a @todo.

As I see we are free to use PHP 7 here. I am not sure if I understand the issue, but I hope it helps.

Fixed #11. I'm not sure about adding an @todo. There is plenty of session changes we can make once we are PHP 7 only. Happy to open an issue though. Opened #3001180: Use session.lazy-write once Drupal 8 only supports PHP 7

+++ b/core/core.services.yml
@@ -1545,10 +1545,6 @@ services:
-  session_handler.write_check:
-    class: Symfony\Component\HttpFoundation\Session\Storage\Handler\WriteCheckSessionHandler
-    tags:
-      - { name: session_handler_proxy, priority: 100 }

I think removing this service in a minor is okay because it is like an event subscriber and not really meant to be overridden. Should have a CR though.

Created https://www.drupal.org/node/3001185

once we are PHP 7 only

I do not understand, Drupal 8.7 is PHP 7 only. And this issue should not be backported to 8.6.
So why not solve it with PHP 7?

@Pasqualle because there is an idea of maintaining PHP 5.6 support in 8.7.x unless we really can't.

+++ b/core/core.services.yml
@@ -1545,10 +1545,6 @@ services:
-  session_handler.write_check:

according to http://grep.xnddx.ru/search?text=session_handler.write_check the only uses of this are in the IDE helper project, which is a meta project

but I will defer to release manager on this, in an ideal world, we'd mark it private in one minor and remove in another, but I don't think it warrants that effort - a change record and release notes snippet will suffice in my opinion

Tagging for release manager review, and we need a release notes snippet here

Agreed this is fine in a minor with a change record, thanks for opening the follow-up issue.

Looks RTBC to me too.

We still need a release notes snippet here, then can go back to RTBC

Updated issue summary.

+++ b/core/lib/Drupal/Core/Session/WriteSafeSessionHandler.php
@@ -64,14 +72,20 @@ public function open($save_path, $session_id) {
+    // Only write the session when it has been modified.
+    if (isset($this->readSessions[$session_id]) && $this->readSessions[$session_id] === $session_data) {
+      return TRUE;
+    }
+    elseif ($this->isSessionWritable()) {
       return $this->wrappedSessionHandler->write($session_id, $session_data);
     }

because we return early, we don't need the elseif and else here

Done. Considering there's no real change straight back to RTBC.

Committed 9498875 and pushed to 8.7.x. Thanks!

Published change record