Fix issue with calling Render*::FromID outside UI thread

Problem : the entire hit test code was using some chromium functionality from the wrong thread

Issue: CBEFL-103, CBWEBCORE-471
Change-Id: I5e5ad931c45f10833f07da69d964d8a7aba86ebf

diff --git a/tizen_src/impl/browser/renderer_host/render_message_filter_efl.cc b/tizen_src/impl/browser/renderer_host/render_message_filter_efl.cc
index c37b5adad70edf8f2899bc36fecf37bb8a1f0433..f0f2aa21d60211602a3882592420a7618a112db0 100644 (file)
--- a/tizen_src/impl/browser/renderer_host/render_message_filter_efl.cc
+++ b/tizen_src/impl/browser/renderer_host/render_message_filter_efl.cc
@@ -40,15 +40,17 @@ RenderMessageFilterEfl::~RenderMessageFilterEfl() { }
 void RenderMessageFilterEfl::OverrideThreadForMessage(const IPC::Message& message,
                                                       content::BrowserThread::ID* thread)
 {
-  if(message.type() == EwkHostMsg_DecideNavigationPolicy::ID)
+  switch (message.type()) {
+  case EwkHostMsg_DecideNavigationPolicy::ID:
     *thread = content::BrowserThread::UI;
+    break;
+  }
 }
 
 bool RenderMessageFilterEfl::OnMessageReceived(const IPC::Message& message) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(RenderMessageFilterEfl, message)
     IPC_MESSAGE_HANDLER(EwkHostMsg_DecideNavigationPolicy, OnDecideNavigationPolicy)
-    IPC_MESSAGE_HANDLER(EwkViewHostMsg_HitTestReply, OnReceivedHitTestData)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
@@ -66,15 +68,3 @@ void RenderMessageFilterEfl::OnDecideNavigationPolicy(NavigationPolicyParams par
   }
 }
 
-void RenderMessageFilterEfl::OnReceivedHitTestData(int render_view,
-  const _Ewk_Hit_Test& hit_test_data,
-  const NodeAttributesMap& node_attributes) {
-  content::WebContents* web_contents = WebContentsFromViewID(
-      render_process_id_, render_view);
-
-  if (web_contents) {
-    content::WebContentsDelegateEfl* delegate =
-    static_cast<content::WebContentsDelegateEfl*>(web_contents->GetDelegate());
-    delegate->web_view()->UpdateHitTestData(hit_test_data, node_attributes);
-  }
-}

diff --git a/tizen_src/impl/content_browser_client_efl.cc b/tizen_src/impl/content_browser_client_efl.cc
index a67d887638280c515373ec782f2dda9fc0e22f58..3f838409af83b65131e52d3e1772c66c2043e17d 100644 (file)
--- a/tizen_src/impl/content_browser_client_efl.cc
+++ b/tizen_src/impl/content_browser_client_efl.cc
@@ -250,6 +250,10 @@ void ContentBrowserClientEfl::RenderProcessWillLaunch(
 
   BrowserContextEfl* browser_context = static_cast<BrowserContextEfl*>(
                                           host->GetBrowserContext());
+
+  // TODO: is this situation possible? Should it ever happen?
+  DCHECK(browser_context);
+
   if (browser_context)
     web_context_ = browser_context->WebContext();
 

diff --git a/tizen_src/impl/eweb_view.cc b/tizen_src/impl/eweb_view.cc
index 6350f8f3855552dba94814f9883f238467d138cd..52eb3b3960f20e514e7ab021842aee8398d9d12c 100755 (executable)
--- a/tizen_src/impl/eweb_view.cc
+++ b/tizen_src/impl/eweb_view.cc
@@ -41,6 +41,7 @@
 #include "content/browser/renderer_host/ui_events_helper.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/web_contents/web_contents_view.h"
+#include "content/public/browser/browser_message_filter.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/resource_dispatcher_host.h"
@@ -169,6 +170,54 @@ DEFINE_EVENT_HANDLER(EVAS_CALLBACK_KEY_UP, Evas_Event_Key_Up, key_up);
 
 } // namespace
 
+class WebViewBrowserMessageFilter: public content::BrowserMessageFilter {
+ public:
+  WebViewBrowserMessageFilter(EWebView* web_view)
+    : BrowserMessageFilter(ChromeMsgStart)
+    , web_view_(web_view) {
+    WebContents* web_contents = GetWebContents();
+    DCHECK(web_contents);
+
+    if (web_contents && web_contents->GetRenderProcessHost())
+      web_contents->GetRenderProcessHost()->AddFilter(this);
+  }
+
+  bool OnMessageReceived(const IPC::Message& message) {
+    bool handled = true;
+    IPC_BEGIN_MESSAGE_MAP(WebViewBrowserMessageFilter, message)
+      IPC_MESSAGE_HANDLER(EwkViewHostMsg_HitTestReply, OnReceivedHitTestData)
+      IPC_MESSAGE_UNHANDLED(handled = false)
+    IPC_END_MESSAGE_MAP()
+    return handled;
+  }
+
+ private:
+  void OnReceivedHitTestData(int render_view, const _Ewk_Hit_Test& hit_test_data,
+      const NodeAttributesMap& node_attributes) {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+    WebContents* contents = GetWebContents();
+    DCHECK(contents);
+
+    if (contents) {
+      RenderViewHost* render_view_host= contents->GetRenderViewHost();
+      DCHECK(render_view_host);
+
+      if (render_view_host && render_view_host->GetRoutingID() == render_view)
+        web_view_->UpdateHitTestData(hit_test_data, node_attributes);
+    }
+  }
+
+  WebContents* GetWebContents() const {
+    if (web_view_ && web_view_->web_contents_delegate())
+      return web_view_->web_contents_delegate()->web_contents();
+
+    return NULL;
+  }
+
+private:
+  EWebView* web_view_;
+};
+
 Evas_Smart_Class EWebView::parent_smart_class_ = EVAS_SMART_CLASS_INIT_NULL;
 
 WebContents* EWebView::contents_for_new_window_ = NULL;
@@ -309,6 +358,7 @@ EWebView::EWebView(tizen_webview::WebContext* context, Evas_Object* object)
 #ifdef TIZEN_EDGE_EFFECT
       , edge_effect_(EdgeEffect::create(object))
 #endif
+      , message_filter_(NULL)
 #ifndef NDEBUG
       ,renderer_crashed_(false)
 #endif
@@ -1569,17 +1619,30 @@ tizen_webview::Hit_Test* EWebView::RequestHitTestDataAt(int x, int y, tizen_webv
 }
 
 tizen_webview::Hit_Test* EWebView::RequestHitTestDataAtBlinkCoords(int x, int y, tizen_webview::Hit_Test_Mode mode) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  // WebViewBrowserMessageFilter requires RenderProcessHost to be already created.
+  // In EWebView constructor we have no guarantee that related RenderProcessHost is already created
+  // We do not destroy message_filter_ manualy as it is managed by RenderProcessHost after setting it as filter
+  // TODO: this pointer should be set to NULL when WebProcess crash/quits
+  if (!message_filter_) {
+    message_filter_ = new WebViewBrowserMessageFilter(this);
+  }
+
   RenderViewHost* render_view_host = web_contents_delegate()->web_contents()->GetRenderViewHost();
+  content::RenderProcessHost* render_process_host = web_contents_delegate()->web_contents()->GetRenderProcessHost();
   DCHECK(render_view_host);
+  DCHECK(render_process_host);
 
-  render_view_host->Send(new EwkViewMsg_DoHitTest(render_view_host->GetRoutingID(), x, y, mode));
-
-  {
+  if (render_view_host && render_process_host) {
     // We wait on UI thread till hit test data is updated.
     base::ThreadRestrictions::ScopedAllowWait allow_wait;
+    render_view_host->Send(new EwkViewMsg_DoHitTest(render_view_host->GetRoutingID(), x, y, mode));
     hit_test_completion_.Wait();
+    return new tizen_webview::Hit_Test(hit_test_data_);
   }
-  return new tizen_webview::Hit_Test(hit_test_data_);
+
+  return NULL;
 }
 
 void EWebView::UpdateHitTestData(const _Ewk_Hit_Test& hit_test_data, const NodeAttributesMap& node_attributes) {

diff --git a/tizen_src/impl/eweb_view.h b/tizen_src/impl/eweb_view.h
index b34b47e08fb82dd454cac125a65185b6dddd2a5c..8315702adae529ba98814b03b3e8684b52d1cf02 100755 (executable)
--- a/tizen_src/impl/eweb_view.h
+++ b/tizen_src/impl/eweb_view.h
@@ -192,6 +192,7 @@ class WebApplicationCapableGetCallback {
 };
 
 class JavaScriptDialogManagerEfl;
+class WebViewBrowserMessageFilter;
 
 class EWebView
     : public ui::GestureConsumer
@@ -517,6 +518,7 @@ class EWebView
 #ifdef TIZEN_EDGE_EFFECT
   scoped_refptr<EdgeEffect> edge_effect_;
 #endif
+  WebViewBrowserMessageFilter* message_filter_;
 
 #ifndef NDEBUG
   bool renderer_crashed_;