Bug 98432 - [WebKit2] Create an API for adding and removing user stylesheets from a page group
Summary: [WebKit2] Create an API for adding and removing user stylesheets from a page ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andy Estes
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-04 11:33 PDT by Andy Estes
Modified: 2022-02-27 23:56 PST (History)
7 users (show)

See Also:


Attachments
Patch (55.09 KB, patch)
2012-10-04 12:23 PDT, Andy Estes
no flags Details | Formatted Diff | Diff
Patch (57.54 KB, patch)
2012-10-04 13:40 PDT, Andy Estes
no flags Details | Formatted Diff | Diff
Patch (56.39 KB, patch)
2012-10-04 20:17 PDT, Andy Estes
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andy Estes 2012-10-04 11:33:34 PDT
[WebKit2] Create an API for adding and removing user stylesheets from a page group
Comment 1 Andy Estes 2012-10-04 12:23:36 PDT
Created attachment 167156 [details]
Patch
Comment 2 Build Bot 2012-10-04 13:29:21 PDT
Comment on attachment 167156 [details]
Patch

Attachment 167156 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14171365
Comment 3 Gyuyoung Kim 2012-10-04 13:31:23 PDT
Comment on attachment 167156 [details]
Patch

Attachment 167156 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14178178
Comment 4 Andy Estes 2012-10-04 13:40:47 PDT
Created attachment 167166 [details]
Patch
Comment 5 Andy Estes 2012-10-04 13:54:26 PDT
Comment on attachment 167166 [details]
Patch

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

> Tools/TestWebKitAPI/Tests/WebKit2ObjC/WKBrowsingContextGroupTest.mm:36
> -#import <WebKit2/WKBrowsingContextGroup.h>
> +#import "PlatformUtilities.h"
> +#import <JavaScriptCore/JSRetainPtr.h>
> +#import <JavaScriptCore/JavaScriptCore.h>
> +#import <WebKit2/WKRetainPtr.h>
> +#import <WebKit2/WKSerializedScriptValue.h>
> +#import <WebKit2/WKViewPrivate.h>
> +#import <WebKit2/WebKit2.h>
> +#import <WebKit2/WebKit2_C.h>

This file was added by mistake. I'll remove it before landing.
Comment 6 Andy Estes 2012-10-04 13:55:15 PDT
Comment on attachment 167166 [details]
Patch

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

>> Tools/TestWebKitAPI/Tests/WebKit2ObjC/WKBrowsingContextGroupTest.mm:36
>> +#import <WebKit2/WebKit2_C.h>
> 
> This file was added by mistake. I'll remove it before landing.

Er, these includes, not this file.
Comment 7 Sam Weinig 2012-10-04 15:40:08 PDT
Comment on attachment 167166 [details]
Patch

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

> Source/WebKit2/Shared/ImmutableArray.h:53
> +        return adoptRef(new ImmutableArray(entries, size, /* shouldAdoptReferences */ false));
> +    }
> +    static PassRefPtr<ImmutableArray> adopt(APIObject** entries, size_t size)
> +    {
> +        return adoptRef(new ImmutableArray(entries, size, /* shouldAdoptReferences */ true));
>      }

It would probably be cleaner to do this with a Tag (See RetainPtr for an example of this) since this is always known at compile time.

> Source/WebKit2/Shared/UserContentContainer.cpp:35
> +class UserContentItemMessageEncoder : public UserMessageEncoder<UserContentItemMessageEncoder> {

I don't think this should be using user message coding, since we know the object graph.
Comment 8 Andy Estes 2012-10-04 20:17:40 PDT
Created attachment 167239 [details]
Patch
Comment 9 Sam Weinig 2012-10-09 21:27:12 PDT
Comment on attachment 167239 [details]
Patch

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

> Source/WebKit2/Shared/ImmutableArray.cpp:35
> +ImmutableArray::ImmutableArray(ImmutableArray::AdoptTag, APIObject** entries, size_t size)

I think you can just call it AdoptTag here.

> Source/WebKit2/Shared/ImmutableArray.cpp:43
> +: m_entries(size)

Please indent.

> Source/WebKit2/Shared/UserContentContainer.cpp:89
> +        vector.uncheckedAppend(adoptRef(WebString::create(string).leakRef()));

Is the leakRef() adoptRef() craziness needed here?.  Could you not just do vector.uncheckedAppend(WebString::create(string))?
Comment 10 Andy Estes 2012-10-10 13:28:27 PDT
Committed r130948: <http://trac.webkit.org/changeset/130948>