Fix crash in SkiaOutputDeviceX11 ctor

Probably the crash is because the window is destroyed, it causes X11
call failed, and then LOG(FATAL) causes crash. Fix the problem by
adding SkiaOutputDeviceX11::Create() method, which will return null
when X11 call fails.

Bug: 1153556
Change-Id: Id19e51b4fd8f0540125609bb479532199d3a7d4a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2598582Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Peng Huang <penghuang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#838915}

components/viz/service/display_embedder/skia_output_device_x11.cc

@@ -27,10 +27,31 @@ x11::GraphicsContext CreateGC(x11::Connection* connection, x11::Window window) {
 
 }  // namespace
 
-SkiaOutputDeviceX11::SkiaOutputDeviceX11(
+// static
+std::unique_ptr<SkiaOutputDeviceX11> SkiaOutputDeviceX11::Create(
     scoped_refptr<gpu::SharedContextState> context_state,
     gfx::AcceleratedWidget widget,
     gpu::MemoryTracker* memory_tracker,
+    DidSwapBufferCompleteCallback did_swap_buffer_complete_callback) {
+  auto window = static_cast<x11::Window>(widget);
+  auto attributes =
+      x11::Connection::Get()->GetWindowAttributes({window}).Sync();
+  if (!attributes) {
+    DLOG(ERROR) << "Failed to get attributes for window "
+                << static_cast<uint32_t>(window);
+    return {};
+  }
+  return std::make_unique<SkiaOutputDeviceX11>(
+      base::PassKey<SkiaOutputDeviceX11>(), std::move(context_state), window,
+      attributes->visual, memory_tracker, did_swap_buffer_complete_callback);
+}
+
+SkiaOutputDeviceX11::SkiaOutputDeviceX11(
+    base::PassKey<SkiaOutputDeviceX11> pass_key,
+    scoped_refptr<gpu::SharedContextState> context_state,
+    x11::Window window,
+    x11::VisualId visual,
+    gpu::MemoryTracker* memory_tracker,
     DidSwapBufferCompleteCallback did_swap_buffer_complete_callback)
     : SkiaOutputDeviceOffscreen(context_state,
                                 gfx::SurfaceOrigin::kTopLeft,
@@ -38,15 +59,9 @@ SkiaOutputDeviceX11::SkiaOutputDeviceX11(
                                 memory_tracker,
                                 did_swap_buffer_complete_callback),
       connection_(x11::Connection::Get()),
-      window_(static_cast<x11::Window>(widget)),
+      window_(window),
+      visual_(visual),
       gc_(CreateGC(connection_, window_)) {
-  if (auto attributes = connection_->GetWindowAttributes({window_}).Sync()) {
-    visual_ = attributes->visual;
-  } else {
-    LOG(FATAL) << "Failed to get attributes for window "
-               << static_cast<uint32_t>(window_);
-  }
-
   // |capabilities_| should be set by SkiaOutputDeviceOffscreen.
   DCHECK_EQ(capabilities_.output_surface_origin, gfx::SurfaceOrigin::kTopLeft);
   DCHECK(capabilities_.supports_post_sub_buffer);

components/viz/service/display_embedder/skia_output_device_x11.h

@@ -5,9 +5,11 @@
 #ifndef COMPONENTS_VIZ_SERVICE_DISPLAY_EMBEDDER_SKIA_OUTPUT_DEVICE_X11_H_
 #define COMPONENTS_VIZ_SERVICE_DISPLAY_EMBEDDER_SKIA_OUTPUT_DEVICE_X11_H_
 
+#include <memory>
 #include <vector>
 
 #include "base/macros.h"
+#include "base/types/pass_key.h"
 #include "components/viz/service/display_embedder/skia_output_device_offscreen.h"
 #include "ui/gfx/native_widget_types.h"
 #include "ui/gfx/x/connection.h"
@@ -18,12 +20,20 @@ namespace viz {
 class SkiaOutputDeviceX11 final : public SkiaOutputDeviceOffscreen {
  public:
   SkiaOutputDeviceX11(
+      base::PassKey<SkiaOutputDeviceX11> pass_key,
       scoped_refptr<gpu::SharedContextState> context_state,
-      gfx::AcceleratedWidget widget,
+      x11::Window window,
+      x11::VisualId visual,
       gpu::MemoryTracker* memory_tracker,
       DidSwapBufferCompleteCallback did_swap_buffer_complete_callback);
   ~SkiaOutputDeviceX11() override;
 
+  static std::unique_ptr<SkiaOutputDeviceX11> Create(
+      scoped_refptr<gpu::SharedContextState> context_state,
+      gfx::AcceleratedWidget widget,
+      gpu::MemoryTracker* memory_tracker,
+      DidSwapBufferCompleteCallback did_swap_buffer_complete_callback);
+
   bool Reshape(const gfx::Size& size,
                float device_scale_factor,
                const gfx::ColorSpace& color_space,
@@ -36,10 +46,10 @@ class SkiaOutputDeviceX11 final : public SkiaOutputDeviceOffscreen {
                      std::vector<ui::LatencyInfo> latency_info) override;
 
  private:
-  x11::Connection* connection_ = nullptr;
-  x11::Window window_ = x11::Window::None;
-  x11::GraphicsContext gc_{};
-  x11::VisualId visual_{};
+  x11::Connection* const connection_;
+  const x11::Window window_;
+  const x11::VisualId visual_;
+  const x11::GraphicsContext gc_;
   scoped_refptr<base::RefCountedMemory> pixels_;
 
   DISALLOW_COPY_AND_ASSIGN(SkiaOutputDeviceX11);

components/viz/service/display_embedder/skia_output_surface_impl_on_gpu.cc

@@ -1265,7 +1265,7 @@ bool SkiaOutputSurfaceImplOnGpu::InitializeForVulkan() {
           GetDidSwapBuffersCompleteCallback());
     }
     if (!output_device_) {
-      output_device_ = std::make_unique<SkiaOutputDeviceX11>(
+      output_device_ = SkiaOutputDeviceX11::Create(
           context_state_, dependency_->GetSurfaceHandle(),
           shared_gpu_deps_->memory_tracker(),
           GetDidSwapBuffersCompleteCallback());
@@ -1310,12 +1310,10 @@ bool SkiaOutputSurfaceImplOnGpu::InitializeForDawn() {
     // TODO(sgilhuly): Set up a Vulkan swapchain so that Linux can also use
     // SkiaOutputDeviceDawn.
     if (!features::IsUsingOzonePlatform()) {
-      output_device_ = std::make_unique<SkiaOutputDeviceX11>(
+      output_device_ = SkiaOutputDeviceX11::Create(
           context_state_, dependency_->GetSurfaceHandle(),
           shared_gpu_deps_->memory_tracker(),
           GetDidSwapBuffersCompleteCallback());
-    } else {
-      return false;
     }
 #elif defined(OS_WIN)
     std::unique_ptr<SkiaOutputDeviceDawn> output_device =
@@ -1334,7 +1332,7 @@ bool SkiaOutputSurfaceImplOnGpu::InitializeForDawn() {
 #endif
   }
 #endif
-  return true;
+  return !!output_device_;
 }
 
 bool SkiaOutputSurfaceImplOnGpu::MakeCurrent(bool need_framebuffer) {