Bug 1927543 - Add an if_page_missing option for history metadata.

This commit:

* Introduces an options argument to the Places
  `noteHistoryMetadataObservation()` functions.
* Adds an `if_page_missing` option that specifies what to do if
  the page for the observation doesn't exist in the Places database.
  The default behavior is to ignore the observation.

We'll use this option to only insert new pages into Places for the
initial `DocumentType` observation, and ignore all other observations
if the page isn't in Places.

Author: linabutler

CHANGELOG.md

@@ -13,11 +13,26 @@
 ### FxA Client
 - Updated the iOS `sendToDevice` function to return the `closeTab` call's result when applicable. ([#6448](https://github.com/mozilla/application-services/pull/6448))
 
+### Places
+- `PlacesConnection.noteHistoryMetadataObservation{ViewTime, DocumentType}()`
+  (Android) and `PlacesWriteConnection.noteHistoryMetadataObservation()` (iOS)
+  now take an optional `NoteHistoryMetadataObservationOptions` argument. The
+  new `if_page_missing` option specifies what to do if the page for the
+  observation doesn't exist in the history database.
+  ([#6443](https://github.com/mozilla/application-services/pull/6443))
+
 ## ⚠️ Breaking Changes ⚠️
 
 ### Nimbus SDK ⛅️🔬🔭
 - Added methods to `RecordedContext` for retrieving event queries and setting their values back to the foreign object ([#6322](https://github.com/mozilla/application-services/pull/6322)).
 
+### Places
+- If an entry for a page doesn't exist in the history database, any
+  history observations for that page will no longer be recorded by default.
+  To revert to the old behavior, and automatically insert an entry for
+  the page before recording the observation, set the new `if_page_missing`
+  option to `HistoryMetadataPageMissingBehavior::InsertPage`.
+
 [Full Changelog](In progress)
 
 # v133.0 (_2024-10-28_)

components/places/android/src/main/java/mozilla/appservices/places/PlacesConnection.kt

@@ -20,6 +20,7 @@ import mozilla.appservices.places.uniffi.InsertableBookmark
 import mozilla.appservices.places.uniffi.InsertableBookmarkFolder
 import mozilla.appservices.places.uniffi.InsertableBookmarkItem
 import mozilla.appservices.places.uniffi.InsertableBookmarkSeparator
+import mozilla.appservices.places.uniffi.NoteHistoryMetadataObservationOptions
 import mozilla.appservices.places.uniffi.PlacesApiException
 import mozilla.appservices.places.uniffi.SearchResult
 import mozilla.appservices.places.uniffi.SqlInterruptHandle
@@ -381,37 +382,20 @@ class PlacesWriterConnection internal constructor(conn: UniffiPlacesConnection,
         }
     }
 
-    override suspend fun noteHistoryMetadataObservation(observation: HistoryMetadataObservation) {
+    override suspend fun noteHistoryMetadataObservation(
+        observation: HistoryMetadataObservation,
+        options: NoteHistoryMetadataObservationOptions,
+    ) {
         // Different types of `HistoryMetadataObservation` are flattened out into a list of values.
         // The other side of this (rust code) is going to deal with missing/absent values. We're just
         // passing them along here.
         // NB: Even though `MsgTypes.HistoryMetadataObservation` has an optional title field, we ignore it here.
         // That's used by consumers which aren't already using the history observation APIs.
         return writeQueryCounters.measure {
-            this.conn.noteHistoryMetadataObservation(observation)
+            this.conn.noteHistoryMetadataObservation(observation, options)
         }
     }
 
-    override suspend fun noteHistoryMetadataObservationViewTime(key: HistoryMetadataKey, viewTime: Int) {
-        val obs = HistoryMetadataObservation(
-            url = key.url,
-            searchTerm = key.searchTerm,
-            referrerUrl = key.referrerUrl,
-            viewTime = viewTime,
-        )
-        noteHistoryMetadataObservation(obs)
-    }
-
-    override suspend fun noteHistoryMetadataObservationDocumentType(key: HistoryMetadataKey, documentType: DocumentType) {
-        val obs = HistoryMetadataObservation(
-            url = key.url,
-            searchTerm = key.searchTerm,
-            referrerUrl = key.referrerUrl,
-            documentType = documentType,
-        )
-        noteHistoryMetadataObservation(obs)
-    }
-
     override suspend fun deleteHistoryMetadataOlderThan(olderThan: Long) {
         return writeQueryCounters.measure {
             this.conn.metadataDeleteOlderThan(olderThan)
@@ -645,13 +629,41 @@ interface WritableHistoryMetadataConnection : ReadableHistoryMetadataConnection
     /**
      * Record or update metadata information about a URL. See [HistoryMetadataObservation].
      */
-    suspend fun noteHistoryMetadataObservation(observation: HistoryMetadataObservation)
+    suspend fun noteHistoryMetadataObservation(
+        observation: HistoryMetadataObservation,
+        options: NoteHistoryMetadataObservationOptions = NoteHistoryMetadataObservationOptions(),
+    )
 
     // There's a bit of an impedance mismatch here; `HistoryMetadataKey` is
     // a concept that only exists here and not in the rust. We can iterate on
     // this as the entire "history metadata" requirement evolves.
-    suspend fun noteHistoryMetadataObservationViewTime(key: HistoryMetadataKey, viewTime: Int)
-    suspend fun noteHistoryMetadataObservationDocumentType(key: HistoryMetadataKey, documentType: DocumentType)
+    suspend fun noteHistoryMetadataObservationViewTime(
+        key: HistoryMetadataKey,
+        viewTime: Int,
+        options: NoteHistoryMetadataObservationOptions = NoteHistoryMetadataObservationOptions(),
+    ) {
+        val obs = HistoryMetadataObservation(
+            url = key.url,
+            searchTerm = key.searchTerm,
+            referrerUrl = key.referrerUrl,
+            viewTime = viewTime,
+        )
+        noteHistoryMetadataObservation(obs, options)
+    }
+
+    suspend fun noteHistoryMetadataObservationDocumentType(
+        key: HistoryMetadataKey,
+        documentType: DocumentType,
+        options: NoteHistoryMetadataObservationOptions = NoteHistoryMetadataObservationOptions(),
+    ) {
+        val obs = HistoryMetadataObservation(
+            url = key.url,
+            searchTerm = key.searchTerm,
+            referrerUrl = key.referrerUrl,
+            documentType = documentType,
+        )
+        noteHistoryMetadataObservation(obs, options)
+    }
 
     /**
      * Deletes [HistoryMetadata] with [HistoryMetadata.updatedAt] older than [olderThan].

components/places/android/src/test/java/mozilla/appservices/places/PlacesConnectionTest.kt

@@ -9,6 +9,8 @@ import mozilla.appservices.Megazord
 import mozilla.appservices.places.uniffi.BookmarkItem
 import mozilla.appservices.places.uniffi.DocumentType
 import mozilla.appservices.places.uniffi.FrecencyThresholdOption
+import mozilla.appservices.places.uniffi.HistoryMetadataPageMissingBehavior
+import mozilla.appservices.places.uniffi.NoteHistoryMetadataObservationOptions
 import mozilla.appservices.places.uniffi.PlacesApiException
 import mozilla.appservices.places.uniffi.VisitObservation
 import mozilla.appservices.places.uniffi.VisitType
@@ -576,6 +578,9 @@ class PlacesConnectionTest {
                 referrerUrl = "https://yandex.ru/query?путин+валдай",
             ),
             documentType = DocumentType.MEDIA,
+            NoteHistoryMetadataObservationOptions(
+                ifPageMissing = HistoryMetadataPageMissingBehavior.INSERT_PAGE,
+            ),
         )
 
         // recording view time first, before the document type. either order should be fine.
@@ -584,7 +589,13 @@ class PlacesConnectionTest {
             searchTerm = null,
             referrerUrl = null,
         )
-        db.noteHistoryMetadataObservationViewTime(metaKey2, 200)
+        db.noteHistoryMetadataObservationViewTime(
+            metaKey2,
+            200,
+            NoteHistoryMetadataObservationOptions(
+                ifPageMissing = HistoryMetadataPageMissingBehavior.INSERT_PAGE,
+            ),
+        )
 
         // document type defaults to `regular`.
         with(db.getLatestHistoryMetadataForUrl("https://www.youtube.com/watch?v=fdf4r43g")) {

components/places/ios/Places/Places.swift

@@ -731,44 +731,57 @@ public class PlacesWriteConnection: PlacesReadConnection {
     // MARK: History metadata write APIs
 
     open func noteHistoryMetadataObservation(
-        observation: HistoryMetadataObservation
+        observation: HistoryMetadataObservation,
+        _ options: NoteHistoryMetadataObservationOptions = NoteHistoryMetadataObservationOptions()
     ) throws {
         try queue.sync {
             try self.checkApi()
-            try self.conn.noteHistoryMetadataObservation(data: observation)
+            try self.conn.noteHistoryMetadataObservation(data: observation, options: options)
         }
     }
 
     // Keeping these three functions inline with what Kotlin (PlacesConnection.kt)
     // to make future work more symmetrical
-    open func noteHistoryMetadataObservationViewTime(key: HistoryMetadataKey, viewTime: Int32?) throws {
+    open func noteHistoryMetadataObservationViewTime(
+        key: HistoryMetadataKey,
+        viewTime: Int32?,
+        _ options: NoteHistoryMetadataObservationOptions = NoteHistoryMetadataObservationOptions()
+    ) throws {
         let obs = HistoryMetadataObservation(
             url: key.url,
             referrerUrl: key.referrerUrl,
             searchTerm: key.searchTerm,
             viewTime: viewTime
         )
-        try noteHistoryMetadataObservation(observation: obs)
+        try noteHistoryMetadataObservation(observation: obs, options)
     }
 
-    open func noteHistoryMetadataObservationDocumentType(key: HistoryMetadataKey, documentType: DocumentType) throws {
+    open func noteHistoryMetadataObservationDocumentType(
+        key: HistoryMetadataKey,
+        documentType: DocumentType,
+        _ options: NoteHistoryMetadataObservationOptions = NoteHistoryMetadataObservationOptions()
+    ) throws {
         let obs = HistoryMetadataObservation(
             url: key.url,
             referrerUrl: key.referrerUrl,
             searchTerm: key.searchTerm,
             documentType: documentType
         )
-        try noteHistoryMetadataObservation(observation: obs)
+        try noteHistoryMetadataObservation(observation: obs, options)
     }
 
-    open func noteHistoryMetadataObservationTitle(key: HistoryMetadataKey, title: String) throws {
+    open func noteHistoryMetadataObservationTitle(
+        key: HistoryMetadataKey,
+        title: String,
+        _ options: NoteHistoryMetadataObservationOptions = NoteHistoryMetadataObservationOptions()
+    ) throws {
         let obs = HistoryMetadataObservation(
             url: key.url,
             referrerUrl: key.referrerUrl,
             searchTerm: key.searchTerm,
             title: title
         )
-        try noteHistoryMetadataObservation(observation: obs)
+        try noteHistoryMetadataObservation(observation: obs, options)
     }
 
     open func deleteHistoryMetadataOlderThan(olderThan: Int64) throws {

components/places/src/ffi.rs

@@ -15,7 +15,8 @@ use crate::storage::bookmarks;
 pub use crate::storage::bookmarks::BookmarkPosition;
 pub use crate::storage::history_metadata::{
     DocumentType, HistoryHighlight, HistoryHighlightWeights, HistoryMetadata,
-    HistoryMetadataObservation,
+    HistoryMetadataObservation, HistoryMetadataPageMissingBehavior,
+    NoteHistoryMetadataObservationOptions,
 };
 pub use crate::storage::RunMaintenanceMetrics;
 use crate::storage::{history, history_metadata};
@@ -251,9 +252,10 @@ impl PlacesConnection {
     pub fn note_history_metadata_observation(
         &self,
         data: HistoryMetadataObservation,
+        options: NoteHistoryMetadataObservationOptions,
     ) -> ApiResult<()> {
         // odd historical naming discrepancy - public function is "note_*", impl is "apply_*"
-        self.with_conn(|conn| history_metadata::apply_metadata_observation(conn, data))
+        self.with_conn(|conn| history_metadata::apply_metadata_observation(conn, data, options))
     }
 
     #[handle_error(crate::Error)]

components/places/src/places.udl

@@ -76,7 +76,7 @@ interface PlacesConnection {
     sequence<HistoryHighlight> get_history_highlights(HistoryHighlightWeights weights, i32 limit);
 
     [Throws=PlacesApiError]
-    void note_history_metadata_observation(HistoryMetadataObservation data);
+    void note_history_metadata_observation(HistoryMetadataObservation data, NoteHistoryMetadataObservationOptions options);
 
     [Throws=PlacesApiError]
     void metadata_delete(Url url, Url? referrer_url, string? search_term);
@@ -267,6 +267,21 @@ dictionary HistoryMetadataObservation {
     string? title = null;
 };
 
+/// The action to take when recording a history metadata observation for
+/// a page that doesn't have an entry in the history database.
+enum HistoryMetadataPageMissingBehavior {
+    /// Insert an entry for the page into the history database.
+    "InsertPage",
+
+    /// Ignore and discard the observation. This is the default behavior.
+    "IgnoreObservation",
+};
+
+/// Options for recording history metadata observations.
+dictionary NoteHistoryMetadataObservationOptions {
+    HistoryMetadataPageMissingBehavior if_page_missing = "IgnoreObservation";
+};
+
 /// This is what is returned.
 dictionary HistoryMetadata {
     string url;