projects / platform / framework / web / chromium-efl.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 32fa684)
[M34-Merge] Attempt to gracefully handle renderer process crashes
authorPiotr Szawdyński <p.szawdynski@samsung.com>
Thu, 8 Jan 2015 16:39:55 +0000 (17:39 +0100)
committerYoungsoo Choi <kenshin.choi@samsung.com>
Tue, 10 Jul 2018 06:57:09 +0000 (06:57 +0000)
The current chromium-efl tries to handle renderer process crashes by
invoking "webprocess,crashed" EFL callback on the EWebView. The callback
is expected to set the value of event_info to true and delete the
EWebView instance for which the crash did happen. After the callback is
invoked the crashed EWebView would be in an unusable state. Using it
would immediately cause the UI process to crash.

The problem with the current approach is only efl_webview_app actually
implements the callback. Other applications like mini_browser,
com.samsung.browser or com.samsung.email do not even register it. The
end result is webprocess crashes can actually bring down the entire app.
This defeats the purpose of chromium multi-process architecture.

This patch solves the problem by re-creating WebContents after the
renderer crash. The "webprocess,crashed" is now only used as a
notification mechanism. The embedder handling it is only responsible for
informing the user about the fact and possibly loading another page. If
it does not handle the callback the engine will show a simple error
page. The behaviour aligns chromium-efl with WebKitEfl behaviour.

Original patches:
1) "Attempt to gracefully handle renderer process crashes" by Piotr Tworek
   http://165.213.202.130:8080/#/c/69203
2) "Handle rendenrer crashes in efl_webview_app properly" by Piotr Tworek
   http://165.213.202.130:8080/#/c/69448

Bug: http://107.108.218.239/bugzilla/show_bug.cgi?id=10194
Reviewed by: Janusz Majnert, Min-Soo Koo

Change-Id: I52407fbcd672f7726fb34bb83e911e6a784a149c
Signed-off-by: Piotr Szawdyński <p.szawdynski@samsung.com>
tizen_src/ewk/efl_webview_app/app.c patch | blob | history
tizen_src/impl/browser/renderer_host/render_widget_host_view_efl.cc [changed mode: 0755->0644] patch | blob | history
tizen_src/impl/eweb_view.cc patch | blob | history
tizen_src/impl/eweb_view.h patch | blob | history
tizen_src/impl/web_contents_delegate_efl.cc patch | blob | history

diff --git a/tizen_src/ewk/efl_webview_app/app.c b/tizen_src/ewk/efl_webview_app/app.c
index 7fe08fd..547d5ce 100644 (file)
--- a/tizen_src/ewk/efl_webview_app/app.c
+++ b/tizen_src/ewk/efl_webview_app/app.c
@@ -1462,12 +1462,11 @@ Eina_Bool __ewk_view_javascript_prompt_cb(Evas_Object* o, const char* message, c
 void __webprocess_crashed_cb(void* data, Evas_Object* obj, void* event_info)
 {
   printf("APP.C callback __webprocess_crashed_cb \n");
+  const char* message = "<html><body><h1>The renderer process has crashed!</h1>"
+      "<a href=\"https://google.com\">Open google</a></body></html>";
+  ewk_view_html_string_load(view, message, 0, 0);
   Eina_Bool* handled = (Eina_Bool*)event_info;
   *handled = EINA_TRUE;
-  // EWebView (Evas_Object) on which this callback is triggered should be deleted immediately.
-  // It is not safe to call any method on it.
-  evas_object_del(obj);
-  exit(EXIT_FAILURE);
 }
 
 Eina_Bool __mime_override_cb(const char* url, const char *mime, char **new_mime)
diff --git a/tizen_src/impl/browser/renderer_host/render_widget_host_view_efl.cc b/tizen_src/impl/browser/renderer_host/render_widget_host_view_efl.cc
old mode 100755 (executable)
new mode 100644 (file)
index b7d78bf..b27c5d6
--- a/tizen_src/impl/browser/renderer_host/render_widget_host_view_efl.cc
+++ b/tizen_src/impl/browser/renderer_host/render_widget_host_view_efl.cc
@@ -1072,12 +1072,6 @@ void RenderWidgetHostViewEfl::ResizeCompositingSurface(const gfx::Size& size) {
 }
 
 void RenderWidgetHostViewEfl::RenderProcessGone(base::TerminationStatus, int error_code) {
-  // RenderWidgetHostImpl sets |view_| i.e. RenderWidgetHostViewEfl to NULL immediately after this call.
-  // It expects RenderWidgetHostView to delete itself.
-  // We only inform |web_view_| that renderer has crashed.
-  // and in "process,crashed" callback, app is expected to delete the view.
-  if (web_view_)
-    web_view_->set_renderer_crashed();
   Destroy();
 }
 
diff --git a/tizen_src/impl/eweb_view.cc b/tizen_src/impl/eweb_view.cc
index e9ef75b..99ed022 100644 (file)
--- a/tizen_src/impl/eweb_view.cc
+++ b/tizen_src/impl/eweb_view.cc
@@ -83,6 +83,9 @@ namespace {
 
 int screen_orientation_ = 0;
 
+static const char* kRendererCrashedHTMLMessage =
+    "<html><body><h1>Renderer process has crashed!</h1></body></html>";
+
 inline void SetDefaultStringIfNull(const char*& variable,
                                    const char* default_string) {
   if (!variable) {
@@ -192,13 +195,6 @@ RenderWidgetHostViewEfl* EWebView::rwhv() const {
   return static_cast<RenderWidgetHostViewEfl*>(web_contents_->GetRenderWidgetHostView());
 }
 
-void EWebView::set_renderer_crashed() {
-#ifndef NDEBUG
-  DCHECK(!renderer_crashed_);
-  renderer_crashed_ = true;
-#endif
-}
-
 EWebView::EWebView(tizen_webview::WebView* owner, tizen_webview::WebContext* context, Evas_Object* object)
     : public_webview_(owner),
       evas_event_handler_(NULL),
@@ -214,9 +210,6 @@ EWebView::EWebView(tizen_webview::WebView* owner, tizen_webview::WebContext* con
       min_page_scale_factor_(-1.0),
       max_page_scale_factor_(-1.0),
       inspector_server_(NULL),
-#ifndef NDEBUG
-      renderer_crashed_(false),
-#endif
       is_initialized_(false) {
 }
 
@@ -1832,6 +1825,21 @@ bool EWebView::StopInspectorServer() {
   return true;
 }
 
+void EWebView::HandleRendererProcessCrash() {
+  const char* last_url = GetURL();
+
+  WebContents::CreateParams params(context_->browser_context());
+  params.context = GetContentImageObject();
+  web_contents_.reset(WebContents::Create(params));
+  web_contents_delegate_.reset(new WebContentsDelegateEfl(this));
+  web_contents_->SetDelegate(web_contents_delegate_.get());
+
+  bool callback_handled = false;
+  SmartCallback<EWebViewCallbacks::WebProcessCrashed>().call(&callback_handled);
+  if (!callback_handled)
+    LoadHTMLString(kRendererCrashedHTMLMessage, NULL, last_url);
+}
+
 #if defined(OS_TIZEN_MOBILE) && !defined(EWK_BRINGUP)
 void EWebView::cameraResultCb(service_h request,
                               service_h reply,
diff --git a/tizen_src/impl/eweb_view.h b/tizen_src/impl/eweb_view.h
index 112e7bb..4045cc3 100644 (file)
--- a/tizen_src/impl/eweb_view.h
+++ b/tizen_src/impl/eweb_view.h
@@ -443,6 +443,8 @@ class EWebView {
   bool HandleGesture(ui::GestureEvent* event);
   bool HandleTouchEvent(ui::TouchEvent* event);
 
+  void HandleRendererProcessCrash();
+
  private:
 #if defined(OS_TIZEN_MOBILE) && !defined(EWK_BRINGUP)
   static void cameraResultCb(service_h request, service_h reply,
@@ -526,9 +528,6 @@ class EWebView {
   scoped_refptr<EdgeEffect> edge_effect_;
 #endif
 
-#ifndef NDEBUG
-  bool renderer_crashed_;
-#endif
 #if defined(OS_TIZEN_MOBILE)
   content::FileChooserParams::Mode filechooser_mode_;
 #endif
diff --git a/tizen_src/impl/web_contents_delegate_efl.cc b/tizen_src/impl/web_contents_delegate_efl.cc
index 5850619..473f5cf 100644 (file)
--- a/tizen_src/impl/web_contents_delegate_efl.cc
+++ b/tizen_src/impl/web_contents_delegate_efl.cc
@@ -546,10 +546,7 @@ void WebContentsDelegateEfl::RenderProcessGone(base::TerminationStatus status) {
   if (status == base::TERMINATION_STATUS_ABNORMAL_TERMINATION
       || status == base::TERMINATION_STATUS_PROCESS_WAS_KILLED
       || status == base::TERMINATION_STATUS_PROCESS_CRASHED) {
-    bool unused = false;
-    web_view_->SmartCallback<EWebViewCallbacks::WebProcessCrashed>().call(&unused);
-    // A sane app would handle the callback and delete the view immediately.
-    // Otherwise, we will most probably crash when a method is called on |web_view_|.
+    web_view_->HandleRendererProcessCrash();
   }
 }
 
Domain: Web Framework / Web Engine;