Bug 86498 - [EFL][DRT] LayoutTestController does not implement originsWithApplicationCache
Summary: [EFL][DRT] LayoutTestController does not implement originsWithApplicationCache
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 85585
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-15 10:31 PDT by Jussi Kukkonen (jku)
Modified: 2022-03-01 02:39 PST (History)
9 users (show)

See Also:


Attachments
Patch (6.50 KB, patch)
2012-05-16 01:14 PDT, Jussi Kukkonen (jku)
no flags Details | Formatted Diff | Diff
Patch (6.39 KB, patch)
2012-05-16 03:34 PDT, Jussi Kukkonen (jku)
no flags Details | Formatted Diff | Diff
Patch (6.70 KB, patch)
2012-05-16 04:19 PDT, Jussi Kukkonen (jku)
no flags Details | Formatted Diff | Diff
Patch (6.82 KB, patch)
2012-05-16 11:14 PDT, Jussi Kukkonen (jku)
no flags Details | Formatted Diff | Diff
Patch (6.83 KB, patch)
2012-05-29 02:47 PDT, Jussi Kukkonen (jku)
no flags Details | Formatted Diff | Diff
Patch (6.72 KB, patch)
2012-06-01 03:45 PDT, Jussi Kukkonen (jku)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jussi Kukkonen (jku) 2012-05-15 10:31:42 PDT
EFL's LayoutTestController should implement so we can unskip http/tests/appcache/origins-with-appcache.html.
Comment 1 Jussi Kukkonen (jku) 2012-05-16 01:14:19 PDT
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.
Comment 2 Chris Dumez 2012-05-16 03:02:20 PDT
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
Comment 3 Jussi Kukkonen (jku) 2012-05-16 03:34:00 PDT
Created attachment 142210 [details]
Patch
Comment 4 Jussi Kukkonen (jku) 2012-05-16 03:36:23 PDT
(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.
Comment 5 Chris Dumez 2012-05-16 03:45:12 PDT
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.
Comment 6 Jussi Kukkonen (jku) 2012-05-16 04:19:55 PDT
Created attachment 142218 [details]
Patch

avoid blank line in header as Christophe suggests
Comment 7 Jussi Kukkonen (jku) 2012-05-16 11:14:27 PDT
Created attachment 142306 [details]
Patch

Rebase patch now that blocker landed
Comment 8 Gyuyoung Kim 2012-05-16 17:59:29 PDT
Comment on attachment 142306 [details]
Patch

Looks good to me.
Comment 9 Jussi Kukkonen (jku) 2012-05-29 02:47:24 PDT
Created attachment 144495 [details]
Patch
Comment 10 Jussi Kukkonen (jku) 2012-05-29 02:49:57 PDT
Rebased patch to current master, no real changes.

Adding reviewers from 'webkit-patch suggest-reviewers' to CC.
Comment 11 Chris Dumez 2012-05-30 10:43:51 PDT
LGTM.
Comment 12 Gustavo Noronha (kov) 2012-05-31 06:33:19 PDT
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.
Comment 13 Jussi Kukkonen (jku) 2012-06-01 03:45:01 PDT
Created attachment 145262 [details]
Patch
Comment 14 Jussi Kukkonen (jku) 2012-06-01 03:46:14 PDT
(In reply to comment #13)
> Created an attachment (id=145262) [details]
> Patch

Used copyToVector as kov suggested: using a single index does look nicer.
Comment 15 Michael Catanzaro 2017-03-11 10:38:31 PST
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.