[Fuchsia] Make our FIDL error-handlers more consistent.

Update the functions we register as FIDL error handlers to use :
- ZX_LOG(FATAL, status) where we never expect to lose connectivity,
  e.g. for connections to system services such as the Netstack. We
  typically follow these with an indication of what disconnected, e.g:
    ZX_LOG(FATAL, status) << " Netstack disconnected.";
- ZX_LOG_IF(FATAL, status != ZX_ERR_PEER_CLOSED, status) where we are
  providing service to a single peer that may disconnect from us at any
  time, where other status values are unexpected.
- ZX_LOG(ERROR, status) where we never expect to lose connectivity, but
  are serving multiple peers, so must not simply crash the process.
- ZX_LOG_IF(ERROR, status != ZX_ERR_PEER_CLOSED, status) where we are
  providing service to multiple peers that we expect may disconnect, to
  verify that they do so gracefully.

We also update FIDL error handlers in some test suites:
- EXPECT_EQ(status, ZX_ERR_PEER_CLOSED) where the test actively expects
  that the FIDL channel will be disconnected gracefully.
- ZX_LOG(ERROR, status) where the test does not expect FIDL errors.

Bug: 927400
Change-Id: I10fef90778e58f2aedadb806f857da00aace96aa
Reviewed-on: https://chromium-review.googlesource.com/c/1448796
Auto-Submit: Wez <wez@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628524}

fuchsia/browser/frame_impl.cc

@@ -191,8 +191,11 @@ FrameImpl::FrameImpl(std::unique_ptr<content::WebContents> web_contents,
       binding_(this, std::move(frame_request)) {
   web_contents_->SetDelegate(this);
   Observe(web_contents_.get());
-  binding_.set_error_handler(
-      [this](zx_status_t status) { context_->DestroyFrame(this); });
+  binding_.set_error_handler([this](zx_status_t status) {
+    ZX_LOG_IF(ERROR, status != ZX_ERR_PEER_CLOSED, status)
+        << " Frame disconnected.";
+    context_->DestroyFrame(this);
+  });
 
   content::UpdateFontRendererPreferencesFromSystemSettings(
       web_contents_->GetMutableRendererPrefs());

fuchsia/browser/frame_impl_browsertest.cc

@@ -152,7 +152,10 @@ IN_PROC_BROWSER_TEST_F(FrameImplTest, ContextDeletedBeforeFrame) {
   EXPECT_TRUE(frame);
 
   base::RunLoop run_loop;
-  frame.set_error_handler([&run_loop](zx_status_t status) { run_loop.Quit(); });
+  frame.set_error_handler([&run_loop](zx_status_t status) {
+    EXPECT_EQ(status, ZX_ERR_PEER_CLOSED);
+    run_loop.Quit();
+  });
   context().Unbind();
   run_loop.Run();
   EXPECT_FALSE(frame);

fuchsia/browser/message_port_impl.cc

@@ -113,8 +113,8 @@ MessagePortImpl::MessagePortImpl(mojo::ScopedMessagePipeHandle mojo_port)
   connector_->set_connection_error_handler(
       base::BindOnce(&MessagePortImpl::OnDisconnected, base::Unretained(this)));
   binding_.set_error_handler([this](zx_status_t status) {
-    if (status != ZX_OK && status != ZX_ERR_PEER_CLOSED)
-      ZX_DLOG(INFO, status) << "Disconnected";
+    ZX_LOG_IF(ERROR, status != ZX_ERR_PEER_CLOSED, status)
+        << " MessagePort disconnected.";
 
     OnDisconnected();
   });

fuchsia/browser/webrunner_browser_main_parts.cc

@@ -8,6 +8,8 @@
 
 #include "base/command_line.h"
 #include "base/files/file_util.h"
+#include "base/fuchsia/fuchsia_logging.h"
+#include "base/logging.h"
 #include "content/public/browser/render_frame_host.h"
 #include "fuchsia/browser/context_impl.h"
 #include "fuchsia/browser/webrunner_browser_context.h"
@@ -52,7 +54,8 @@ void WebRunnerBrowserMainParts::PreMainMessageLoopRun() {
 
   // Quit the browser main loop when the Context connection is dropped.
   context_binding_->set_error_handler([this](zx_status_t status) {
-    DLOG(WARNING) << "Client connection to Context service dropped.";
+    ZX_LOG_IF(ERROR, status != ZX_ERR_PEER_CLOSED, status)
+        << " Context disconnected.";
     context_service_.reset();
     std::move(quit_closure_).Run();
   });

fuchsia/common/named_message_port_connector_browsertest.cc

@@ -111,7 +111,8 @@ IN_PROC_BROWSER_TEST_F(NamedMessagePortConnectorTest,
   // Ensure that the MessagePort is dropped when navigating away.
   {
     base::RunLoop run_loop;
-    (*message_port).set_error_handler([&run_loop](zx_status_t) {
+    (*message_port).set_error_handler([&run_loop](zx_status_t status) {
+      EXPECT_EQ(status, ZX_ERR_PEER_CLOSED);
       run_loop.Quit();
     });
     controller->LoadUrl("about:blank", nullptr);

fuchsia/http/url_loader_impl.cc

@@ -122,7 +122,11 @@ URLLoaderImpl::URLLoaderImpl(std::unique_ptr<net::URLRequestContext> context,
       context_(std::move(context)),
       buffer_(new net::GrowableIOBuffer()),
       write_watch_(FROM_HERE) {
-  binding_.set_error_handler([this](zx_status_t status) { delete this; });
+  binding_.set_error_handler([this](zx_status_t status) {
+    ZX_LOG_IF(ERROR, status != ZX_ERR_PEER_CLOSED, status)
+        << " URLLoader disconnected.";
+    delete this;
+  });
   g_active_requests++;
 }
 

fuchsia/runners/cast/cast_channel_bindings.cc

@@ -35,7 +35,7 @@ CastChannelBindings::CastChannelBindings(
   DCHECK(frame_);
 
   channel_consumer_.set_error_handler([this](zx_status_t status) mutable {
-    ZX_LOG(ERROR, status) << "CastChannel error:";
+    ZX_LOG(ERROR, status) << " Agent disconnected.";
     std::move(on_error_closure_).Run();
   });
 

fuchsia/runners/common/web_component.cc

@@ -9,6 +9,7 @@
 #include <lib/fit/function.h>
 #include <utility>
 
+#include "base/fuchsia/fuchsia_logging.h"
 #include "base/fuchsia/scoped_service_binding.h"
 #include "base/fuchsia/service_directory.h"
 #include "base/logging.h"
@@ -57,7 +58,10 @@ WebComponent::WebComponent(
   if (controller_request.is_valid()) {
     controller_binding_.Bind(std::move(controller_request));
     controller_binding_.set_error_handler([this](zx_status_t status) {
-      // Signal graceful process termination.
+      ZX_LOG_IF(ERROR, status != ZX_ERR_PEER_CLOSED, status)
+          << " ComponentController disconnected.";
+      // Teardown the component with dummy values, since ComponentController
+      // channel isn't there to receive them.
       DestroyComponent(0, fuchsia::sys::TerminationReason::EXITED);
     });
   }

fuchsia/service/context_provider_impl_unittest.cc

@@ -82,8 +82,10 @@ MULTIPROCESS_TEST_MAIN(SpawnContextServer) {
 
   // Quit the process when the context is destroyed.
   base::RunLoop run_loop;
-  context_binding.set_error_handler(
-      [&run_loop](zx_status_t status) { run_loop.Quit(); });
+  context_binding.set_error_handler([&run_loop](zx_status_t status) {
+    EXPECT_EQ(status, ZX_ERR_PEER_CLOSED);
+    run_loop.Quit();
+  });
   run_loop.Run();
 
   return 0;
@@ -111,12 +113,14 @@ class ContextProviderImplTest : public base::MultiProcessTest {
     // Call a Context method and wait for it to invoke an observer call.
     base::RunLoop run_loop;
     context->set_error_handler([&run_loop](zx_status_t status) {
+      ZX_LOG(ERROR, status) << " Context lost.";
       ADD_FAILURE();
       run_loop.Quit();
     });
 
     chromium::web::FramePtr frame_ptr;
     frame_ptr.set_error_handler([&run_loop](zx_status_t status) {
+      ZX_LOG(ERROR, status) << " Frame lost.";
       ADD_FAILURE();
       run_loop.Quit();
     });
@@ -151,8 +155,10 @@ class ContextProviderImplTest : public base::MultiProcessTest {
   void CheckContextUnresponsive(
       fidl::InterfacePtr<chromium::web::Context>* context) {
     base::RunLoop run_loop;
-    context->set_error_handler(
-        [&run_loop](zx_status_t status) { run_loop.Quit(); });
+    context->set_error_handler([&run_loop](zx_status_t status) {
+      EXPECT_EQ(status, ZX_ERR_PEER_CLOSED);
+      run_loop.Quit();
+    });
 
     chromium::web::FramePtr frame;
     (*context)->CreateFrame(frame.NewRequest());

fuchsia/test/fake_context.cc

@@ -4,13 +4,17 @@
 
 #include "fuchsia/test/fake_context.h"
 
+#include "base/fuchsia/fuchsia_logging.h"
 #include "base/logging.h"
 
 namespace webrunner {
 
 FakeFrame::FakeFrame(fidl::InterfaceRequest<chromium::web::Frame> request)
     : binding_(this, std::move(request)) {
-  binding_.set_error_handler([this](zx_status_t status) { delete this; });
+  binding_.set_error_handler([this](zx_status_t status) {
+    ZX_CHECK(status == ZX_ERR_PEER_CLOSED, status);
+    delete this;
+  });
 }
 
 FakeFrame::~FakeFrame() = default;

net/base/network_change_notifier_fuchsia.cc

@@ -52,7 +52,7 @@ NetworkChangeNotifierFuchsia::NetworkChangeNotifierFuchsia(
   DCHECK(netstack_);
 
   netstack_.set_error_handler([](zx_status_t status) {
-    ZX_LOG(ERROR, status) << "Lost connection to netstack.";
+    ZX_LOG(FATAL, status) << "Lost connection to netstack.";
   });
   netstack_.events().OnInterfacesChanged =
       [this](std::vector<fuchsia::netstack::NetInterface> interfaces) {

ui/base/ime/fuchsia/input_method_keyboard_controller_fuchsia.cc

@@ -20,13 +20,8 @@ InputMethodKeyboardControllerFuchsia::InputMethodKeyboardControllerFuchsia(
               ->ConnectToService<fuchsia::ui::input::ImeVisibilityService>()) {
   DCHECK(ime_service_);
 
-  ime_visibility_.set_error_handler([this](zx_status_t status) {
-    ZX_LOG(WARNING, status) << "ImeVisibilityService connection lost.";
-
-    // We can't observe visibility events anymore, so dismiss the keyboard and
-    // assume that it's closed for good.
-    DismissVirtualKeyboard();
-    keyboard_visible_ = false;
+  ime_visibility_.set_error_handler([](zx_status_t status) {
+    ZX_LOG(FATAL, status) << " ImeVisibilityService lost.";
   });
 
   ime_visibility_.events().OnKeyboardVisibilityChanged = [this](bool visible) {

ui/ozone/platform/scenic/scenic_window_manager.cc

@@ -5,6 +5,7 @@
 #include "ui/ozone/platform/scenic/scenic_window_manager.h"
 
 #include "base/fuchsia/component_context.h"
+#include "base/fuchsia/fuchsia_logging.h"
 #include "ui/ozone/platform/scenic/ozone_platform_scenic.h"
 
 namespace ui {
@@ -24,9 +25,7 @@ fuchsia::ui::viewsv1::ViewManager* ScenicWindowManager::GetViewManager() {
     view_manager_ = base::fuchsia::ComponentContext::GetDefault()
                         ->ConnectToService<fuchsia::ui::viewsv1::ViewManager>();
     view_manager_.set_error_handler([](zx_status_t status) {
-      LOG(ERROR)
-          << "The ViewManager channel was unexpectedly terminated with status "
-          << status << ".";
+      ZX_LOG(FATAL, status) << " ViewManager lost.";
     });
   }
 
@@ -37,9 +36,8 @@ fuchsia::ui::scenic::Scenic* ScenicWindowManager::GetScenic() {
   if (!scenic_) {
     scenic_ = base::fuchsia::ComponentContext::GetDefault()
                   ->ConnectToService<fuchsia::ui::scenic::Scenic>();
-    scenic_.set_error_handler([](zx_status_t status) {
-      LOG(ERROR) << "The Scenic channel was unexpectedly terminated.";
-    });
+    scenic_.set_error_handler(
+        [](zx_status_t status) { ZX_LOG(FATAL, status) << " Scenic lost."; });
   }
   return scenic_.get();
 }