Bug 34167 – [Qt] DRT WebHistory support

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Diego Gonzalez]

Activate WebHistory support in Qt DRT.

Implement the keepWebHistory() and webHistoryItemCount() in LayoutTestController 

LayoutTests: http/tests/history

[Diego Gonzalez]

Created attachment 47412 [details]
Activate history in DRT

Possible way to activate the history in Qt DRT

[Diego Gonzalez]

The tests is not passing with the patch above (47412) because in the redirect file page (LayoutTests/http/tests/history/resources/redirect-target.html) the webHistoryItemCount is increased in one according the code below.

function testHistoryItemCount()
{
    var expected = parseInt(location.hash.slice(1));
    var actual = layoutTestController.webHistoryItemCount + 1; // Add one to include the referring page, which loaded before we started recording history.
    if (actual === expected)
        log("PASS: History item count should be " + expected + " and is.");
    else
        log("FAIL: History item count should be " + expected + " but instead is " + actual + ".");
}

In our case is not necessary to add one to webHistoryItemCount because QWebHistory, created in QWebPage, records the referred page.

One possible way to make the tests succeed is use specific expected files for Qt port use in these tests.

Other ways coukd be: decrease in one the return of layoutTestController.webHistoryItemCount(); use the backItems() size; or limit the maximum count of QWebHistory.

Someone has any suggestion or comment of the the best way to solve it?

[Antonio Gomes]

> In our case is not necessary to add one to webHistoryItemCount because
> QWebHistory, created in QWebPage, records the referred page.
> 
> One possible way to make the tests succeed is use specific expected files for
> Qt port use in these tests.

I vote for this option.

> 
> Other ways coukd be: decrease in one the return of
> layoutTestController.webHistoryItemCount(); use the backItems() size; or limit
> the maximum count of QWebHistory.

This way, we would be adapting our implementation to match a test result.

@kenneth, simon: thoughts ?

[Kenneth Rohde Christiansen]

(In reply to comment #3)
> > In our case is not necessary to add one to webHistoryItemCount because
> > QWebHistory, created in QWebPage, records the referred page.
> > 
> > One possible way to make the tests succeed is use specific expected files for
> > Qt port use in these tests.
> 
> I vote for this option.

Expected results saying FAIL. That doesn't sound good. Better just add 1 to the result.

[Diego Gonzalez]

(In reply to comment #4)
> (In reply to comment #3)
> > > In our case is not necessary to add one to webHistoryItemCount because
> > > QWebHistory, created in QWebPage, records the referred page.
> > > 
> > > One possible way to make the tests succeed is use specific expected files for
> > > Qt port use in these tests.
> > 
> > I vote for this option.
> 
> Expected results saying FAIL. That doesn't sound good. Better just add 1 to the
> result.

Kenneth, you mean add 1 in the expected file or when the method returns?

[Diego Gonzalez]

Created attachment 47427 [details]
Proposed patch to pass history tests

With this patch the following tests are passing now:

http/tests/history/redirect-js-form-submit-2-seconds.html
http/tests/history/redirect-js-location-2-seconds.html
http/tests/history/redirect-js-location-assign-before-load.html
http/tests/history/redirect-js-location-href-2-seconds.html
http/tests/history/redirect-js-location-href-0-seconds.html
http/tests/history/redirect-js-location-replace-2-seconds.html
http/tests/history/redirect-js-document-location-before-load.html
http/tests/history/redirect-js-location-assign-0-seconds.html
http/tests/history/redirect-js-location-0-seconds.html
http/tests/history/redirect-meta-refresh-2-seconds.html
http/tests/history/redirect-js-document-location-2-seconds.html
http/tests/history/redirect-js-location-replace-before-load.html
http/tests/history/redirect-js-form-submit-0-seconds.html
http/tests/history/redirect-js-location-before-load.html
http/tests/history/redirect-js-location-href-before-load.html
http/tests/history/redirect-js-form-submit-before-load.html
http/tests/history/redirect-js-document-location-0-seconds.html
http/tests/history/redirect-js-location-assign-2-seconds.html
http/tests/history/redirect-js-location-replace-0-seconds.html
http/tests/history/redirect-meta-refresh-0-seconds.html
http/tests/history/redirect-301.pl
http/tests/history/redirect-303.pl
http/tests/history/redirect-200-refresh-2-seconds.pl
http/tests/history/redirect-307.pl
http/tests/history/redirect-302.pl
http/tests/history/redirect-200-refresh-0-seconds.pl

[Kenneth Rohde Christiansen]

Comment on attachment 47427 [details]
Proposed patch to pass history tests

Please don't mark your own patches as r+

[Kenneth Rohde Christiansen]

Comment on attachment 47427 [details]
Proposed patch to pass history tests


> +    // Subtract one here as out QWebHistory::count() includes the actual page,
> +    // which is not considered in the DRT tests.

our, not out

> +    return m_webHistory->count() - 1;
> +}
> +
>  void LayoutTestController::keepWebHistory()
>  {
> -    // FIXME: implement
> +    m_webHistory = m_drt->webPage()->history();
>  }

[Diego Gonzalez]

Created attachment 47428 [details]
Proposed patch v0.2

Fixed typo

[Diego Gonzalez]

Created attachment 47429 [details]
Proposed patch v0.3

return -1 if web history doesn't exists

[WebKit Commit Bot]

Comment on attachment 47429 [details]
Proposed patch v0.3

Clearing flags on attachment: 47429

Committed r53895: <http://trac.webkit.org/changeset/53895>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

There are 4 failing tests on buildbot after http://trac.webkit.org/changeset/53895
http/tests/history/redirect-301.pl
http/tests/history/redirect-302.pl
http/tests/history/redirect-303.pl
http/tests/history/redirect-307.pl

Skipped them by http://trac.webkit.org/changeset/53906

[Csaba Osztrogonác]

detailed results: http://build.webkit.org/results/Qt%20Linux%20Release/r53905%20%286565%29/results.html

[Diego Gonzalez]

(In reply to comment #14)
> detailed results:
> http://build.webkit.org/results/Qt%20Linux%20Release/r53905%20%286565%29/results.html

I'm taking a look

[Diego Gonzalez]

For some reason LayoutTestController is not being instatiated in these tests. I'm checking it now!

[Diego Gonzalez]

(In reply to comment #16)
> For some reason LayoutTestController is not being instatiated in these tests.
> I'm checking it now!

Actually the LayoutTestController is being instantiated. The keepWebHistory() is not being called because the check:
 
if (window.layoutTestController) {
    layoutTestController.keepWebHistory();
    layoutTestController.waitUntilDone();
}

is not passing.

[Diego Gonzalez]

(In reply to comment #17)
> (In reply to comment #16)
> > For some reason LayoutTestController is not being instatiated in these tests.
> > I'm checking it now!
> 
> Actually the LayoutTestController is being instantiated. The keepWebHistory()
> is not being called because the check:
> 
> if (window.layoutTestController) {
>     layoutTestController.keepWebHistory();
>     layoutTestController.waitUntilDone();
> }
> 
> is not passing.

Created another bug: https://bugs.webkit.org/show_bug.cgi?id=34212 to investigate it. So, closing it