fixup! Implement ewk_settings_auto_fitting_set

chromium-efl has a mechanism to inform the embedding
app about a blocked window opening. It is the "popup,blocked"
smart signal.
In [1], we removed the EWK specific preference that used to
control window opening by javascript, in favor of chromium's
WebPreferences::javascript_can_open_windows_automatically.

Problem is that this changed the way chromium-efl blocks popups.
When javascript_can_open_windows_automatically is set, it gets
bubbled down to Blink, and popup blocking happens in Blink level
(see third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp,
LocalDOMWindow::open() method).
This makes it impossible to inform the embedding app about a blocked
popup.

Additionally, [1] also started to cache a RenderView instance
(see ContentRendererClientEfl::RenderViewCreated) in order to
query for javascript_can_open_windows_automatically.
This is crash-prone because it caches the latest RenderView
instance created. If the for instance window associated to the
RenderView gets closed, ContentRendererClientEfl will hold a dangling
pointer, and crash upon accessing it.

Patch fixes this by reimplementing the way we control window
opening by javascript. Now
WebPreferences::javascript_can_open_windows_automatically (the chromium
preference) is always ON, so new window creation request messages
always reach the browser process. From there, we can control it
according to the EWK specific setting, and emit the
"popup,blocked" smart signal if needed.

The following EWK unit test were crashing due to this:
- utc_blink_ewk_settings_scripts_can_open_windows_set
Tests were also changed based on the default value of
javascript_can_open_windows_automatically_ewk, which is true.

[1] http://165.213.202.130/gerrit/#/c/92971/

Bug: http://web.sec.samsung.net/bugzilla/show_bug.cgi?id=14640

Reviewed by: a.renevier, djmix.kim, g.czajkowski

Change-Id: I3c5e45961265d78c0832387586c4f2aa103f398a
Signed-off-by: Antonio Gomes <a1.gomes@samsung.com>

diff --git a/tizen_src/ewk/efl_integration/common/render_messages_ewk.h b/tizen_src/ewk/efl_integration/common/render_messages_ewk.h
index c50c7b1..78b8ab1 100644 (file)
--- a/tizen_src/ewk/efl_integration/common/render_messages_ewk.h
+++ b/tizen_src/ewk/efl_integration/common/render_messages_ewk.h
@@ -36,6 +36,7 @@ IPC_STRUCT_TRAITS_END()
 
 IPC_STRUCT_TRAITS_BEGIN(WebPreferencesEfl)
   IPC_STRUCT_TRAITS_MEMBER(shrinks_viewport_content_to_fit)
+  IPC_STRUCT_TRAITS_MEMBER(javascript_can_open_windows_automatically_ewk)
 IPC_STRUCT_TRAITS_END()
 
 IPC_STRUCT_TRAITS_BEGIN(ErrorParams)

diff --git a/tizen_src/ewk/efl_integration/common/web_preferences_efl.h b/tizen_src/ewk/efl_integration/common/web_preferences_efl.h
index 67bf49d..e875ea4 100644 (file)
--- a/tizen_src/ewk/efl_integration/common/web_preferences_efl.h
+++ b/tizen_src/ewk/efl_integration/common/web_preferences_efl.h
@@ -14,6 +14,7 @@ struct WebPreferencesEfl {
 #else
       false;
 #endif
+  bool javascript_can_open_windows_automatically_ewk = true;
 };
 
 #endif // WEB_PREFERENCES_EFL_H

diff --git a/tizen_src/ewk/efl_integration/public/ewk_settings.cc b/tizen_src/ewk/efl_integration/public/ewk_settings.cc
index 300b2da..d8dd663 100644 (file)
--- a/tizen_src/ewk/efl_integration/public/ewk_settings.cc
+++ b/tizen_src/ewk/efl_integration/public/ewk_settings.cc
@@ -615,7 +615,7 @@ Eina_Bool ewk_settings_default_text_encoding_name_set(Ewk_Settings* settings, co
 Eina_Bool ewk_settings_scripts_can_open_windows_set(Ewk_Settings* settings, Eina_Bool enable)
 {
   EINA_SAFETY_ON_NULL_RETURN_VAL(settings, false);
-  settings->getPreferences().javascript_can_open_windows_automatically = enable;
+  settings->getPreferencesEfl().javascript_can_open_windows_automatically_ewk = enable;
   ewkUpdateWebkitPreferences(settings->getEvasObject());
   return true;
 }
@@ -623,7 +623,7 @@ Eina_Bool ewk_settings_scripts_can_open_windows_set(Ewk_Settings* settings, Eina
 Eina_Bool ewk_settings_scripts_can_open_windows_get(const Ewk_Settings* settings)
 {
   EINA_SAFETY_ON_NULL_RETURN_VAL(settings, false);
-  return settings->getPreferences().javascript_can_open_windows_automatically;
+  return settings->getPreferencesEfl().javascript_can_open_windows_automatically_ewk;
 }
 
 Eina_Bool ewk_settings_encoding_detector_enabled_set(Ewk_Settings* settings, Eina_Bool enable)

diff --git a/tizen_src/ewk/efl_integration/renderer/content_renderer_client_efl.cc b/tizen_src/ewk/efl_integration/renderer/content_renderer_client_efl.cc
index 5959995..6956a63 100644 (file)
--- a/tizen_src/ewk/efl_integration/renderer/content_renderer_client_efl.cc
+++ b/tizen_src/ewk/efl_integration/renderer/content_renderer_client_efl.cc
@@ -82,7 +82,7 @@ class WrtUrlParseImpl : public content::WrtUrlParseBase {
 };
 
 ContentRendererClientEfl::ContentRendererClientEfl()
-    : render_view_(nullptr) {
+    : javascript_can_open_windows_(true) {
 }
 
 ContentRendererClientEfl::~ContentRendererClientEfl() {
@@ -144,7 +144,6 @@ void ContentRendererClientEfl::RenderViewCreated(content::RenderView* render_vie
 #if !defined(EWK_BRINGUP)
   render_view->SetWrtUrlParser(wrt_url_parser_.get());
 #endif
-  render_view_ = render_view;
 }
 
 bool ContentRendererClientEfl::OverrideCreatePlugin(
@@ -174,12 +173,6 @@ bool ContentRendererClientEfl::WillSendRequest(blink::WebFrame* frame,
   return true;
 }
 
-bool ContentRendererClientEfl::AllowPopup() {
-  return render_view_
-      ? render_view_->GetWebkitPreferences().javascript_can_open_windows_automatically
-      : false;
-}
-
 void ContentRendererClientEfl::WillReleaseScriptContext(blink::WebFrame* frame,
                                                         v8::Handle<v8::Context> context,
                                                         int world_id) {

diff --git a/tizen_src/ewk/efl_integration/renderer/content_renderer_client_efl.h b/tizen_src/ewk/efl_integration/renderer/content_renderer_client_efl.h
index f490b2b..b41e804 100644 (file)
--- a/tizen_src/ewk/efl_integration/renderer/content_renderer_client_efl.h
+++ b/tizen_src/ewk/efl_integration/renderer/content_renderer_client_efl.h
@@ -52,7 +52,15 @@ class ContentRendererClientEfl : public content::ContentRendererClient
                                 v8::Handle<v8::Context>,
                                 int world_id);
 
-  bool AllowPopup() override;
+  bool AllowPopup() override { return javascript_can_open_windows_; }
+
+  // This setter is an EFL extension, where we cache the preference
+  // that controls whether JS content is allowed to open new windows.
+  // Note that strictly speaking this cached preference is incorrect:
+  // Every 'view' holds a preference instance. So it is possible that
+  // different views objects hold different preference set tha another
+  // at a given time.
+  void SetAllowPopup(bool value) { javascript_can_open_windows_ = value; }
 
   bool WillSendRequest(blink::WebFrame* frame,
                        ui::PageTransition transition_type,
@@ -83,7 +91,7 @@ class ContentRendererClientEfl : public content::ContentRendererClient
   scoped_ptr<WrtUrlParseImpl> wrt_url_parser_;
   scoped_ptr<RenderProcessObserverEfl> render_process_observer_;
   scoped_ptr<visitedlink::VisitedLinkSlave> visited_link_slave_;
-  content::RenderView* render_view_;
+  bool javascript_can_open_windows_;
 };
 
 #endif

diff --git a/tizen_src/ewk/efl_integration/renderer/render_view_observer_efl.cc b/tizen_src/ewk/efl_integration/renderer/render_view_observer_efl.cc
index dc2bc79..a0d2f33 100644 (file)
--- a/tizen_src/ewk/efl_integration/renderer/render_view_observer_efl.cc
+++ b/tizen_src/ewk/efl_integration/renderer/render_view_observer_efl.cc
@@ -603,6 +603,10 @@ void RenderViewObserverEfl::OnUpdateWebKitPreferencesEfl(const WebPreferencesEfl
     view->settings()->setShrinksViewportContentToFit(web_preferences_efl.shrinks_viewport_content_to_fit);
     // and more if they exist in web_preferences_efl.
   }
+
+  DCHECK(renderer_client_);
+  static_cast<ContentRendererClientEfl*>(renderer_client_)->SetAllowPopup(
+      web_preferences_efl.javascript_can_open_windows_automatically_ewk);
 }
 
 void RenderViewObserverEfl::HandleTap(const blink::WebGestureEvent& event)

diff --git a/tizen_src/ewk/unittest/utc_blink_ewk_settings_scripts_can_open_windows_set_func.cpp b/tizen_src/ewk/unittest/utc_blink_ewk_settings_scripts_can_open_windows_set_func.cpp
index 7954a74..0e94d9a 100644 (file)
--- a/tizen_src/ewk/unittest/utc_blink_ewk_settings_scripts_can_open_windows_set_func.cpp
+++ b/tizen_src/ewk/unittest/utc_blink_ewk_settings_scripts_can_open_windows_set_func.cpp
@@ -16,7 +16,7 @@ protected:
     settings = ewk_view_settings_get(GetEwkWebView());
     ASSERT_TRUE(settings);
     // make sure default value is proper
-    ASSERT_EQ(EINA_FALSE, ewk_settings_scripts_can_open_windows_get(settings));
+    ASSERT_EQ(EINA_TRUE, ewk_settings_scripts_can_open_windows_get(settings));
 
     evas_object_smart_callback_add(GetEwkWebView(), "popup,blocked", ToSmartCallback(popup_blocked_cb), this);
     evas_object_smart_callback_add(GetEwkWebView(), "create,window", ToSmartCallback(create_window_cb), this);
@@ -56,22 +56,22 @@ TEST_F(utc_blink_ewk_settings_scripts_can_open_windows_set, SetTrue)
                       "</html>";
 
   ASSERT_EQ(EINA_TRUE, ewk_view_html_string_load(GetEwkWebView(), htmlBuffer, NULL, NULL));
-  ASSERT_EQ(Success, EventLoopStart());
+  ASSERT_EQ(Failure, EventLoopStart());
 
   // now toggle option
-  ASSERT_EQ(EINA_TRUE, ewk_settings_scripts_can_open_windows_set(settings, EINA_TRUE));
+  ASSERT_EQ(EINA_TRUE, ewk_settings_scripts_can_open_windows_set(settings, EINA_FALSE));
 
   // check if option was toggled
-  ASSERT_EQ(EINA_TRUE, ewk_settings_scripts_can_open_windows_get(settings));
+  ASSERT_EQ(EINA_FALSE, ewk_settings_scripts_can_open_windows_get(settings));
 
   // reload page and expect create window smart callback
   ASSERT_EQ(EINA_TRUE, ewk_view_html_string_load(GetEwkWebView(), htmlBuffer, NULL, NULL));
-  ASSERT_EQ(Failure, EventLoopStart());
+  ASSERT_EQ(Success, EventLoopStart());
 }
 
 TEST_F(utc_blink_ewk_settings_scripts_can_open_windows_set, ToggleBeforeLoad)
 {
-  // Got feedback, that whem property is modified before loading anything to the view
+  // Got feedback, that when property is modified before loading anything to the view
   // it may not work. This TC checks such case and tests provided solution
 
   // toggle option