Bug 134479 – Fix build break on EFL and GTK ports since r170611 and r170614

• Component: WebKit2
• Status: RESOLVED FIXED

[Gyuyoung Kim]

EFL and GTK port have been broken since r170611 and r170614. Those commits were only for mac port.

[Gyuyoung Kim]

Created attachment 234134 [details]
Patch

[Gyuyoung Kim]

Created attachment 234136 [details]
Patch

[Anders Carlsson]

Comment on attachment 234136 [details]
Patch

I think a better solution would be to make encodeLegacySessionHistoryEntryData and decodeLegacySessionHistoryEntryData build for GTK and EFL as well, there's nothing mac specific about that code.

[Gyuyoung Kim]

Created attachment 234140 [details]
Patch

[Gyuyoung Kim]

(In reply to comment #3)
> (From update of attachment 234136 [details])
> I think a better solution would be to make encodeLegacySessionHistoryEntryData and decodeLegacySessionHistoryEntryData build for GTK and EFL as well, there's nothing mac specific about that code.

Andersa, it looks there are two choices we can do. One is to share existing LegacySessionStateCoding.cpp for Mac, Gtk and EFL port. Other is each port has own LegacySessionStateCoding.cpp file. I think it would be good to have own file for each port because each port may has specific code in future. If not, plz let me know.

And, I just add dummy functions to fix build break. I need to have time to implement those function for EFL port. If this patch can be landed, I'm going to start to implement those functions.

[Gyuyoung Kim]

CC'ing KaL as well.

[Gyuyoung Kim]

Created attachment 234144 [details]
Patch

[Gyuyoung Kim]

Created attachment 234146 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 234146 [details]
Patch

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

I wonder if we could add a single LegacySessionStateCodingNone.cpp, since this is mac legacy stuff, I guess EFL and GTK+ will not implement it in any case.

> Source/WebKit2/UIProcess/WebPageProxy.cpp:1858
> -    if (sessionState.provisionalURL) {
> +    if (!sessionState.provisionalURL.isNull()) {

This looks unrelated to the build fix, I guess this bug was introduced in r170627, so I would file a different bug report for this.

[Build Bot]

Comment on attachment 234146 [details]
Patch

Attachment 234146 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4927838788517888

New failing tests:
media/W3C/video/networkState/networkState_during_loadstart.html

[Build Bot]

Created attachment 234148 [details]
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-16  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Gyuyoung Kim]

(In reply to comment #9)
> (From update of attachment 234146 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=234146&action=review
> 
> I wonder if we could add a single LegacySessionStateCodingNone.cpp, since this is mac legacy stuff, I guess EFL and GTK+ will not implement it in any case.

ok, let me add it for EFL and GTK port.

> > Source/WebKit2/UIProcess/WebPageProxy.cpp:1858
> > -    if (sessionState.provisionalURL) {
> > +    if (!sessionState.provisionalURL.isNull()) {
> 
> This looks unrelated to the build fix, I guess this bug was introduced in r170627, so I would file a different bug report for this.

I agree.

[Gyuyoung Kim]

(In reply to comment #12)
> (In reply to comment #9)
> > (From update of attachment 234146 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=234146&action=review
> > 
> > I wonder if we could add a single LegacySessionStateCodingNone.cpp, since this is mac legacy stuff, I guess EFL and GTK+ will not implement it in any case.
> 
> ok, let me add it for EFL and GTK port.

I agree to add LegacySessionStateCodingNone.cpp though, I'm not sure if back/forward can work as before behavior. Because, rr170611 and r170614 removed some existing backforward data.

My first patch is to restore previous patch.
https://bugs.webkit.org/attachment.cgi?id=234134&action=review

[Gyuyoung Kim]

Created attachment 234149 [details]
Patch

[Carlos Garcia Campos]

Is backforward data used for any other thing or only for session save/restore?

[Gyuyoung Kim]

(In reply to comment #15)
> Is backforward data used for any other thing or only for session save/restore?

It seems it was only used for session save/restore.

[Carlos Garcia Campos]

Comment on attachment 234149 [details]
Patch

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

Ok, let's try to fix the build asap. Thanks!

> Source/WebKit2/UIProcess/LegacySessionStateCodingNone.cpp:30
> +#include "NotImplemented.h"

This should be <WebCore/NotImplemented.h>

[Gyuyoung Kim]

Committed r170631: <http://trac.webkit.org/changeset/170631>

[Gyuyoung Kim]

(In reply to comment #17)
> (From update of attachment 234149 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=234149&action=review
> 
> Ok, let's try to fix the build asap. Thanks!
> 
> > Source/WebKit2/UIProcess/LegacySessionStateCodingNone.cpp:30
> > +#include "NotImplemented.h"
> 
> This should be <WebCore/NotImplemented.h>

Ok, I fixed it. Could you take a look Bug 134487 as well ?