Bug 86498 – [EFL][DRT] LayoutTestController does not implement originsWithApplicationCache

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

[Jussi Kukkonen (jku)]

EFL's LayoutTestController should implement so we can unskip http/tests/appcache/origins-with-appcache.html.

[Jussi Kukkonen (jku)]

Created attachment 142186 [details]
Patch

This depends on bug 85585. I wasn't totally sure what was the right place for the origins-array creation, please let me know if DumpRenderTreeSupportEfl was not appropriate.

[Chris Dumez]

Comment on attachment 142186 [details]
Patch

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

> Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.cpp:44
> +#include <JavaScriptCore/JSRetainPtr.h>

probably not needed

> Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.cpp:757
> +        JSRetainPtr<JSStringRef> originJS(Adopt, JSStringCreateWithUTF8CString(it->get()->databaseIdentifier().utf8().data()));

JSStringRef originJS = JSStringCreateWithUTF8CString(it->get()->databaseIdentifier().utf8().data());

> Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.cpp:758
> +        jsOriginsArray[i++] = JSValueMakeString(context, originJS.get());

jsOriginsArray[i++] = JSValueMakeString(context, originJS);

> Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.h:110
> +

Unneeded blank line

[Jussi Kukkonen (jku)]

Created attachment 142210 [details]
Patch

[Jussi Kukkonen (jku)]

(In reply to comment #2)
> (From update of attachment 142186 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=142186&action=review
> 
> > Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.cpp:44
> > +#include <JavaScriptCore/JSRetainPtr.h>
> 
> probably not needed
> 
> > Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.cpp:757
> > +        JSRetainPtr<JSStringRef> originJS(Adopt, JSStringCreateWithUTF8CString(it->get()->databaseIdentifier().utf8().data()));
> 
> JSStringRef originJS = JSStringCreateWithUTF8CString(it->get()->databaseIdentifier().utf8().data());
> 
> > Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.cpp:758
> > +        jsOriginsArray[i++] = JSValueMakeString(context, originJS.get());
> 
> jsOriginsArray[i++] = JSValueMakeString(context, originJS);


Thanks. This makes all kinds of sense.

> > Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.h:110
> > +
> 
> Unneeded blank line

The previous block is commented "// TextInputController", didn't want to lead the reader into any conclusions about the new function.

[Chris Dumez]

Comment on attachment 142186 [details]
Patch

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

>>> Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.h:110
>>> +
>> 
>> Unneeded blank line
> 
> The previous block is commented "// TextInputController", didn't want to lead the reader into any conclusions about the new function.

Then, probably add it to another section above in the header. It is better to avoid creating new sections for no reason.

[Jussi Kukkonen (jku)]

Created attachment 142218 [details]
Patch

avoid blank line in header as Christophe suggests

[Jussi Kukkonen (jku)]

Created attachment 142306 [details]
Patch

Rebase patch now that blocker landed

[Gyuyoung Kim]

Comment on attachment 142306 [details]
Patch

Looks good to me.

[Jussi Kukkonen (jku)]

Created attachment 144495 [details]
Patch

[Jussi Kukkonen (jku)]

Rebased patch to current master, no real changes.

Adding reviewers from 'webkit-patch suggest-reviewers' to CC.

[Chris Dumez]

LGTM.

[Gustavo Noronha (kov)]

Comment on attachment 144495 [details]
Patch

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

> Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.cpp:686
> +    HashSet<RefPtr<WebCore::SecurityOrigin>, WebCore::SecurityOriginHash> origins;
> +    WebCore::cacheStorage().getOriginsWithCache(origins);
> +
> +    HashSet<RefPtr<WebCore::SecurityOrigin>, WebCore::SecurityOriginHash>::const_iterator it;
> +    HashSet<RefPtr<WebCore::SecurityOrigin>, WebCore::SecurityOriginHash>::const_iterator end = origins.end();
> +
> +    JSValueRef jsOriginsArray[origins.size()];
> +    uint i = 0;
> +    for (it = origins.begin(); it != end; ++it) {
> +        JSStringRef originJS = JSStringCreateWithUTF8CString(it->get()->databaseIdentifier().utf8().data());
> +        jsOriginsArray[i++] = JSValueMakeString(context, originJS);
> +    }

Consider using HashSet::copyToVector and using only the uint counter for more readability.

[Jussi Kukkonen (jku)]

Created attachment 145262 [details]
Patch

[Jussi Kukkonen (jku)]

(In reply to comment #13)
> Created an attachment (id=145262) [details]
> Patch

Used copyToVector as kov suggested: using a single index does look nicer.

[Michael Catanzaro]

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.