[WIP] Fix Windows crashes (#109)

src/electron/ELECTRON_VERSION

@@ -1 +1 @@
-20.3.0-rc11
+20.3.0-rc12

src/electron/office/document_client.cc

@@ -65,11 +65,15 @@ gin::WrapperInfo DocumentClient::kWrapperInfo = {gin::kEmbedderNativeGin};
 namespace {
 
 inline void lok_safe_free(void* ptr) {
+#if BUILDFLAG(IS_WIN)
+  free(ptr);
+#else
   base::UncheckedFree(ptr);
+#endif
 }
 
 struct LokSafeDeleter {
-  inline void operator()(void* ptr) const { base::UncheckedFree(ptr); }
+  inline void operator()(void* ptr) const { lok_safe_free(ptr); }
 };
 
 typedef std::unique_ptr<char[], LokSafeDeleter> LokStrPtr;
@@ -467,10 +471,18 @@ v8::Local<v8::Value> DocumentClient::GotoOutline(int idx,
 }
 
 namespace {
+#if BUILDFLAG(IS_WIN)
+extern "C" void* (*const malloc_unchecked)(size_t);
+#endif
+
 void* UncheckedAlloc(size_t size) {
+#if BUILDFLAG(IS_WIN)
+  return malloc_unchecked(size);
+#else
   void* ptr;
   std::ignore = base::UncheckedMalloc(size, &ptr);
   return ptr;
+#endif
 }
 
 }  // namespace

src/electron/office/lok_tilebuffer.cc

@@ -229,8 +229,8 @@ std::vector<TileRange> TileBuffer::InvalidRangesRemaining(
 
 TileBuffer::RowLimit TileBuffer::LimitRange(int y_pos,
                                             unsigned int view_height) {
-  unsigned int start_row = std::max(
-      std::floor((double)y_pos / (double)TileBuffer::kTileSizePx), 0.0);
+  unsigned int start_row = y_pos < 0 ? 0 :
+      std::floor((double)y_pos / (double)TileBuffer::kTileSizePx);
   unsigned int end_row = start_row + std::ceil((double)view_height /
                                                (double)TileBuffer::kTileSizePx);
   return {start_row, std::max(start_row, end_row)};
@@ -263,7 +263,8 @@ std::vector<TileRange> TileBuffer::ClipRanges(std::vector<TileRange> ranges,
 
 TileRange TileBuffer::NextScrollTileRange(int next_y_pos,
                                           unsigned int view_height) {
-  auto row_limit = LimitRange(next_y_pos - view_height, view_height * 3);
+  next_y_pos = std::max(0, next_y_pos - (int)view_height);
+  auto row_limit = LimitRange(next_y_pos, view_height * 3);
   unsigned int start_row = row_limit.start > 0 ? row_limit.start : 0;
   unsigned int end_row = row_limit.end;
 
@@ -525,6 +526,10 @@ size_t TileCount(std::vector<TileRange> tile_ranges_) {
   return result;
 }
 
+bool TileBuffer::IsEmpty() {
+  return rows_ == 0 || columns_ == 0;
+}
+
 Snapshot TileBuffer::MakeSnapshot(CancelFlagPtr cancel_flag,
                                   const gfx::Rect& rect) {
   std::vector<cc::PaintImage> tiles;

src/electron/office/lok_tilebuffer.h

@@ -117,6 +117,7 @@ class TileBuffer : public base::RefCountedDeleteOnSequence<TileBuffer> {
 
   void SetActiveContext(std::size_t active_context_hash);
   TileBuffer();
+  bool IsEmpty();
 
  private:
   friend class base::RefCountedDeleteOnSequence<TileBuffer>;
@@ -167,24 +168,24 @@ class TileBuffer : public base::RefCountedDeleteOnSequence<TileBuffer> {
   }
 
   struct RowLimit {
-    unsigned int start;
-    unsigned int end;
+    unsigned int start = 0;
+    unsigned int end = 0;
   };
 
   RowLimit LimitRange(int y_pos, unsigned int view_height);
 
-  unsigned int columns_;
-  unsigned int rows_;
-  float scale_;
+  unsigned int columns_ = 0;
+  unsigned int rows_ = 0;
+  float scale_ = 1.0f;
 
-  long doc_width_twips_;
-  long doc_height_twips_;
-  float doc_width_scaled_px_;
-  float doc_height_scaled_px_;
+  long doc_width_twips_ = 0;
+  long doc_height_twips_ = 0;
+  float doc_width_scaled_px_ = 0.0f;
+  float doc_height_scaled_px_ = 0.0f;
 
-  AtomicBitset valid_tile_;
+  AtomicBitset valid_tile_{};
 
-  std::atomic<std::size_t> active_context_hash_;
+  std::atomic<std::size_t> active_context_hash_ = 0;
 
   // ring pool (in order to prevent OOM crash on invididual tile allocations)
 
@@ -213,4 +214,3 @@ class TileBuffer : public base::RefCountedDeleteOnSequence<TileBuffer> {
   bool in_paint_ = false;
 };
 }  // namespace electron::office
-

src/electron/office/office_client.cc

@@ -206,14 +206,15 @@ v8::Local<v8::Promise> OfficeClient::LoadDocumentAsync(
   auto promise_handle = promise.GetHandle();
 
   auto load_ = base::BindOnce(
-      [](const base::WeakPtr<OfficeClient>& client, const std::string& path) {
-        if (!client.MaybeValid())
-          return static_cast<lok::Document*>(nullptr);
-
-        return client->GetOffice()->documentLoad(path.c_str(),
+      [](OfficeClient* client, const std::string& path) {
+        if (client->GetOffice()) {
+          return client->GetOffice()->documentLoad(path.c_str(),
                                                  "Language=en-US,Batch=true");
+        } else {
+          return static_cast<lok::Document*>(nullptr);
+        }
       },
-      weak_factory_.GetWeakPtr(), std::string(path));
+      base::Unretained(this), std::string(path));
   auto complete_ =
       base::BindOnce(&ResolveLoadWithDocumentClient, weak_factory_.GetWeakPtr(),
                      std::move(promise), std::string(path));
@@ -307,11 +308,10 @@ v8::Local<v8::Promise> OfficeClient::SetDocumentPasswordAsync(
     auto async = base::BindOnce(
         [](const base::WeakPtr<OfficeClient>& client, Promise<void> promise,
            const std::string& url, const std::string& password) {
-          if (!client.MaybeValid())
-            return;
-
-          client->GetOffice()->setDocumentPassword(url.c_str(),
-                                                   password.c_str());
+          if (client.MaybeValid()) {
+            client->GetOffice()->setDocumentPassword(url.c_str(),
+                                                    password.c_str());
+          }
           promise.Resolve();
         },
         weak_factory_.GetWeakPtr(), std::move(promise), std::move(url),

src/electron/office/office_instance.h

@@ -4,8 +4,6 @@
 
 #pragma once
 
-#include <map>
-#include <set>
 #include <unordered_map>
 #include <atomic>
 #include "base/hash/hash.h"
@@ -25,8 +23,8 @@ struct DocumentEventId {
   const int event_id;
   const int view_id;
 
-  DocumentEventId(size_t doc_id, int evt_id, int view_id)
-      : document_id(doc_id), event_id(evt_id), view_id(view_id) {}
+  DocumentEventId(size_t doc_id, int evt_id, int view_id_)
+      : document_id(doc_id), event_id(evt_id), view_id(view_id_) {}
 
   bool operator==(const DocumentEventId& other) const {
     return (document_id == other.document_id && event_id == other.event_id &&
@@ -105,4 +103,3 @@ class OfficeInstance {
 };
 
 }  // namespace electron::office
-

src/electron/office/office_web_plugin.cc

@@ -4,8 +4,6 @@
 
 #include "office/office_web_plugin.h"
 
-#include <chrono>
-#include <memory>
 #include <string_view>
 
 #include "LibreOfficeKit/LibreOfficeKit.hxx"
@@ -50,24 +48,18 @@
 #include "third_party/blink/public/common/input/web_input_event.h"
 #include "third_party/blink/public/common/input/web_keyboard_event.h"
 #include "third_party/blink/public/common/input/web_mouse_event.h"
-#include "third_party/blink/public/common/input/web_pointer_properties.h"
-#include "third_party/blink/public/platform/task_type.h"
 #include "third_party/blink/public/platform/web_input_event_result.h"
 #include "third_party/blink/public/web/web_frame_widget.h"
 #include "third_party/blink/public/web/web_plugin_params.h"
 #include "third_party/blink/public/web/web_widget.h"
 #include "ui/base/clipboard/clipboard.h"
 #include "ui/base/cursor/cursor.h"
 #include "ui/base/cursor/mojom/cursor_type.mojom-shared.h"
-#include "ui/base/cursor/platform_cursor.h"
 #include "ui/events/blink/blink_event_util.h"
-#include "ui/gfx/geometry/point3_f.h"
 #include "ui/gfx/geometry/point_conversions.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/rect_f.h"
 #include "ui/gfx/geometry/size.h"
-#include "ui/gfx/geometry/size_conversions.h"
-#include "ui/gfx/geometry/size_f.h"
 #include "ui/gfx/geometry/skia_conversions.h"
 #include "ui/gfx/native_widget_types.h"
 #include "v8/include/v8-isolate.h"
@@ -98,7 +90,7 @@ OfficeWebPlugin::OfficeWebPlugin(blink::WebPluginParams params,
   auto* inst = office::OfficeInstance::Get();
   if (inst)
     inst->AddDestroyedObserver(this);
-};
+}
 
 bool OfficeWebPlugin::Initialize(blink::WebPluginContainer* container) {
   container_ = container;
@@ -111,6 +103,7 @@ bool OfficeWebPlugin::Initialize(blink::WebPluginContainer* container) {
 }
 
 void OfficeWebPlugin::Destroy() {
+  paint_manager_->OnDestroy();
   if (document_client_.MaybeValid()) {
     document_client_->Unmount();
     document_client_->MarkRendererWillRemount(
@@ -924,6 +917,10 @@ void OfficeWebPlugin::UpdateIntersectingPages() {
 void OfficeWebPlugin::UpdateScroll(int y_position) {
   if (!document_ || !document_client_.MaybeValid() || stop_scrolling_)
     return;
+  if (!tile_buffer_ || tile_buffer_->IsEmpty()) {
+    LOG(ERROR) << "Tile buffer is empty during scroll";
+    return;
+  }
 
   float view_height =
       plugin_rect_.height() / device_scale_ / (float)viewport_zoom_;
@@ -982,7 +979,7 @@ std::string OfficeWebPlugin::RenderDocument(
   if (needs_reset && document_client_.MaybeValid()) {
     document_client_->Unmount();
   }
-  bool needs_restore = maybe_restore_key.has_value();
+  bool needs_restore = !needs_reset && document_ && document_ == client->GetDocument() && maybe_restore_key.has_value();
 
   document_ = client->GetDocument();
   document_client_ = client->GetWeakPtr();
@@ -993,20 +990,27 @@ std::string OfficeWebPlugin::RenderDocument(
   }
 
   if (needs_reset) {
+    first_paint_ = true;
     tile_buffer_->InvalidateAllTiles();
   }
 
   if (needs_restore) {
     auto transferable = client->GetRestoredRenderer(maybe_restore_key.value());
-    tile_buffer_ = std::move(transferable.tile_buffer);
+    if (transferable.tile_buffer && !transferable.tile_buffer->IsEmpty()) {
+      tile_buffer_ = std::move(transferable.tile_buffer);
+    }
     snapshot_ = std::move(transferable.snapshot);
-    paint_manager_ = std::move(transferable.paint_manager);
+    if (transferable.paint_manager) {
+      paint_manager_ = std::make_unique<office::PaintManager>(this, std::move(transferable.paint_manager));
+    }
     first_paint_ = false;
     page_rects_cached_ = std::move(transferable.page_rects);
     first_intersect_ = transferable.first_intersect;
     last_intersect_ = transferable.last_intersect;
     last_cursor_rect_ = std::move(transferable.last_cursor_rect);
+    if (transferable.zoom > 0) {
     zoom_ = transferable.zoom;
+    }
   }
 
   client->Mount(isolate);
@@ -1015,6 +1019,10 @@ std::string OfficeWebPlugin::RenderDocument(
   } else {
     auto size = document_client_->DocumentSizeTwips();
     scroll_y_position_ = 0;
+    // TODO: figure out why zoom_ can sometimes be set to NaN
+    if (zoom_ != zoom_ || zoom_ < 0) {
+      zoom_ = 1.0f;
+    }
     tile_buffer_->SetYPosition(0);
     tile_buffer_->Resize(size.width(), size.height(), TotalScale());
   }
@@ -1059,6 +1067,11 @@ void OfficeWebPlugin::ScheduleAvailableAreaPaint(bool invalidate) {
   gfx::RectF offset_area(available_area_);
   offset_area.Offset(0, scroll_y_position_);
   auto view_height = offset_area.height();
+  // this is a crash case that should not occur anymore
+  if (tile_buffer_->IsEmpty()) {
+    LOG(ERROR) << "Full area paint, but tile buffer is empty";
+    return;
+  }
   auto range = tile_buffer_->InvalidateTilesInRect(offset_area, !invalidate);
   auto limit = tile_buffer_->LimitIndex(scroll_y_position_, view_height);