Bug 63013 – [EFL] Page Cache was not Enabled(Enable Page Cache)

• Status: RESOLVED
• Resolution: WORKSFORME
• Component: WebKit EFL

[DongJae KIM]

On EFL Port, Page Cache was disabled, 

bool FrameLoaderClientEfl::canCachePage() const
{
    return false;
}

[DongJae KIM]

Created attachment 98662 [details]
patch for efl port back/forward cache enable

Add Enable back/forward Cache code for efl port.

[WebKit Review Bot]

Attachment 98662 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/efl/ChangeLog', u'Source/Web..." exit_code: 1

Tools/ChangeLog:5:  Line contains tab character.  [whitespace/tab] [5]
Tools/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
Source/WebKit/efl/ChangeLog:5:  Line contains tab character.  [whitespace/tab] [5]
Source/WebKit/efl/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
Source/WebKit/efl/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
Source/WebKit/efl/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
Source/WebKit/efl/ChangeLog:10:  Line contains tab character.  [whitespace/tab] [5]
Source/WebKit/efl/ChangeLog:11:  Line contains tab character.  [whitespace/tab] [5]
Source/WebKit/efl/ChangeLog:12:  Line contains tab character.  [whitespace/tab] [5]
Source/WebKit/efl/ewk/ewk_view.cpp:4520:  Missing space before ( in if(  [whitespace/parens] [5]
Source/WebKit/efl/ewk/ewk_view.cpp:4522:  Missing space before ( in if(  [whitespace/parens] [5]
Source/WebKit/efl/ewk/ewk_view.cpp:4525:  Missing space before ( in if(  [whitespace/parens] [5]
Source/WebKit/efl/ewk/ewk_view.cpp:4527:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/WebKit/efl/WebCoreSupport/FrameLoaderClientEfl.cpp:606:  Should have a space between // and comment  [whitespace/comments] [4]
Source/WebKit/efl/WebCoreSupport/FrameLoaderClientEfl.cpp:607:  Missing space after ,  [whitespace/comma] [3]
Total errors found: 15 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gyuyoung Kim]

(In reply to comment #2)
> Attachment 98662 [details] did not pass style-queue:
> 
> Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/efl/ChangeLog', u'Source/Web..." exit_code: 1
> 
> Tools/ChangeLog:5:  Line contains tab character.  [whitespace/tab] [5]
> Tools/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
> Source/WebKit/efl/ChangeLog:5:  Line contains tab character.  [whitespace/tab] [5]
> Source/WebKit/efl/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
> Source/WebKit/efl/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
> Source/WebKit/efl/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
> Source/WebKit/efl/ChangeLog:10:  Line contains tab character.  [whitespace/tab] [5]
> Source/WebKit/efl/ChangeLog:11:  Line contains tab character.  [whitespace/tab] [5]
> Source/WebKit/efl/ChangeLog:12:  Line contains tab character.  [whitespace/tab] [5]
> Source/WebKit/efl/ewk/ewk_view.cpp:4520:  Missing space before ( in if(  [whitespace/parens] [5]
> Source/WebKit/efl/ewk/ewk_view.cpp:4522:  Missing space before ( in if(  [whitespace/parens] [5]
> Source/WebKit/efl/ewk/ewk_view.cpp:4525:  Missing space before ( in if(  [whitespace/parens] [5]
> Source/WebKit/efl/ewk/ewk_view.cpp:4527:  One line control clauses should not use braces.  [whitespace/braces] [4]
> Source/WebKit/efl/WebCoreSupport/FrameLoaderClientEfl.cpp:606:  Should have a space between // and comment  [whitespace/comments] [4]
> Source/WebKit/efl/WebCoreSupport/FrameLoaderClientEfl.cpp:607:  Missing space after ,  [whitespace/comma] [3]
> Total errors found: 15 in 6 files
> 
> 
> If any of these errors are false positives, please file a bug against check-webkit-style.

You SHOULD  run your patch with *Tools/Scripts/check-webkit-style* before uploading your patch.

[Gyuyoung Kim]

Comment on attachment 98662 [details]
patch for efl port back/forward cache enable

View in context: https://bugs.webkit.org/attachment.cgi?id=98662&action=review

Informal review - on my side.

> Source/WebKit/efl/ewk/ewk_view.cpp:61
> +#include "HistoryItem.h"

Move above line to  #include "HistoryItem.h" as below,

37 #include "GraphicsContext.h"
38 #include "HistoryItem.h"
39 #include "HTMLElement.h"

> Source/WebKit/efl/ewk/ewk_view.cpp:4512
> +void ewk_view_frame_restore_cachedpage(Evas_Object* o, Evas_Object* frame)

I think ewk_view_cached_page_restore is better.

[Ryuan Choi]

Great, it's what I wish for a long time.
I commented small things in addition to gyuyoung's comment.

View in context: https://bugs.webkit.org/attachment.cgi?id=98662&action=review

> Source/WebKit/efl/WebCoreSupport/FrameLoaderClientEfl.cpp:607
> +    //This method must be called before WebCore::FrameView::layout()
> +    ewk_view_frame_restore_cachedpage(m_view,m_frame);

m_view, m_frame);
And is this comment necessary?

> Source/WebKit/efl/ewk/ewk_private.h:181
> +void ewk_view_frame_restore_cachedpage(Evas_Object* o, Evas_Object* frame);
> +

How do you think about moving it below of ewk_view_frame_create ?

> Source/WebKit/efl/ewk/ewk_view.cpp:4517
> +    WebCore::Frame* main_frame = ewk_frame_core_get(frame);

I think that sd already know main frame.

> Source/WebKit/efl/ewk/ewk_view.cpp:4521
> +    WebCore::HistoryItem* item = main_frame->loader()->history()->currentItem();
> +
> +    if(!main_frame)
> +        return;

It checked after using main_frame.

> Tools/EWebLauncher/main.c:279
> -    ewk_view_fixed_layout_size_set(app->browser, app->viewport.w, app->viewport.h);
> -    ewk_view_zoom_set(app->browser, app->viewport.initScale, 0, 0);
>      if (!ewk_view_zoom_range_set(app->browser, app->viewport.minScale, app->viewport.maxScale))
>          info(" Fail to set zoom range. minScale = %f, maxScale = %f\n", app->viewport.minScale, app->viewport.maxScale);
>      ewk_view_user_scalable_set(app->browser, app->viewport.userScalable);
> +
> +    ewk_view_fixed_layout_size_set(app->browser, app->viewport.w, app->viewport.h);
> +    ewk_view_zoom_set(app->browser, app->viewport.initScale, 0, 0);
> +

Can I know why you moved?

[Gyuyoung Kim]

Comment on attachment 98662 [details]
patch for efl port back/forward cache enable

View in context: https://bugs.webkit.org/attachment.cgi?id=98662&action=review

> Source/WebKit/efl/ewk/ewk_frame.cpp:-1906
> -        return;

Why do you remove this code ?

[DongJae KIM]

(In reply to comment #4)
> (From update of attachment 98662 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=98662&action=review
> 
> Informal review - on my side.
> 
> > Source/WebKit/efl/ewk/ewk_view.cpp:61
> > +#include "HistoryItem.h"
> 
> Move above line to  #include "HistoryItem.h" as below,
> 
> 37 #include "GraphicsContext.h"
> 38 #include "HistoryItem.h"
> 39 #include "HTMLElement.h"
> 
> > Source/WebKit/efl/ewk/ewk_view.cpp:4512
> > +void ewk_view_frame_restore_cachedpage(Evas_Object* o, Evas_Object* frame)
> 
> I think ewk_view_cached_page_restore is better.

I agree you comment. it will be changed.

[DongJae KIM]

(In reply to comment #6)
> (From update of attachment 98662 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=98662&action=review
> 
> > Source/WebKit/efl/ewk/ewk_frame.cpp:-1906
> > -        return;
> 
> Why do you remove this code ?

when user add 4th cached page, first cached page will be destroy. 
then frame was released and has null reference assert.

so sd->frame->view() was create always.

test case ::
1. add 3 cached page
2. add one more cached page
3. scroll page
4. assert 


Program received signal SIGSEGV, Segmentation fault.
0x00b671ed in WebCore::EventHandler::setFrameWasScrolledByUser (this=0x4f0)
    at /home/dongjae/Projects/WebKit_EFL/Source/WebCore/page/EventHandler.cpp:2997
2997	    FrameView* v = m_frame->view();
(gdb) bt
#0  0x00b671ed in WebCore::EventHandler::setFrameWasScrolledByUser (this=0x4f0)
    at /home/dongjae/Projects/WebKit_EFL/Source/WebCore/page/EventHandler.cpp:2997
#1  0x00b6714d in WebCore::EventHandler::sendScrollEvent (this=0x4f0)
    at /home/dongjae/Projects/WebKit_EFL/Source/WebCore/page/EventHandler.cpp:2990
#2  0x00b7af83 in WebCore::FrameView::scrollPositionChanged (this=0x80b98e0)
    at /home/dongjae/Projects/WebKit_EFL/Source/WebCore/page/FrameView.cpp:1468
#3  0x00b7d3ff in WebCore::FrameView::scrollTo (this=0x80b98e0, newOffset=...)
    at /home/dongjae/Projects/WebKit_EFL/Source/WebCore/page/FrameView.cpp:2110
#4  0x00bd65a7 in WebCore::ScrollView::setScrollOffset (this=0x80b98e0, offset=...)
    at /home/dongjae/Projects/WebKit_EFL/Source/WebCore/platform/ScrollView.cpp:351
#5  0x00bd2592 in WebCore::ScrollableArea::setScrollOffsetFromAnimation (this=0x80b990c, offset=...)
    at /home/dongjae/Projects/WebKit_EFL/Source/WebCore/platform/ScrollableArea.cpp:133
#6  0x0127e999 in WebCore::ScrollAnimator::notityPositionChanged (this=0x80935e0)
    at /home/dongjae/Projects/WebKit_EFL/Source/WebCore/platform/ScrollAnimator.cpp:129
#7  0x0127e4a9 in WebCore::ScrollAnimator::scroll (this=0x80935e0, orientation=WebCore::VerticalScrollbar, step=1, multiplier=40)
    at /home/dongjae/Projects/WebKit_EFL/Source/WebCore/platform/ScrollAnimator.cpp:70
#8  0x0127e8a4 in WebCore::ScrollAnimator::handleWheelEvent (this=0x80935e0, e=...)
    at /home/dongjae/Projects/WebKit_EFL/Source/WebCore/platform/ScrollAnimator.cpp:110
#9  0x00bd2563 in WebCore::ScrollableArea::handleWheelEvent (this=0x80b990c, wheelEvent=...)
    at /home/dongjae/Projects/WebKit_EFL/Source/WebCore/platform/ScrollableArea.cpp:120
#10 0x00bd8558 in WebCore::ScrollView::wheelEvent (this=0x80b98e0, e=...)
    at /home/dongjae/Projects/WebKit_EFL/Source/WebCore/platform/ScrollView.cpp:800
#11 0x00b6437a in WebCore::EventHandler::handleWheelEvent (this=0x808de18, e=...)
    at /home/dongjae/Projects/WebKit_EFL/Source/WebCore/page/EventHandler.cpp:2138
#12 0x008b0d3a in ewk_frame_feed_mouse_wheel (o=0x80b8fc0, ev=0xbfffdfc0)
    at /home/dongjae/Projects/WebKit_EFL/Source/WebKit/efl/ewk/ewk_frame.cpp:1405
#13 0x008c57cf in _ewk_view_smart_mouse_wheel (sd=0x808f338, ev=0xbfffdfc0)
    at /home/dongjae/Projects/WebKit_EFL/Source/WebKit/efl/ewk/ewk_view.cpp:376
#14 0x008c5e95 in _ewk_view_on_mouse_wheel (data=0x808f338, e=0x80a2f28, o=0x808f1f8, event_info=0xbfffdfc0)
    at /home/dongjae/Projects/WebKit_EFL/Source/WebKit/efl/ewk/ewk_view.cpp:474
#15 0x0021d4a2 in evas_object_event_callback_call (obj=0x808f1f8, type=EVAS_CALLBACK_MOUSE_WHEEL, event_info=0xbfffdfc0)
    at evas_callbacks.c:222
#16 0x0021d57f in evas_object_event_callback_call (obj=0x80b8ea0, type=EVAS_CALLBACK_MOUSE_WHEEL, event_info=0xbfffdfc0)
    at evas_callbacks.c:251
#17 0x002203d5 in evas_event_feed_mouse_wheel (e=0x80a2f28, direction=0, z=1, timestamp=9067795, data=0x0) at evas_events.c:599
#18 0x035f2bd3 in ecore_event_evas_mouse_wheel (data=0x0, type=17, event=0x8b34c50) at ecore_input_evas.c:310
#19 0x002c12d9 in _ecore_event_call () at ecore_events.c:641
#20 0x002c67f7 in _ecore_main_loop_iterate_internal (once_only=0) at ecore_main.c:1518
#21 0x002c69f7 in ecore_main_loop_begin () at ecore_main.c:663
#22 0x0804c7c7 in main (argc=2, argv=0xbffff254) at /home/dongjae/Projects/WebKit_EFL/Tools/EWebLauncher/main.c:890

[DongJae KIM]

(In reply to comment #5)
> Great, it's what I wish for a long time.
> I commented small things in addition to gyuyoung's comment.
> 
> View in context: https://bugs.webkit.org/attachment.cgi?id=98662&action=review
> 
> > Source/WebKit/efl/WebCoreSupport/FrameLoaderClientEfl.cpp:607
> > +    //This method must be called before WebCore::FrameView::layout()
> > +    ewk_view_frame_restore_cachedpage(m_view,m_frame);
> 
> m_view, m_frame);
> And is this comment necessary?

ok, I will delete it.
 
> > Source/WebKit/efl/ewk/ewk_private.h:181
> > +void ewk_view_frame_restore_cachedpage(Evas_Object* o, Evas_Object* frame);
> > +
> 
> How do you think about moving it below of ewk_view_frame_create ?
> 

Please tell me why? there are any rules?

> > Source/WebKit/efl/ewk/ewk_view.cpp:4517
> > +    WebCore::Frame* main_frame = ewk_frame_core_get(frame);
> 
> I think that sd already know main frame.
>
ok I will be change it(using sd->main_frame)

> > Source/WebKit/efl/ewk/ewk_view.cpp:4521
> > +    WebCore::HistoryItem* item = main_frame->loader()->history()->currentItem();
> > +
> > +    if(!main_frame)
> > +        return;
> 
> It checked after using main_frame.
> 
Ok, I will change it.

> > Tools/EWebLauncher/main.c:279
> > -    ewk_view_fixed_layout_size_set(app->browser, app->viewport.w, app->viewport.h);
> > -    ewk_view_zoom_set(app->browser, app->viewport.initScale, 0, 0);
> >      if (!ewk_view_zoom_range_set(app->browser, app->viewport.minScale, app->viewport.maxScale))
> >          info(" Fail to set zoom range. minScale = %f, maxScale = %f\n", app->viewport.minScale, app->viewport.maxScale);
> >      ewk_view_user_scalable_set(app->browser, app->viewport.userScalable);
> > +
> > +    ewk_view_fixed_layout_size_set(app->browser, app->viewport.w, app->viewport.h);
> > +    ewk_view_zoom_set(app->browser, app->viewport.initScale, 0, 0);
> > +
> 
> Can I know why you moved?

when view port changed, zoom level was set. ewk_view_zoom_set check zoom range. 
but zoom range value was old one.

so i changed setting sequence.

[Gyuyoung Kim]

(In reply to comment #8)

> when user add 4th cached page, first cached page will be destroy. 
> then frame was released and has null reference assert.
> 
> so sd->frame->view() was create always.

If we don't use cached page or frame is not null, I think we need to check the condition. If frame already has a view instance, we don't need to create new view. Please consider both normal case and your case.

[Ryuan Choi]

(In reply to comment #9)
> (In reply to comment #5)
> > > Source/WebKit/efl/ewk/ewk_private.h:181
> > > +void ewk_view_frame_restore_cachedpage(Evas_Object* o, Evas_Object* frame);
> > > +
> > 
> > How do you think about moving it below of ewk_view_frame_create ?
> > 
> 
> Please tell me why? there are any rules?

At least, I don't know rules.
But, I think that we'd better to split APIs related ewk_view.cpp and ewk_frame.cpp and so on.

IMO, we can refactor this file more readable.

[Ryuan Choi]

(In reply to comment #9)
> (In reply to comment #5)
> > > Tools/EWebLauncher/main.c:279
> > > -    ewk_view_fixed_layout_size_set(app->browser, app->viewport.w, app->viewport.h);
> > > -    ewk_view_zoom_set(app->browser, app->viewport.initScale, 0, 0);
> > >      if (!ewk_view_zoom_range_set(app->browser, app->viewport.minScale, app->viewport.maxScale))
> > >          info(" Fail to set zoom range. minScale = %f, maxScale = %f\n", app->viewport.minScale, app->viewport.maxScale);
> > >      ewk_view_user_scalable_set(app->browser, app->viewport.userScalable);
> > > +
> > > +    ewk_view_fixed_layout_size_set(app->browser, app->viewport.w, app->viewport.h);
> > > +    ewk_view_zoom_set(app->browser, app->viewport.initScale, 0, 0);
> > > +
> > 
> > Can I know why you moved?
> 
> when view port changed, zoom level was set. ewk_view_zoom_set check zoom range. 
> but zoom range value was old one.
> 
> so i changed setting sequence.

Thanks, I checked it.
You're right.

[Raphael Kubo da Costa (:rakuco)]

>> Source/WebKit/efl/ChangeLog:8
>> +	add CachedPage's content size update func becuase CachedPage 's content size was delete when it was  cached.
> 
> Line contains tab character.  [whitespace/tab] [5]

The English here is a bit hard do understand. Could you try to rephrase your description?

> Source/WebKit/efl/ewk/ewk_view.cpp:4508
> + * @param[in] o a Evas_Object m_view

There is no need for the '[in'] here.

> Source/WebKit/efl/ewk/ewk_view.cpp:4509
> + * @param[in] frame a Evas_Object m_frame

Ditto.

> Source/WebKit/efl/ewk/ewk_view.cpp:4510
> + * @return void

There is no need for this if the function does not return anything.

It'd also be good to add an "@internal" tag in the apidox as well.

>> Tools/ChangeLog:6
>> +	when view port attribute was set, zoom range and user scalable value must setted before fixed size set
> 
> Line contains tab character.  [whitespace/tab] [5]

The English here could be cleaned up a little too.

[DongJae KIM]

(In reply to comment #10)
> (In reply to comment #8)
> 
> > when user add 4th cached page, first cached page will be destroy. 
> > then frame was released and has null reference assert.
> > 
> > so sd->frame->view() was create always.
> 
> If we don't use cached page or frame is not null, I think we need to check the condition. If frame already has a view instance, we don't need to create new view. Please consider both normal case and your case.

Currently, EWeblauncher Browser has some crash when use click some link.(Using Current Revision)

please see below call stack.

Program received signal SIGSEGV, Segmentation fault.
0x0090db99 in WebCore::Document::didRemoveWheelEventHandler (this=0x81a5b48)
    at /home/dongjae/Projects/WebKit-Open/WebKit/Source/WebCore/dom/Document.cpp:5051
5051	    ASSERT(m_wheelEventHandlerCount > 0);
(gdb) bt
#0  0x0090db99 in WebCore::Document::didRemoveWheelEventHandler (this=0x81a5b48)
    at /home/dongjae/Projects/WebKit-Open/WebKit/Source/WebCore/dom/Document.cpp:5051
#1  0x00b96c8a in WebCore::FrameView::willRemoveHorizontalScrollbar (this=0x80ba4c8, scrollbar=0x8166f78)
    at /home/dongjae/Projects/WebKit-Open/WebKit/Source/WebCore/page/FrameView.cpp:323
#2  0x00bf6bc6 in WebCore::ScrollView::setHasHorizontalScrollbar (this=0x80ba4c8, hasBar=false)
    at /home/dongjae/Projects/WebKit-Open/WebKit/Source/WebCore/platform/ScrollView.cpp:100
#3  0x00bf8041 in WebCore::ScrollView::updateScrollbars (this=0x80ba4c8, desiredOffset=...)
    at /home/dongjae/Projects/WebKit-Open/WebKit/Source/WebCore/platform/ScrollView.cpp:476
#4  0x00bf6fbd in WebCore::ScrollView::setScrollbarModes (this=0x80ba4c8, horizontalMode=WebCore::ScrollbarAlwaysOff, 
    verticalMode=WebCore::ScrollbarAlwaysOn, horizontalLock=false, verticalLock=false)
    at /home/dongjae/Projects/WebKit-Open/WebKit/Source/WebCore/platform/ScrollView.cpp:162
#5  0x00b9fb37 in WebCore::ScrollView::setHorizontalScrollbarMode (this=0x80ba4c8, mode=WebCore::ScrollbarAlwaysOff, lock=false)
    at /home/dongjae/Projects/WebKit-Open/WebKit/Source/WebCore/platform/ScrollView.h:94
#6  0x00b98c42 in WebCore::FrameView::layout (this=0x80ba4c8, allowSubtree=false)
    at /home/dongjae/Projects/WebKit-Open/WebKit/Source/WebCore/page/FrameView.cpp:979
#7  0x00b9e3d3 in WebCore::FrameView::forceLayout (this=0x80ba4c8, allowSubtree=false)
    at /home/dongjae/Projects/WebKit-Open/WebKit/Source/WebCore/page/FrameView.cpp:2603
#8  0x008d88fc in ewk_view_fixed_layout_size_set (o=0x808e6d8, w=980, h=729)
    at /home/dongjae/Projects/WebKit-Open/WebKit/Source/WebKit/efl/ewk/ewk_view.cpp:1159
#9  0x0804acf7 in viewport_set () at /home/dongjae/Projects/WebKit-Open/WebKit/Tools/EWebLauncher/main.c:277
#10 0x0804b3b4 in on_viewport_changed (user_data=0x8064490, webview=0x808e6d8, event_info=0x0)
    at /home/dongjae/Projects/WebKit-Open/WebKit/Tools/EWebLauncher/main.c:475
#11 0x00238be6 in evas_object_smart_callback_call (obj=0x808e6d8, event=0x17c24c6 "viewport,changed", event_info=0x0)
    at evas_object_smart.c:523
#12 0x008f31ae in ewk_view_viewport_attributes_set (o=0x808e6d8, arguments=...)
    at /home/dongjae/Projects/WebKit-Open/WebKit/Source/WebKit/efl/ewk/ewk_view.cpp:4259

This crash will be fixed.

I think, create WebCore::FrameView is common patch.
How about it?

[DongJae KIM]

(In reply to comment #12)
> (In reply to comment #9)
> > (In reply to comment #5)
> > > > Tools/EWebLauncher/main.c:279
> > > > -    ewk_view_fixed_layout_size_set(app->browser, app->viewport.w, app->viewport.h);
> > > > -    ewk_view_zoom_set(app->browser, app->viewport.initScale, 0, 0);
> > > >      if (!ewk_view_zoom_range_set(app->browser, app->viewport.minScale, app->viewport.maxScale))
> > > >          info(" Fail to set zoom range. minScale = %f, maxScale = %f\n", app->viewport.minScale, app->viewport.maxScale);
> > > >      ewk_view_user_scalable_set(app->browser, app->viewport.userScalable);
> > > > +
> > > > +    ewk_view_fixed_layout_size_set(app->browser, app->viewport.w, app->viewport.h);
> > > > +    ewk_view_zoom_set(app->browser, app->viewport.initScale, 0, 0);
> > > > +
> > > 
> > > Can I know why you moved?
> > 
> > when view port changed, zoom level was set. ewk_view_zoom_set check zoom range. 
> > but zoom range value was old one.
> > 
> > so i changed setting sequence.
> 
> Thanks, I checked it.
> You're right.

Thanks.

[Ryuan Choi]

(In reply to comment #14)
> (In reply to comment #10)
> > (In reply to comment #8)
> > 
> > > when user add 4th cached page, first cached page will be destroy. 
> > > then frame was released and has null reference assert.
> > > 
> > > so sd->frame->view() was create always.
> > 
> > If we don't use cached page or frame is not null, I think we need to check the condition. If frame already has a view instance, we don't need to create new view. Please consider both normal case and your case.
> 
> Currently, EWeblauncher Browser has some crash when use click some link.(Using Current Revision)

Great. I couldn't catch how to fix it.
I wish new patch. :)

[DongJae KIM]

(In reply to comment #13)
> >> Source/WebKit/efl/ChangeLog:8
> >> +	add CachedPage's content size update func becuase CachedPage 's content size was delete when it was  cached.
> > 
> > Line contains tab character.  [whitespace/tab] [5]
> 
> The English here is a bit hard do understand. Could you try to rephrase your description?
> 
> > Source/WebKit/efl/ewk/ewk_view.cpp:4508
> > + * @param[in] o a Evas_Object m_view
> 
> There is no need for the '[in'] here.
> 
> > Source/WebKit/efl/ewk/ewk_view.cpp:4509
> > + * @param[in] frame a Evas_Object m_frame
> 
> Ditto.
> 
> > Source/WebKit/efl/ewk/ewk_view.cpp:4510
> > + * @return void
> 
> There is no need for this if the function does not return anything.
> 
> It'd also be good to add an "@internal" tag in the apidox as well.
> 
> >> Tools/ChangeLog:6
> >> +	when view port attribute was set, zoom range and user scalable value must setted before fixed size set
> > 
> > Line contains tab character.  [whitespace/tab] [5]
> 
> The English here could be cleaned up a little too.

Thanks your comments.

[DongJae KIM]

Created attachment 98814 [details]
patch for efl port back/forward cache enable

Second patch for check comments

[Ryuan Choi]

Comment on attachment 98814 [details]
patch for efl port back/forward cache enable

View in context: https://bugs.webkit.org/attachment.cgi?id=98814&action=review

> Source/WebKit/efl/ewk/ewk_view.cpp:4515
> +    WebCore::Frame* main_frame = ewk_frame_core_get(sd->main_frame); 
> +    if (!main_frame)
> +        return;

Sorry, you can use priv->main_frame. I confused.
I think that you don't need to check whether it is null.

> Tools/ChangeLog:8
> +        when view port changed, zoom level was set. ewk_view_zoom_set() check zoom range.

When

[DongJae KIM]

(In reply to comment #19)
> (From update of attachment 98814 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=98814&action=review
> 
> > Source/WebKit/efl/ewk/ewk_view.cpp:4515
> > +    WebCore::Frame* main_frame = ewk_frame_core_get(sd->main_frame); 
> > +    if (!main_frame)
> > +        return;
> 
> Sorry, you can use priv->main_frame. I confused.
> I think that you don't need to check whether it is null.
> 
> > Tools/ChangeLog:8
> > +        when view port changed, zoom level was set. ewk_view_zoom_set() check zoom range.
> 
> When

did you mean " sd->_priv->main_frame->loader()->history()->currentItem(); "?

[Gyuyoung Kim]

Comment on attachment 98814 [details]
patch for efl port back/forward cache enable

View in context: https://bugs.webkit.org/attachment.cgi?id=98814&action=review

> Source/WebKit/efl/ChangeLog:13
> +            because, WebCore::FrameView will be create always when user go to new page.

Adhere indentation in here

> Source/WebKit/efl/ewk/ewk_view.cpp:4516
> +

You already get priv instance from sd. So, you can use priv->main_frame. And also you already check if it is null or not in EWK_VIEW_PRIV_GET_OR_RETURN().

[Gyuyoung Kim]

Comment on attachment 98814 [details]
patch for efl port back/forward cache enable

View in context: https://bugs.webkit.org/attachment.cgi?id=98814&action=review

> Source/WebKit/efl/ewk/ewk_view.cpp:4506
> + * @param o a Evas_Object m_view

Change a with an. And also, it is better to use below expression.

 * @param o view object to get cached page.

> Source/WebKit/efl/ewk/ewk_view.cpp:4519
> +        return;

Move return to item below in order to adhere indentation.

> Source/WebKit/efl/ewk/ewk_view.cpp:4522
> +        main_frame->view()->adjustViewSize();

ditto.

> Tools/EWebLauncher/main.c:276
> +

Unnecessary blank line.

[DongJae KIM]

Created attachment 98828 [details]
patch for efl port back/forward cache enable

Check ryuan.choi's comment

[DongJae KIM]

Created attachment 98831 [details]
patch for efl port back/forward cache enable

check Gyuyoung Kim comment

[DongJae KIM]

(In reply to comment #22)
> (From update of attachment 98814 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=98814&action=review
> 
> > Source/WebKit/efl/ewk/ewk_view.cpp:4506
> > + * @param o a Evas_Object m_view
> 
> Change a with an. And also, it is better to use below expression.
> 
>  * @param o view object to get cached page.
> 

change description 

> > Source/WebKit/efl/ewk/ewk_view.cpp:4519
> > +        return;
> 
> Move return to item below in order to adhere indentation.
> 
> > Source/WebKit/efl/ewk/ewk_view.cpp:4522
> > +        main_frame->view()->adjustViewSize();
> 
> ditto.
> 

Indentation was right, please check it again.

> > Tools/EWebLauncher/main.c:276
> > +
> 
> Unnecessary blank line.

remove blank line.

[DongJae KIM]

Created attachment 98834 [details]
patch for efl port back/forward cache enable

[Gyuyoung Kim]

Comment on attachment 98834 [details]
patch for efl port back/forward cache enable

View in context: https://bugs.webkit.org/attachment.cgi?id=98834&action=review

> Tools/EWebLauncher/main.c:277
> +    ewk_view_zoom_set(app->browser, app->viewport.initScale, 0, 0);

If we move fixed_layout_set and zoom_set to after zoom_range_set, user_scalable_set, we may not set init width, height and initial zoom level when website defines wrong zoom range. I mean we need to set initial width,height and initial scale value before setting zoom range.

[DongJae KIM]

(In reply to comment #27)
> (From update of attachment 98834 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=98834&action=review
> 
> > Tools/EWebLauncher/main.c:277
> > +    ewk_view_zoom_set(app->browser, app->viewport.initScale, 0, 0);
> 
> If we move fixed_layout_set and zoom_set to after zoom_range_set, user_scalable_set, we may not set init width, height and initial zoom level when website defines wrong zoom range. I mean we need to set initial width,height and initial scale value before setting zoom range.

In my patch, back/forward zoom value was wrong so i changed.

I think this is different issue. create new bug is better.

How about?

[Gyuyoung Kim]

(In reply to comment #28)
> (In reply to comment #27)
> > (From update of attachment 98834 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=98834&action=review
> > 
> > > Tools/EWebLauncher/main.c:277
> > > +    ewk_view_zoom_set(app->browser, app->viewport.initScale, 0, 0);
> > 
> > If we move fixed_layout_set and zoom_set to after zoom_range_set, user_scalable_set, we may not set init width, height and initial zoom level when website defines wrong zoom range. I mean we need to set initial width,height and initial scale value before setting zoom range.
> 
> In my patch, back/forward zoom value was wrong so i changed.
> 
> I think this is different issue. create new bug is better.
> 
> How about?

Ok, file a new bug.

[Raphael Kubo da Costa (:rakuco)]

> Source/WebKit/efl/ChangeLog:8
> +        1) Add contentsSize update function for cached page resotre ( ewk_view_cached_page_restore() )

resotre -> restore
The spaces in the parentheses are not necessary.

> Source/WebKit/efl/ChangeLog:9
> +        2) Change return value for enable EFL Port back/forward Cache ( FrameLoaderClientEfl::canCachePage() )

Same thing about spaces.

> Source/WebKit/efl/ChangeLog:13
> +            because, WebCore::FrameView will be create always when user go to new page.

The comma after because is not needed.
I'd rephrase it as "WebCore::FrameView will always be created when a new page is loaded" (or opened).

> Source/WebKit/efl/ChangeLog:14
> +            Currently, EWebLauncher browser has crashes. 

I'd rephrase this section as "This patch also ends up fixing two crashes in EWebLauncher: one when loading the 4th cached page and another triggered when a user clicks any link".

> Source/WebKit/efl/ewk/ewk_view.cpp:4502
> +/**

Missing blank line before this. Please also add an "@internal" tag, as this function is defined in ewk_private.h

[DongJae KIM]

(In reply to comment #30)
> > Source/WebKit/efl/ChangeLog:8
> > +        1) Add contentsSize update function for cached page resotre ( ewk_view_cached_page_restore() )
> 
> resotre -> restore
> The spaces in the parentheses are not necessary.
> 
> > Source/WebKit/efl/ChangeLog:9
> > +        2) Change return value for enable EFL Port back/forward Cache ( FrameLoaderClientEfl::canCachePage() )
> 
> Same thing about spaces.
> 
> > Source/WebKit/efl/ChangeLog:13
> > +            because, WebCore::FrameView will be create always when user go to new page.
> 
> The comma after because is not needed.
> I'd rephrase it as "WebCore::FrameView will always be created when a new page is loaded" (or opened).
> 
> > Source/WebKit/efl/ChangeLog:14
> > +            Currently, EWebLauncher browser has crashes. 
> 
> I'd rephrase this section as "This patch also ends up fixing two crashes in EWebLauncher: one when loading the 4th cached page and another triggered when a user clicks any link".
> 
> > Source/WebKit/efl/ewk/ewk_view.cpp:4502
> > +/**
> 
> Missing blank line before this. Please also add an "@internal" tag, as this function is defined in ewk_private.h

Thanks your comment.

[DongJae KIM]

Created attachment 98972 [details]
patch for efl port back/forward cache enable

[DongJae KIM]

Created attachment 98983 [details]
patch for efl port back/forward cache enable

[DongJae KIM]

Comment on attachment 98972 [details]
patch for efl port back/forward cache enable

There are some conflict things.

[Raphael Kubo da Costa (:rakuco)]

There are some "( foo )" instead of "(foo)" in the first ChangeLog, and the text after 3) is indented weirdly.

Other than that, r+ from my side after the Windows bot stops complaining.

[DongJae KIM]

(In reply to comment #35)
> There are some "( foo )" instead of "(foo)" in the first ChangeLog, and the text after 3) is indented weirdly.
> 

I cann't understand your comment.

How about below?
=====================================
        1) Add contentsSize update function for cached page restore
        2) Change return value for enable EFL Port back/forward Cache
        3) Delete below code in ewk_frame_view_create_for_view()
            if (sd->frame->view())
                return;
            because WebCore::FrameView will always be created when a new page is opened.
            This patch also ends up fixing two crashes in EWebLauncher: one when loading the 4th cached page
            and another triggered when a user clicks any link
=====================================

> Other than that, r+ from my side after the Windows bot stops complaining.
what r+? please tell me more details.

Thanks your comments.

[DongJae KIM]

(In reply to comment #35)
> There are some "( foo )" instead of "(foo)" in the first ChangeLog, and the text after 3) is indented weirdly.
> 
> Other than that, r+ from my side after the Windows bot stops complaining.

I change my ChangeLog.

[DongJae KIM]

Created attachment 99192 [details]
patch for efl port back/forward cache enable

[Gyuyoung Kim]

Comment on attachment 99192 [details]
patch for efl port back/forward cache enable

View in context: https://bugs.webkit.org/attachment.cgi?id=99192&action=review

> Source/WebKit/efl/ChangeLog:40
> +>>>>>>> .r90072

Remove this.

> Tools/ChangeLog:280
> +>>>>>>> .r90071

ditto.

[DongJae KIM]

(In reply to comment #39)
> (From update of attachment 99192 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=99192&action=review
> 
> > Source/WebKit/efl/ChangeLog:40
> > +>>>>>>> .r90072
> 
> Remove this.
> 
> > Tools/ChangeLog:280
> > +>>>>>>> .r90071
> 
> ditto.

Oh.. sorry..I will be fix it.
Thanks.

[DongJae KIM]

Created attachment 99597 [details]
patch for efl port back/forward cache enable

Fix Change Log.

[DongJae KIM]

Created attachment 99601 [details]
patch for efl port back/forward cache enable

Change Log Fix.

[Gyuyoung Kim]

Comment on attachment 99601 [details]
patch for efl port back/forward cache enable

View in context: https://bugs.webkit.org/attachment.cgi?id=99601&action=review

> Source/WebKit/efl/ewk/ewk_view.cpp:4592
> + * @internal

You have to add summary for this API first to here. Make reference to other APIs.

[Gyuyoung Kim]

(In reply to comment #10)
> (In reply to comment #8)
> 
> > when user add 4th cached page, first cached page will be destroy. 
> > then frame was released and has null reference assert.
> > 
> > so sd->frame->view() was create always.
> 
> If we don't use cached page or frame is not null, I think we need to check the condition. If frame already has a view instance, we don't need to create new view. Please consider both normal case and your case.

Did you reply my question ? I can't find your reply this thread.

[DongJae KIM]

Created attachment 99605 [details]
patch for efl port back/forward cache enable

Fixe ewk_view.cpp comment.(apply Gyuyoung's comment)

[DongJae KIM]

(In reply to comment #44)
> (In reply to comment #10)
> > (In reply to comment #8)
> > 
> > > when user add 4th cached page, first cached page will be destroy. 
> > > then frame was released and has null reference assert.
> > > 
> > > so sd->frame->view() was create always.
> > 
> > If we don't use cached page or frame is not null, I think we need to check the condition. If frame already has a view instance, we don't need to create new view. Please consider both normal case and your case.
> 
> Did you reply my question ? I can't find your reply this thread.

My answer is Comment#14.

Additional Comment:: when create view, WebCore::Frame::CreateView was call below api for create FrameView
setView(0) -> view = FrameView::createView() ->setView(view);

so, there are no problem to create new page.

[Raphael Kubo da Costa (:rakuco)]

LGTM.

[Gyuyoung Kim]

Comment on attachment 99605 [details]
patch for efl port back/forward cache enable

View in context: https://bugs.webkit.org/attachment.cgi?id=99605&action=review

> Source/WebKit/efl/ewk/ewk_view.cpp:4593
> + * Restore cached page contentsSize

First, %s/contentsSize/content size/g 

Second, missing a period end of line.

Third, add a blank between summary and description.

[DongJae KIM]

Created attachment 99681 [details]
 patch for efl port back/forward cache enable

Change patch, Fix Gyuyoung's comment
Thanks.

[DongJae KIM]

Created attachment 99682


Change patch, Fix Gyuyoung's comment
Thanks.

[Martin Robinson]

Comment on attachment 99681 [details]
 patch for efl port back/forward cache enable

View in context: https://bugs.webkit.org/attachment.cgi?id=99681&action=review

> Source/WebKit/efl/ChangeLog:24
> +        1) Add contentsSize update function for cached page restore
> +        2) Change return value for enable EFL Port back/forward Cache
> +        3) Delete below code in ewk_frame_view_create_for_view()
> +            if (sd->frame->view())
> +                return;
> +            because WebCore::FrameView will always be created when a new page is opened.
> +            This patch also ends up fixing two crashes in EWebLauncher: one when loading the 4th cached page
> +            and another triggered when a user clicks any link
> +
> +        * WebCoreSupport/FrameLoaderClientEfl.cpp:
> +        (WebCore::FrameLoaderClientEfl::dispatchDidCommitLoad):
> +        (WebCore::FrameLoaderClientEfl::canCachePage):
> +        * ewk/ewk_frame.cpp:
> +        (ewk_frame_view_create_for_view):
> +        * ewk/ewk_private.h:
> +        * ewk/ewk_view.cpp:
> +        (ewk_view_cached_page_restore):

Instead of putting the summary of what you did above the empty method name template, why not just fill out the template with the appropriate information? :)

[DongJae KIM]

Created attachment 101431 [details]
patch for efl port back/forward cache enable

Check Martin's comment about my changes. and modify patch.
Thank Martin Robinson.

[Antonio Gomes]

Comment on attachment 101431 [details]
patch for efl port back/forward cache enable

View in context: https://bugs.webkit.org/attachment.cgi?id=101431&action=review

Needs one more interaction for clearance.

> Source/WebKit/efl/ewk/ewk_view.cpp:4595
> + * Restore cached page contents size.
> + * 
> + * This method must be called before WebCore::FrameView::layout().

How does the code ensure that? It is too error-prone, imo.

> Tools/ChangeLog:10
> +        When view port changed, zoom level was set. ewk_view_zoom_set() check zoom range
> +        but zoom range value was old one.
> +        So, I changed setting sequence.

typo: checkS*

Not sure if I understood this sentence.

[DongJae KIM]

(In reply to comment #53)
> (From update of attachment 101431 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=101431&action=review
> 
> Needs one more interaction for clearance.
> 
> > Source/WebKit/efl/ewk/ewk_view.cpp:4595
> > + * Restore cached page contents size.
> > + * 
> > + * This method must be called before WebCore::FrameView::layout().
> 
> How does the code ensure that? It is too error-prone, imo.
> 
> > Tools/ChangeLog:10
> > +        When view port changed, zoom level was set. ewk_view_zoom_set() check zoom range
> > +        but zoom range value was old one.
> > +        So, I changed setting sequence.
> 
> typo: checkS*
> 
> Not sure if I understood this sentence.

Thanks your comment.

> > Source/WebKit/efl/ewk/ewk_view.cpp:4595
> > + * Restore cached page contents size.
> > + * 
> > + * This method must be called before WebCore::FrameView::layout().
> 
> How does the code ensure that? It is too error-prone, imo.

:: It is must be called before cached page restore. I will be change this comment more clearly.

> > +        When view port changed, zoom level was set. ewk_view_zoom_set() check zoom range
> > +        but zoom range value was old one.
> > +        So, I changed setting sequence.
> 
> typo: checkS*
> 
> Not sure if I understood this sentence.

Currently, EWebLaunch browser view port change function has some bugs. 
Zoom range must be setted before zoom set, but not. so I changed it.

[DongJae KIM]

Created attachment 103315 [details]
patch for efl port back/forward cache enable

[Antonio Gomes]

Comment on attachment 103315 [details]
patch for efl port back/forward cache enable

View in context: https://bugs.webkit.org/attachment.cgi?id=103315&action=review

> Source/WebKit/efl/ChangeLog:22
> +        (ewk_view_cached_page_restore): Added. adjust view size for restored cached page.
> +
> +

too many empty lines.

> Source/WebKit/efl/ewk/ewk_frame.cpp:-1459
> -    if (sd->frame->view())
> -        return;
> -

how is this related? Could you explain?

> Source/WebKit/efl/ewk/ewk_view.cpp:3664
> +* Restore cached page contents size.

RestoreS

> Tools/ChangeLog:10
> +        Zoom range value must be setted before zoom set, but not in EWebLauncher.
> +
> +

too many empty lines.

[DongJae KIM]

(In reply to comment #56)
> (From update of attachment 103315 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=103315&action=review
> 
> > Source/WebKit/efl/ChangeLog:22
> > +        (ewk_view_cached_page_restore): Added. adjust view size for restored cached page.
> > +
> > +
> 
> too many empty lines.
> 
> > Source/WebKit/efl/ewk/ewk_frame.cpp:-1459
> > -    if (sd->frame->view())
> > -        return;
> > -
> 
> how is this related? Could you explain?
> 
> > Source/WebKit/efl/ewk/ewk_view.cpp:3664
> > +* Restore cached page contents size.
> 
> RestoreS
> 
> > Tools/ChangeLog:10
> > +        Zoom range value must be setted before zoom set, but not in EWebLauncher.
> > +
> > +
> 
> too many empty lines.

Thanks, I will remove empty lines.

> > Source/WebKit/efl/ewk/ewk_frame.cpp:-1459
> > -    if (sd->frame->view())
> > -        return;
> > -
>
EFL Port, FrameView was created, it will be re-use other page, but I think FrameView will be create always when use go to new page, isn't it?

when user add 4th cached page, first cached page will be destroy. 
then frame was released and has null reference assert.

so sd->frame->view() was create always.

test case ::
1. add 3 cached page
2. add one more cached page
3. scroll page
4. assert 


Program received signal SIGSEGV, Segmentation fault.
0x00b671ed in WebCore::EventHandler::setFrameWasScrolledByUser (this=0x4f0)
    at /home/dongjae/Projects/WebKit_EFL/Source/WebCore/page/EventHandler.cpp:2997
2997        FrameView* v = m_frame->view();
(gdb) bt
#0  0x00b671ed in WebCore::EventHandler::setFrameWasScrolledByUser (this=0x4f0)
    at /home/dongjae/Projects/WebKit_EFL/Source/WebCore/page/EventHandler.cpp:2997
#1  0x00b6714d in WebCore::EventHandler::sendScrollEvent (this=0x4f0)
    at /home/dongjae/Projects/WebKit_EFL/Source/WebCore/page/EventHandler.cpp:2990
#2  0x00b7af83 in WebCore::FrameView::scrollPositionChanged (this=0x80b98e0)
    at /home/dongjae/Projects/WebKit_EFL/Source/WebCore/page/FrameView.cpp:1468
#3  0x00b7d3ff in WebCore::FrameView::scrollTo (this=0x80b98e0, newOffset=...)
    at /home/dongjae/Projects/WebKit_EFL/Source/WebCore/page/FrameView.cpp:2110
#4  0x00bd65a7 in WebCore::ScrollView::setScrollOffset (this=0x80b98e0, offset=...)
    at /home/dongjae/Projects/WebKit_EFL/Source/WebCore/platform/ScrollView.cpp:351
#5  0x00bd2592 in WebCore::ScrollableArea::setScrollOffsetFromAnimation (this=0x80b990c, offset=...)
    at /home/dongjae/Projects/WebKit_EFL/Source/WebCore/platform/ScrollableArea.cpp:133
#6  0x0127e999 in WebCore::ScrollAnimator::notityPositionChanged (this=0x80935e0)
    at /home/dongjae/Projects/WebKit_EFL/Source/WebCore/platform/ScrollAnimator.cpp:129
#7  0x0127e4a9 in WebCore::ScrollAnimator::scroll (this=0x80935e0, orientation=WebCore::VerticalScrollbar, step=1, multiplier=40)

[DongJae KIM]

Created attachment 103329 [details]
patch for efl port back/forward cache enable

[Antonio Gomes]

Comment on attachment 103329 [details]
patch for efl port back/forward cache enable

View in context: https://bugs.webkit.org/attachment.cgi?id=103329&action=review

> Source/WebKit/efl/ChangeLog:15
> +        (ewk_frame_view_create_for_view): Delete WebCore::FrameView check code
> +        because WebCore::FrameView will always be created when a new page is opened but not in EFL port.

It is really strange. FrameView being persisted cross page loads.

> Tools/ChangeLog:8
> +        Zoom range value must be setted before zoom set, but not in EWebLauncher.

setted = set

[DongJae KIM]

(In reply to comment #59)
> (From update of attachment 103329 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=103329&action=review
> 
> > Source/WebKit/efl/ChangeLog:15
> > +        (ewk_frame_view_create_for_view): Delete WebCore::FrameView check code
> > +        because WebCore::FrameView will always be created when a new page is opened but not in EFL port.
> 
> It is really strange. FrameView being persisted cross page loads.

GTK and QT port create FrameView always when go to new page. 

and when forth page cached, there are crash. in this case, there no FrameView.

> 
> > Tools/ChangeLog:8
> > +        Zoom range value must be setted before zoom set, but not in EWebLauncher.
> 
> setted = set

I will be changed.

Thanks. your comment.

[DongJae KIM]

(In reply to comment #60)
> (In reply to comment #59)
> > (From update of attachment 103329 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=103329&action=review
> > 
> > > Source/WebKit/efl/ChangeLog:15
> > > +        (ewk_frame_view_create_for_view): Delete WebCore::FrameView check code
> > > +        because WebCore::FrameView will always be created when a new page is opened but not in EFL port.
> > 
> > It is really strange. FrameView being persisted cross page loads.
> 
> GTK and QT port create FrameView always when go to new page. 
> 
> and when forth page cached, there are crash. in this case, there no FrameView.

This code already merged main branch.

Please check "https://bugs.webkit.org/show_bug.cgi?id=66690"
> 
> > 
> > > Tools/ChangeLog:8
> > > +        Zoom range value must be setted before zoom set, but not in EWebLauncher.
> > 
> > setted = set
> 
> I will be changed.
> 
> Thanks. your comment.

[DongJae KIM]

*** Bug 67355 has been marked as a duplicate of this bug. ***

[DongJae KIM]

Created attachment 105910 [details]
Enable EFL Port PageCache

[Kenneth Rohde Christiansen]

Comment on attachment 105910 [details]
Enable EFL Port PageCache

View in context: https://bugs.webkit.org/attachment.cgi?id=105910&action=review

> Source/WebKit/efl/ChangeLog:8
> [phone]  DongJae KIM  <[e-mail]>
> +
> +        [EFL] Enable EFL Port Page Cache function.
> +        https://bugs.webkit.org/show_bug.cgi?id=63013
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Enable back/forward cache for EFL port.

We dont seems to need something like ewk_view_cached_page_restore in public API for Qt, so why is this really needed? This doesn't seem like the right solution.

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 105910 [details]
Enable EFL Port PageCache

Clearing r? flag while Kenneth's question is not answered.

[Raphael Kubo da Costa (:rakuco)]

Reverting from RESOLVED/LATER to UNCONFIRMED. This has not been resolved yet, so the bug shouldn't usually be closed (the fact that the patch has no flags is enough to make it not show up for reviewers, for example).

[Ryuan Choi]

(In reply to comment #66)
> Reverting from RESOLVED/LATER to UNCONFIRMED. This has not been resolved yet, so the bug shouldn't usually be closed (the fact that the patch has no flags is enough to make it not show up for reviewers, for example).

DongJae,
Can you address whether we still need this?

page cashe looks already enabled after r116341.
http://trac.webkit.org/browser/trunk/Source/WebKit/efl/WebCoreSupport/FrameLoaderClientEfl.cpp#L935

[Ryuan Choi]

Close this bug because page cache is already enabled.