Bug 44784 – [GTK] fast/history/history-subframe-with-name.html fails with GTK DRT

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

[Mihai Parparita]

http://trac.webkit.org/changeset/66238 added fast/history/history-subframe-with-name.htm, and it fails with this diff:

--- /home/slave/webkitgtk/gtk-linux-64-debug/build/layout-test-results/fast/history/history-subframe-with-name-expected.txt	2010-08-27 11:51:06.357907230 -0700
+++ /home/slave/webkitgtk/gtk-linux-64-debug/build/layout-test-results/fast/history/history-subframe-with-name-actual.txt	2010-08-27 11:51:06.357907230 -0700
@@ -9,6 +9,6 @@
 PASS 2 is currentPageId
 PASS 3 is currentPageId
 PASS 2 is currentPageId
-PASS 3 is currentPageId
+FAIL 3 should be 2. Was 3.
 PASS Complete: navigated through all the states

[Mihai Parparita]

Created attachment 65757 [details]
Patch

[Dimitri Glazkov (Google)]

Comment on attachment 65757 [details]
Patch

ok.

[Dimitri Glazkov (Google)]

Comment on attachment 65757 [details]
Patch

Clearing flags on attachment: 65757

Committed r66250: <http://trac.webkit.org/changeset/66250>

[Dimitri Glazkov (Google)]

All reviewed patches have been landed.  Closing bug.

[Mihai Parparita]

Unfortunately I'm not really able to investigate the GTK test failure (I don't have a machine that can run the GTK tests).

Martin, do you think you could look into this? Let me know if you need any help with the changes that r66238 made or the new test (given that there's a bunch of other skipped history tests for GTK, it might just be missing DRT features).

[Martin Robinson]

Sure. I'll take a look later today.

[Martin Robinson]

I still haven't gotten very far on this. The WebKitGTK behavior is bewildering, to say the least.

[Mihai Parparita]

(In reply to comment #7)
> I still haven't gotten very far on this. The WebKitGTK behavior is bewildering, to say the least.

Would there be any race conditions from doing a history.forward() in an onload handler (since presumably the GTK DRT uses the GTK implementation of the back-forward list)?

[Martin Robinson]

(In reply to comment #8)
> (In reply to comment #7)
> > I still haven't gotten very far on this. The WebKitGTK behavior is bewildering, to say the least.
> 
> Would there be any race conditions from doing a history.forward() in an onload handler (since presumably the GTK DRT uses the GTK implementation of the back-forward list)?

The issue here seems to be this:

1. When the history item is for the second subframe is creatied, it has a unique name like "<!--framePath //<!--frame1-->-->".

2. window.name of the subframe changes and its unique name in the tree becomes "foo" (since there is no collision -- see the first conditional in FrameTree::uniqueChildName).

3. When HistoryController::itemsAreClones runs during back/forward navigation, the two history items do not look like clones because their frame trees contain a different set of names.

So it appears that the problem is that the HistoryItem gets out of sync with the actual frame tree. I'm not sure why this does cause a problem on other ports.

[Martin Robinson]

CCing Daniel, since he has tangled recently with unique frame names.

[Martin Robinson]

(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > > I still haven't gotten very far on this. The WebKitGTK behavior is bewildering, to say the least.
> > 
> > Would there be any race conditions from doing a history.forward() in an onload handler (since presumably the GTK DRT uses the GTK implementation of the back-forward list)?
> 
> The issue here seems to be this:

This turned out to be a huge red herring, since other ports function even with this issue. It may point to some ineffeciency though. 

The problem that GTK+ is having is that it is calling FrameLoader::loadURLIntoChildFrame on the child frame's loader as opposed to the parent frame. Frankly, I'm amazed that this worked at all. I should have a patch soon.

[Martin Robinson]

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

[Martin Robinson]

Created attachment 81412 [details]
Patch

[Daniel Bates]

Comment on attachment 81412 [details]
Patch

This makes sense to me.

[Daniel Bates]

Note, both the Haiku and EFL ports have a similar issue. That is, they use the child frame's loader to load the child frame instead of the parent frame's loader.

[Martin Robinson]

Comment on attachment 81412 [details]
Patch

Clearing flags on attachment: 81412

Committed r77818: <http://trac.webkit.org/changeset/77818>

[Martin Robinson]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/77818 might have broken Qt Linux Release, Qt Linux Release minimal, Qt Linux ARMv5 Release, Qt Linux ARMv7 Release, Qt Windows 32-bit Release, and Qt Windows 32-bit Debug

[Csaba Osztrogonác]

(In reply to comment #18)
> http://trac.webkit.org/changeset/77818 might have broken Qt Linux Release, Qt Linux Release minimal, Qt Linux ARMv5 Release, Qt Linux ARMv7 Release, Qt Windows 32-bit Release, and Qt Windows 32-bit Debug

77819 is the culprit