EFL's LayoutTestController needs keepWebHistory implementation in order to unskip fast/history/visited-generated-content-test.html fast/history/visited-link-background-color.html fast/history/window-open.html LayoutTestController keepWebHistory implementation requires shared history support.
Created attachment 135059 [details] ewk_history is extended to support shared history feature + dummy implementation of FrameLoaderClientEfl::updateGlobalHistory.
Comment on attachment 135059 [details] ewk_history is extended to support shared history feature + dummy implementation of FrameLoaderClientEfl::updateGlobalHistory. Rejecting attachment 135059 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: offset 12 lines). patching file Source/WebKit/efl/ewk/ewk_history.cpp patching file Source/WebKit/efl/ewk/ewk_history.h patching file Tools/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp Hunk #1 succeeded at 108 with fuzz 1. Hunk #2 succeeded at 161 (offset 7 lines). Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Nate Chapin']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue/ Full output: http://queues.webkit.org/results/12477015
Please don't cq+ it yet, I'd like to take a look at it before.
Comment on attachment 135059 [details] ewk_history is extended to support shared history feature + dummy implementation of FrameLoaderClientEfl::updateGlobalHistory. View in context: https://bugs.webkit.org/attachment.cgi?id=135059&action=review > LayoutTests/ChangeLog:8 > + * platform/efl/Skipped: You don't mention any description for this change. > Source/WebKit/efl/ewk/ewk_history.h:69 > +EAPI void ewk_history_free(Ewk_History* history); Move * to variable name side. (Ewk_History *history); > Tools/ChangeLog:7 > + Missing description too.
Created attachment 140184 [details] rebased + fixed changelog description
(In reply to comment #4) > (From update of attachment 135059 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=135059&action=review > > > LayoutTests/ChangeLog:8 > > + * platform/efl/Skipped: > > You don't mention any description for this change. > > > Source/WebKit/efl/ewk/ewk_history.h:69 > > +EAPI void ewk_history_free(Ewk_History* history); > > Move * to variable name side. > > (Ewk_History *history); > > > Tools/ChangeLog:7 > > + > > Missing description too. Fixed.
Comment on attachment 140184 [details] rebased + fixed changelog description View in context: https://bugs.webkit.org/attachment.cgi?id=140184&action=review The API does not look very good to me yet: - "optional_shared_history" is too clunky. I still don't know if it makes sense to make the shared history an Ewk_History at all, or go the Qt route and have a separate object for that. - There seems to be no way to destroy the shared history object correctly (it's not done in DRT, and Qt uses qAddPostRoutine for that). > Source/WebKit/efl/WebCoreSupport/FrameLoaderClientEfl.cpp:961 > + // FIXME: primitive behaviour yet for enabling Layout tests. See mac or win ports implementation. Can you elaborate on what these ports do differently from us? > Source/WebKit/efl/ewk/ewk_history.cpp:215 > +int ewk_history_all_item_count(const Ewk_History *history) > +{ > + EWK_HISTORY_CORE_GET_OR_RETURN(history, core, 0); > + return static_cast<int>(core->entries().size()); > +} I wonder if there's really a need for this method -- it's the same as back_list_length + forward_list_length. > Tools/ChangeLog:9 > + LayoutTestController::keepWebHistory() implementation added. > + LayoutTestController::webHistoryItemCount() returns count of all history items including the current one. These can be moved to the method list below. > Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp:112 > + if (!ewk_history_optional_shared_history_get()) > + ewk_history_optional_shared_history_set(ewk_history_new()); Don't you need to reset this after each test?
Comment on attachment 135059 [details] ewk_history is extended to support shared history feature + dummy implementation of FrameLoaderClientEfl::updateGlobalHistory. Cleared Nate Chapin's review+ from obsolete attachment 135059 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment on attachment 140184 [details] rebased + fixed changelog description View in context: https://bugs.webkit.org/attachment.cgi?id=140184&action=review Set cq- because of Kubo comment and doxyzen errors. > Source/WebKit/efl/ewk/ewk_history.h:56 > + * @return newly allocated history instance or @c NULL on errors. You must Nit : We don't use '.' at the end of *@XXXX* sentence. In addition, new line is placed just below at the beginning of description. > Source/WebKit/efl/ewk/ewk_history.h:74 > + * @return shared history instance or @c NULL if shared history is not set. ditto. > Source/WebKit/efl/ewk/ewk_history.h:81 > + * @note when a new history instance is set the previous one is not freed. ditto. > Source/WebKit/efl/ewk/ewk_history.h:83 > + * @param history the history to use for the shared history. ditto. > Source/WebKit/efl/ewk/ewk_history.h:200 > + * @param history which history instance to query. ditto. > Source/WebKit/efl/ewk/ewk_history.h:202 > + * @return number of elements in whole list. ditto.
Closing this bug because the EFL port has been removed from trunk. If you feel this bug applies to a different upstream WebKit port and was closed in error, please either update the title and reopen the bug, or leave a comment to request this.