Shrink large images, reporting wherever the caller already reports (BL-16646)

src/BloomExe/Book/Book.cs

@@ -1107,7 +1107,7 @@ public void EnsureUpToDate(IProgress progress = null, bool forCopyOfUpToDateBook
             EnsureUpToDateMemory(progress);
             UpdateSupportFiles();
 
-            Storage.MigrateToMediaLevel1ShrinkLargeImages();
+            Storage.MigrateToMediaLevel1ShrinkLargeImages(progress);
 
             Storage.CleanupUnusedSupportFiles(forCopyOfUpToDateBook);
 
@@ -1895,7 +1895,7 @@ public void EnsureUpToDateMemory(IProgress progress)
             // already been done, so they must be called in exactly this order.
             Storage.RestoreStuffBeforeMigration();
             Storage.MigrateMaintenanceLevels();
-            Storage.MigrateToMediaLevel1ShrinkLargeImages();
+            Storage.MigrateToMediaLevel1ShrinkLargeImages(progress);
             Storage.MigrateToLevel2RemoveTransparentComicalSvgs();
             Storage.MigrateToLevel3PutImgFirst();
             Storage.MigrateToLevel4UseAppearanceSystem();

src/BloomExe/Book/BookProcessor.cs

@@ -88,13 +88,12 @@ public static int ProcessBook(Book book, bool fitImageTextSplits = false)
             // BookStorage.MigrateToMediaLevel1ShrinkLargeImages, won't help here: the bridge HTML
             // already carries a modern maintenance level, so BringBookUpToDate below treats that
             // migration as already done and skips it. So we do the shrink ourselves, unconditionally
-            // (not gated by mediaMaintenanceLevel), and it must come BEFORE BringBookUpToDate: (a) on
-            // a book old enough that the migration WOULD run, it normally finds nothing left to
-            // shrink, so its modal progress dialog is not created on this background thread (where a
-            // WinForms dialog is illegal, BL-16646) -- "normally" because a GraphicsMagick failure can
-            // leave an image oversized, in which case that narrow old-book case can still hit the
-            // dialog; (b) the off-screen per-page fix-up then measures and lays out against the
-            // final, already-shrunk images.
+            // (not gated by mediaMaintenanceLevel), and it must come BEFORE BringBookUpToDate so that
+            // the off-screen per-page fix-up measures and lays out against the final, already-shrunk
+            // images. (This used to have a second reason -- keeping the migration from creating its
+            // modal progress dialog on this background thread, which WinForms forbids. BL-16646
+            // removed that dialog in favor of reporting through the caller's IProgress, so only the
+            // layout reason remains.)
             if (ImageUtils.NeedToShrinkImages(book.FolderPath))
             {
                 Log("shrinking oversized images in the book folder");

src/BloomExe/Book/BookStorage.cs

@@ -107,7 +107,7 @@ void CleanupUnusedSupportFiles(
         void CaptureInitialStateForMigration();
         void RestoreStuffBeforeMigration();
         void MigrateMaintenanceLevels();
-        void MigrateToMediaLevel1ShrinkLargeImages();
+        void MigrateToMediaLevel1ShrinkLargeImages(IProgress progress);
         void MigrateToLevel2RemoveTransparentComicalSvgs();
         void MigrateToLevel3PutImgFirst();
 
@@ -4047,7 +4047,11 @@ public void MigrateMaintenanceLevels()
         /// memory. However, it is still helpful for performance and reducing published file sizes.
         /// Does nothing if mediaMaintenanceLevel indicates it has already been done.
         /// </summary>
-        public void MigrateToMediaLevel1ShrinkLargeImages()
+        /// <param name="progress">Where to report the (potentially very slow) shrinking, so the
+        /// user can see why we are busy. Callers pass whatever progress they already own; headless
+        /// callers such as the bulk-upload CLI pass a NullProgress and so report nothing. Must not
+        /// be null: ImageUtils.FixSizeAndTransparencyOfImagesInFolder dereferences it.</param>
+        public void MigrateToMediaLevel1ShrinkLargeImages(IProgress progress)
         {
             var levelString = Dom.GetMetaValue("mediaMaintenanceLevel", "0");
             if (!int.TryParse(levelString, out int level))
@@ -4058,10 +4062,11 @@ public void MigrateToMediaLevel1ShrinkLargeImages()
             {
                 // If the book contains overlarge images, we want to fix those before editing because this can lead
                 // to thumbnails not being created properly and other bad behavior.  This is a one-time fix that can
-                // permanently change the images in the original book folder.  If any images must be shrunk, then a
-                // progress dialog pops up because that can be a very slow process.  If nothing needs to be done,
-                // nothing will appear on the screen, and it usually takes a small fraction of a second to determine
-                // this.
+                // permanently change the images in the original book folder.  Shrinking can be very slow, so we
+                // report it through the caller's progress rather than putting up our own dialog (BL-16646: this
+                // used to create a WinForms dialog, which is illegal on the background threads several of these
+                // callers run on).  If nothing needs to be done, nothing is reported at all, and it usually takes
+                // a small fraction of a second to determine this.
 
                 // Bloom 4.9 and later limit images used by Bloom books to be no larger than 3500x2550 in
                 // order to avoid out of memory errors that can happen with really large images.
@@ -4073,13 +4078,21 @@ public void MigrateToMediaLevel1ShrinkLargeImages()
                 // This update can be very slow, so encourage the user that something is happening.
                 // NO images should have transparency removed.  See https://issues.bloomlibrary.org/youtrack/issue/BL-8846.
 
-                if (Program.RunningUnitTests)
+                var shell = Shell.GetShellOrOtherOpenForm();
+                // shell is null when no window is open at all -- the headless CLI (bulk upload,
+                // hydrate). There is nothing to show a dialog on, and nothing to ask about thread
+                // affinity, so just do the work against whatever progress we were handed.
+                if (
+                    Program.RunningUnitTests
+                    || progress is WebProgressAdapter
+                    || shell == null
+                    || !shell.InvokeRequired
+                )
                 {
-                    // TeamCity enforces not showing modal dialogs during unit tests on Windows 10.
                     ImageUtils.FixSizeAndTransparencyOfImagesInFolder(
                         FolderPath,
                         new List<string>(),
-                        new NullProgress()
+                        progress
                     );
                 }
                 else
@@ -4098,7 +4111,6 @@ public void MigrateToMediaLevel1ShrinkLargeImages()
                     }
                 }
             }
-
             Dom.UpdateMetaElement("mediaMaintenanceLevel", "1");
         }
 

src/BloomExe/CollectionTab/CollectionModel.BloomSourceImport.cs

@@ -154,7 +154,7 @@ public void ImportBloomSourceFiles(
                     {
                         // The derivative path builds its Book in memory and adds it to the collection
                         // itself, so it needs no reload.
-                        var book = MakeDerivativeFromBloomSourceFile(path);
+                        var book = MakeDerivativeFromBloomSourceFile(path, progress);
                         if (book != null)
                             lastImported = book;
                     }
@@ -561,7 +561,10 @@ out string instanceId
         /// creation was cancelled (e.g. a template configuration dialog). Because a derivative always
         /// gets a fresh id, there is never a duplicate to resolve.
         /// </summary>
-        internal Book.Book MakeDerivativeFromBloomSourceFile(string sourcePath)
+        internal Book.Book MakeDerivativeFromBloomSourceFile(
+            string sourcePath,
+            IWebSocketProgress progress
+        )
         {
             var tempFolder = ExtractAndPrepareBloomSourceToTemp(sourcePath, out _, out _);
             try
@@ -589,7 +592,10 @@ internal Book.Book MakeDerivativeFromBloomSourceFile(string sourcePath)
                     newBook.BookData.Language1Tag
                 );
 
-                newBook.BringBookUpToDate(new NullProgress(), false);
+                newBook.BringBookUpToDate(
+                    progress == null ? new NullProgress() : new WebProgressAdapter(progress),
+                    false
+                );
 
                 TheOneEditableCollection.AddBookInfo(newBook.BookInfo);
                 newBook.RecordPendingCreatedHistoryEvent();

src/BloomExe/web/WebProgressAdapter.cs

@@ -7,7 +7,7 @@
 namespace Bloom.web
 {
     /// <summary>
-    /// Class that allows code expecting an SIL.Progress.IProgress object to use a WebSocketProgress instead.
+    /// Class that allows code expecting an SIL.Progress.IProgress object to use an IWebSocketProgress instead.
     /// </summary>
     public class WebProgressAdapter : IProgress
     {
@@ -32,9 +32,9 @@ public void IndicateUnknownProgress() { }
             public void Initialize() { }
         }
 
-        private readonly WebSocketProgress _webProgress;
+        private readonly IWebSocketProgress _webProgress;
 
-        public WebProgressAdapter(WebSocketProgress progress)
+        public WebProgressAdapter(IWebSocketProgress progress)
         {
             _webProgress = progress;
         }

src/BloomTests/Book/BookStorageTests.cs

@@ -1999,7 +1999,7 @@ public void PerformNecessaryMaintenanceOnBook_DeletesSVGIfOnlyNoneStyle()
             );
 
             //SUT
-            storage.MigrateToMediaLevel1ShrinkLargeImages(); // does nothing in this situation, but should still bump level
+            storage.MigrateToMediaLevel1ShrinkLargeImages(new NullProgress()); // does nothing in this situation, but should still bump level
             var mediaLevel = storage.Dom.GetMetaValue("mediaMaintenanceLevel", "0");
             Assert.That(mediaLevel, Is.EqualTo("1"));
             storage.MigrateToLevel2RemoveTransparentComicalSvgs();
@@ -2129,7 +2129,7 @@ public void PerformNecessaryMaintenanceOnBook_EnsuresImgAtStartOfImageContainer(
             );
 
             //SUT
-            storage.MigrateToMediaLevel1ShrinkLargeImages();
+            storage.MigrateToMediaLevel1ShrinkLargeImages(new NullProgress());
             storage.MigrateToLevel2RemoveTransparentComicalSvgs();
             storage.MigrateToLevel3PutImgFirst();
 
@@ -2167,7 +2167,7 @@ public void PerformNecessaryMaintenanceOnBook_DoesNotDeleteSVGIfOtherStylePresen
             );
 
             //SUT
-            storage.MigrateToMediaLevel1ShrinkLargeImages();
+            storage.MigrateToMediaLevel1ShrinkLargeImages(new NullProgress());
             storage.MigrateToLevel2RemoveTransparentComicalSvgs();
             storage.MigrateToLevel3PutImgFirst();
 
@@ -2333,7 +2333,7 @@ public void PerformNecessaryMaintenanceOnBook_HandlesMultipleSVGs()
             );
 
             //SUT
-            storage.MigrateToMediaLevel1ShrinkLargeImages();
+            storage.MigrateToMediaLevel1ShrinkLargeImages(new NullProgress());
             storage.MigrateToLevel2RemoveTransparentComicalSvgs();
             storage.MigrateToLevel3PutImgFirst();
 
@@ -2615,7 +2615,7 @@ public void PerformNecessaryMaintenanceOnBook_DoesNothingIfAlreadyProcessed()
             );
 
             //SUT
-            storage.MigrateToMediaLevel1ShrinkLargeImages();
+            storage.MigrateToMediaLevel1ShrinkLargeImages(new NullProgress());
             storage.MigrateToLevel2RemoveTransparentComicalSvgs();
             storage.MigrateToLevel3PutImgFirst();