report_rom: versioning and explicit notification

The former implementation did not internally track ROM changes notified
vs. delivered to the client. We adapt the versioning implementation
implemented in dynamic_rom_session.h and enable explicit notification of
the current version.

The feature is used by the clipboard to notify permitted readers of the
clipboard ROM service on focus change via the newly created private
Rom::Module::_notify_permitted_readers() function.

Fixes #4274

repos/os/include/report_rom/rom_module.h

@@ -52,8 +52,9 @@ struct Rom::Reader : private Reader_list::Element, Interface
 	friend class Genode::List<Reader>;
 	using Reader_list::Element::next;
 
-	virtual void notify_module_changed()     = 0;
-	virtual void notify_module_invalidated() = 0;
+	virtual void mark_as_outdated()    = 0;
+	virtual void mark_as_invalidated() = 0;
+	virtual void notify_client()       = 0;
 };
 
 
@@ -244,6 +245,17 @@ struct Rom::Module : private Module_list::Element, Readable_module
 			return cnt;
 		}
 
+		void _notify_permitted_readers()
+		{
+			if (!_last_writer)
+				return;
+
+			for (Reader *r = _readers.first(); r; r = r->next()) {
+				if (_read_policy.read_permitted(*this, *_last_writer, *r))
+					r->notify_client();
+			}
+		}
+
 	public:
 
 		/**
@@ -281,9 +293,10 @@ struct Rom::Module : private Module_list::Element, Readable_module
 			for (Reader *r = _readers.first(); r; r = r->next()) {
 
 				if (_read_policy.read_permitted(*this, *_last_writer, *r))
-					r->notify_module_changed();
+					r->mark_as_outdated();
 				else
-					r->notify_module_invalidated();
+					r->mark_as_invalidated();
+				r->notify_client();
 			}
 		}
 

repos/os/include/report_rom/rom_service.h

@@ -53,19 +53,28 @@ class Rom::Session_component : public Genode::Rpc_object<Genode::Rom_session>,
 
 		Constructible<Genode::Attached_ram_dataspace> _ds { };
 
-		size_t _content_size = 0;
-
 		/**
-		 * Keep state of valid content to notify the client only once when
-		 * the ROM module becomes invalid.
+		 * Size of content delivered to the client
+		 *
+		 * Zero-sized content means invalidated ROM state was delivered to the
+		 * client.
 		 */
-		bool _valid = false;
+		size_t _content_size = 0;
 
 		Genode::Signal_context_capability _sigh { };
 
+		/*
+		 * Keep track of the last version handed out to the client (at the
+		 * time of the last 'Rom_session::update' RPC call, and the newest
+		 * version that is available. If the client version is out of date
+		 * when the client registers a signal handler, submit a signal
+		 * immediately.
+		 */
+		unsigned _current_version = 0, _client_version = 0;
+
 		void _notify_client()
 		{
-			if (_sigh.valid())
+			if (_sigh.valid() && (_current_version != _client_version))
 				Genode::Signal_transmitter(_sigh).submit();
 		}
 
@@ -90,19 +99,19 @@ class Rom::Session_component : public Genode::Rpc_object<Genode::Rom_session>,
 		{
 			using namespace Genode;
 
-				/* replace dataspace by new one */
-				/* XXX we could keep the old dataspace if the size fits */
-				_ds.construct(_ram, _rm, _module.size());
+			/* replace dataspace by new one */
+			/* XXX we could keep the old dataspace if the size fits */
+			_ds.construct(_ram, _rm, _module.size());
 
-				/* fill dataspace content with report contained in module */
-				_content_size =
-					_module.read_content(*this, _ds->local_addr<char>(), _ds->size());
+			/* fill dataspace content with report contained in module */
+			_content_size =
+				_module.read_content(*this, _ds->local_addr<char>(), _ds->size());
 
-				_valid = _content_size > 0;
+			_client_version = _current_version;
 
-				/* cast RAM into ROM dataspace capability */
-				Dataspace_capability ds_cap = static_cap_cast<Dataspace>(_ds->cap());
-				return static_cap_cast<Rom_dataspace>(ds_cap);
+			/* cast RAM into ROM dataspace capability */
+			Dataspace_capability ds_cap = static_cap_cast<Dataspace>(_ds->cap());
+			return static_cap_cast<Rom_dataspace>(ds_cap);
 		}
 
 		bool update() override
@@ -120,7 +129,7 @@ class Rom::Session_component : public Genode::Rpc_object<Genode::Rom_session>,
 
 			_content_size = new_content_size;
 
-			_valid = _content_size > 0;
+			_client_version = _current_version;
 
 			return true;
 		}
@@ -140,23 +149,22 @@ class Rom::Session_component : public Genode::Rpc_object<Genode::Rom_session>,
 		/**
 		 * Reader interface
 		 */
-		void notify_module_changed() override
+		void mark_as_outdated() override { _current_version++; }
+
+		/**
+		 * Reader interface
+		 */
+		void mark_as_invalidated() override
 		{
-			_notify_client();
+			/* increase version only if we delivered valid content last */
+			if (_content_size > 0)
+				_current_version++;
 		}
 
 		/**
 		 * Reader interface
 		 */
-		void notify_module_invalidated() override
-		{
-			/* deliver a signal for an invalidated module only once */
-			if (!_valid)
-				return;
-
-			_valid = false;
-			_notify_client();
-		}
+		void notify_client() override { _notify_client(); }
 };
 
 

repos/os/recipes/pkg/test-report_rom/runtime

@@ -26,7 +26,7 @@
 			[init -> test-report_rom] Reporter: start reporting (while the ROM client still listens)
 			[init -> test-report_rom] ROM client: wait for update notification
 			[init -> test-report_rom] ROM client: try to open the same report again
-			[init -> test-report_rom] Error: Report-session creation failed (label="brightness", ram_quota=14336, cap_quota=3, buffer_size=4096)
+			[init -> test-report_rom] *Error: Report-session creation failed (label="brightness", ram_quota=14336, cap_quota=3, buffer_size=4096)*
 			[init -> test-report_rom] ROM client: caught Service_denied - OK
 			[init -> test-report_rom] --- test-report_rom finished ---
 			[init] child "test-report_rom" exited with exit value 0

repos/os/src/app/pointer/main.cc

@@ -115,20 +115,14 @@ class Pointer::Main : public Rom::Reader
 		void _handle_hover();
 		void _handle_xray();
 
-	public:
-
 		/**
 		 * Reader interface
 		 */
-		void notify_module_changed() override
-		{
-			_update_pointer();
-		}
+		void mark_as_outdated()    override { }
+		void mark_as_invalidated() override { }
+		void notify_client()       override { _update_pointer(); }
 
-		void notify_module_invalidated() override
-		{
-			_update_pointer();
-		}
+	public:
 
 		Main(Genode::Env &);
 };

repos/os/src/server/clipboard/main.cc

@@ -65,6 +65,11 @@ struct Rom::Registry : Rom::Registry_for_reader, Rom::Registry_for_writer
 	:
 		module(ram, rm, "clipboard", read_policy, write_policy)
 	{ }
+
+	void notify_reader_on_focus()
+	{
+		module._notify_permitted_readers();
+	}
 };
 
 
@@ -120,6 +125,8 @@ struct Clipboard::Main : Rom::Module::Read_policy, Rom::Module::Write_policy
 			_focused_domain = focus.attribute_value("domain", Domain());
 			_focused_label  = focus.attribute_value("label",  Label());
 		}
+
+		_rom_registry.notify_reader_on_focus();
 	}
 
 	Domain _domain(Genode::Session_label const &label) const