Re: [webkit-dev] Approving / Rejects PRs on GitHub when not a reviewer?

2023-11-28 Thread Tim Horton via webkit-dev
+1! The bugzilla-style “unofficial r=me” comment was much clearer for exactly 
these reasons.

> On Nov 28, 2023, at 10:27 AM, Chris Dumez via webkit-dev 
>  wrote:
> 
> Hi,
> 
> Back in the Bugzilla days, only reviewers were allowed to r+ or r- patches. 
> Non-reviewers were - of course - encouraged to do informal reviews but they 
> would do so by leaving comments. They would never r+ / r-.
> 
> Since we’ve moved to Github, it seems we have become a lot more lax about 
> this and I have seen non-reviewers approve and reject PRs, not just leaving 
> comments.
> My understanding is that there is no way to prevent this with Github but 
> could we at least make it a policy that non-reviewers should not approve / 
> reject PRs and only leave comments instead?
> 
> The reason I would like us to make enforce this rule is that I find it 
> confusing. We have a lot of new comers in the project and I do not always 
> know if a person is a reviewer or not yet. I imagine it may be even more 
> confusing for non-Apple people.
> 
> I have in some cases not reviewed patches because I had seen the "green 
> check” and thought the PR already had been approved.
> I have also seen cases of PRs rejected, asking the author to do more work, 
> that I didn’t feel was necessary.
> There is no easy way from the GitHub UI to tell if the person who 
> approved/rejected your PR is actually a reviewer, as far as I know.
> 
> What do you think?
> 
> Thanks,
> Chris Dumez.
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Smart Pointers Usage Guidelines

2023-08-21 Thread Tim Horton via webkit-dev


> On Aug 21, 2023, at 4:39 PM, Ryosuke Niwa  wrote:
> 
> 
>> On Aug 21, 2023, at 4:34 PM, Darin Adler  wrote:
>> 
>> 
>>> On Aug 21, 2023, at 4:31 PM, Tim Horton via webkit-dev 
>>>  wrote:
>>> 
>>>> One subtle thing is that even when a member variable is already Ref / 
>>>> RefPtr / CheckedRef / CheckedPtr, we must create another one in stack as 
>>>> seen here:
>>>> https://commits.webkit.org/267108@main
>>> 
>>> (I asked rniwa to send this mail because this patch surprised me, so I hope 
>>> now we can chat about it).
>>> 
>>> The scope of this rule, and the … lack of elegance … at so many callsites 
>>> worries me a bit. If it’s possible to automate enforcement, that might help 
>>> with part of the problem, but it’s also just really not very pretty, and I 
>>> wonder if someone can come up with some clever alternative solution before 
>>> we go too far down this path (not me!).
>>> 
>>> Alternatively, it’s possible other people OK with this syntax/requirement 
>>> and I should just get over it. What do you all think?
>> 
>> I hope we can make this better by using a getter function that returns a 
>> CheckedPtr so instead of writing CheckedPtr { _page }, we would write page().
> 
> One drawback making a member function return CheckedPtr is that then code 
> like this: `page()->document()->foo()` would cause a ref churn.
> 
> Maybe we don’t care about such ref churns?

How can we tell!

> But then a simpler rule to deploy will be that every function must return 
> CheckedRef/CheckedPtr/Ref/RefPtr.

+1 to that rule instead of the one in Wenson’s patch.

> Alternatively, we could add a new member function which returns CheckedPtr 
> like `pageChecked()`.

Would rather Darin’s plan. I don’t want to have to think about CheckedPtr every 
5 seconds.

> - R. Niwa
> 

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Smart Pointers Usage Guidelines

2023-08-21 Thread Tim Horton via webkit-dev


> On Aug 21, 2023, at 4:34 PM, Darin Adler  wrote:
> 
> 
>> On Aug 21, 2023, at 4:31 PM, Tim Horton via webkit-dev 
>>  wrote:
>> 
>>> One subtle thing is that even when a member variable is already Ref / 
>>> RefPtr / CheckedRef / CheckedPtr, we must create another one in stack as 
>>> seen here:
>>> https://commits.webkit.org/267108@main
>> 
>> (I asked rniwa to send this mail because this patch surprised me, so I hope 
>> now we can chat about it).
>> 
>> The scope of this rule, and the … lack of elegance … at so many callsites 
>> worries me a bit. If it’s possible to automate enforcement, that might help 
>> with part of the problem, but it’s also just really not very pretty, and I 
>> wonder if someone can come up with some clever alternative solution before 
>> we go too far down this path (not me!).
>> 
>> Alternatively, it’s possible other people OK with this syntax/requirement 
>> and I should just get over it. What do you all think?
> 
> I hope we can make this better by using a getter function that returns a 
> CheckedPtr so instead of writing CheckedPtr { _page }, we would write page().

That, for example, seems wildly preferable.

> — Darin

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Smart Pointers Usage Guidelines

2023-08-21 Thread Tim Horton via webkit-dev

> On Aug 21, 2023, at 4:25 PM, Ryosuke Niwa via webkit-dev 
>  wrote:
> 
> Hi all,
> 
> It has been a while since I last announced the plan to adopt smart pointers 
> using clang static analyzer:
> https://lists.webkit.org/pipermail/webkit-dev/2020-September/031386.html
> 
> Here are some updates.
> 
> 
> 1. We’ve made a progress in implementing all the rules including rules for 
> local variables in clang static analyzer:
> https://github.com/WebKit/WebKit/wiki/Smart-Pointer-Usage-Guidelines#rules-for-using-ref-counted-objects
> 
> 
> 2. We also have a new kind of smart pointers: CheckedRef 
>  / 
> CheckedPtr 
> . 
> These behave like Ref and RefPtr in that they increment & decrement a counter 
> in an object but unlike them don’t extend the lifetime of the object. 
> Instead, the destructor of the base object release asserts that there are no 
> live CheckedRef / CheckedPtr left.
> 
> I added a new section in the guide describing when to use each smart pointer 
> type:
> https://github.com/WebKit/WebKit/wiki/Smart-Pointer-Usage-Guidelines#when-to-use-which-smart-pointer
> 
> 
> 3. I wanted to describe what applying these smart pointer rules mean. A lot 
> of code changes needed for this work involves creating Ref / RefPtr / 
> CheckedRef / CheckedPtr in stack:
> https://commits.webkit.org/267082@main
> 
> One subtle thing is that even when a member variable is already Ref / RefPtr 
> / CheckedRef / CheckedPtr, we must create another one in stack as seen here:
> https://commits.webkit.org/267108@main

(I asked rniwa to send this mail because this patch surprised me, so I hope now 
we can chat about it).

The scope of this rule, and the … lack of elegance … at so many callsites 
worries me a bit. If it’s possible to automate enforcement, that might help 
with part of the problem, but it’s also just really not very pretty, and I 
wonder if someone can come up with some clever alternative solution before we 
go too far down this path (not me!).

Alternatively, it’s possible other people OK with this syntax/requirement and I 
should just get over it. What do you all think?

> This is because these member variables can be cleared during the course of 
> invoking a non-trivial function; or put it another way, it’s not immediately 
> obvious from the code inspection that the object pointed to stays alive over 
> the course of a non-trivial function call.
> 
> - R. Niwa
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] OK to flatten WTF's header directory?

2022-02-01 Thread Tim Horton via webkit-dev


> On Feb 1, 2022, at 12:30 PM, Elliott Williams via webkit-dev 
>  wrote:
> 
> Hi webkit-dev,
> 
> I’m working on fixing some ambiguities in our Xcode projects to permit 
> adoption of Xcode’s new build system and better parallelize our builds. I 
> noticed that WTF’s headers (/usr/lib/include/wtf) are atypical in that they 
> aren’t copied into a single directory – they’re deeply nested and mirror the 
> same directory hierarchy as exists in WTF’s sources.
> 
> Is this intentional, and if not, would there be opposition to me flattening 
> them into a single directory? Flattening them makes it easier to explain to 
> Xcode what headers go where, which is useful for tracking dependencies 
> between targets and ensuring proper build order. Headers would still be in 
> their same source locations (alongside implementation files) but would be 
> copied to the top-level /usr/lib/include/wtf directory at build time.

Question: If they're flattened in the SDK, and not flattened in the source 
tree, which include path do we use when including a WTF header in e.g. WebCore?

Right now you say `#include `. In your world, would you 
say `#include `?

If no, how does that work in the case where you only have WTF from the SDK?

If yes, how does that work for the other build systems that are building with 
the full source tree? (maybe we have all directories in WTF listed as header 
search paths, or something? or maybe they look in `usr/lib/include/wtf` in the 
build directory?)

> The only other place in our projects [1] I’m aware of with deeply-nested 
> headers is PAL, and there’s a bug to change that: 
> https://bugs.webkit.org/show_bug.cgi?id=175705 
> 
> 
> -Elliott
> 
> [1]: libwebrtc also produces deeply-nested headers, but since it’s a 
> third-party project, I don’t think our header conventions should apply.
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


[webkit-dev] Aditya Keerthi is now a WebKit reviewer!

2021-10-28 Thread Tim Horton via webkit-dev
Hello!

I am delighted to announce that Aditya Keerthi is now a WebKit reviewer.

Now it's time for all of you to send him tricky patches and see how he does :)

Congratulations, Aditya!

--Tim
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] New EWS Non-Unified builder

2021-04-29 Thread Tim Horton via webkit-dev


> On Apr 29, 2021, at 9:02 PM, Darin Adler  wrote:
> 
>> On Apr 29, 2021, at 9:01 PM, Tim Horton  wrote:
>> 
>> This is a huge problem for people on the Apple platforms using the Xcode 
>> projects. Nearly every time someone adds a file they have to add random 
>> headers to unrelated code.
> 
> OK, sorry, I must be out of touch with the rest of you about how difficult 
> this is. I’ve done that many times, and it was always easy for me.

I'm not saying it's hard (depending on your familiarity with the code that 
breaks, or if it's one of those 'using namespace' situations, or something even 
more subtle), but it is definitely highly annoying, and judging by the number 
of times I (and I'm sure others too) have had to explain what's up to other 
people it seems like "huge" is a only-maybe-slightly-overstated estimate :) 
Maybe others disagree!

> — Darin

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] New EWS Non-Unified builder

2021-04-29 Thread Tim Horton via webkit-dev


> On Apr 29, 2021, at 7:15 PM, Darin Adler via webkit-dev 
>  wrote:
> 
>> On Apr 29, 2021, at 12:04 PM, Ross Kirsling via webkit-dev 
>>  wrote:
>> 
>> Yeah, I think it's important to clarify that nobody is "using 
>> non-Unified-Source building for their development", at least to my 
>> knowledge. Being broken by the shifting sands of unified sources is an 
>> everybody problem (or at the very least an "everybody that builds via CMake 
>> problem", which is ultimately an everybody problem).
> 
> How is this a bigger problem in CMake than for people on the Apple platforms 
> who are using Xcode projects? Can we create greater parity between the two?

I don't follow. This is a huge problem for people on the Apple platforms using 
the Xcode projects. Nearly every time someone adds a file they have to add 
random headers to unrelated code. 

> — Darin
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Having trouble building

2019-10-05 Thread Tim Horton



> On Oct 4, 2019, at 8:55 PM, Ken Russell  wrote:
> 
> Hi,
> 
> Recently I upgraded to Xcode 11.0 and re-synced to top-of-tree WebKit. With a 
> clean WebKitBuild directory I'm seeing the following build error:

What OS are you on?

You might want to peek at https://bugs.webkit.org/show_bug.cgi?id=199705

> $ ./Tools/Scripts/build-webkit --release > build-log.txt
> 
> ...
> 
> In file included from 
> /Users/kbr/src/WebKit/WebKitBuild/Release/DerivedSources/WebKitLegacy/unified-sources/UnifiedSource20-mm.mm:4:
> /Users/kbr/src/WebKit/Source/WebKitLegacy/mac/WebView/WebHTMLView.mm:222:5: 
> error: use of undeclared identifier '_subviews'; did you mean 'subviews'?
> _subviews = subviews;
> ^
> subviews
> /Users/kbr/src/WebKit/Source/WebKitLegacy/mac/WebView/WebHTMLView.mm:220:63: 
> note: 'subviews' declared here
> - (void)_setSubviewsIvar:(NSMutableArray<__kindof NSView *> *)subviews {
>   ^
> /Users/kbr/src/WebKit/Source/WebKitLegacy/mac/WebView/WebHTMLView.mm:222:15: 
> error: explicitly assigning value of variable of type 
> 'NSMutableArray<__kindof NSView *> *' to itself [-Werror,-Wself-assign]
> _subviews = subviews;
> ~ ^ 
> /Users/kbr/src/WebKit/Source/WebKitLegacy/mac/WebView/WebHTMLView.mm:228:30: 
> error: use of undeclared identifier '_subviews'
> return (NSMutableArray *)_subviews;
>  ^
> 3 errors generated.
> 
> 
> 
> Has anyone else run into this?
> 
> Thanks,
> 
> -Ken
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Moving to Python 3

2019-07-12 Thread Tim Horton


> On Jul 12, 2019, at 12:18 PM, Jonathan Bedard  wrote:
> 
> Hello WebKit developers,
> 
> Now that the Catalina developer seeds are available, it is official that the 
> new Mac developer tools come with Python 3. As a result, we need to continue 
> the ongoing discussion about migrating our Python 2.7 scripts to Python 3.
> 
> I propose that, over the next 9 months, we do the following:
> 
> 1. Make any no-cost Python 3 compatibility changes, in particular
> - print foo -> print(foo)
> - import .foo -> import webkitpy.foo
> 2. Convert any scripts not used in automation to Python 3 ASAP (scripts like 
> bisect-builds, block-spammers, compare-results)
> 3. Make most Python 3 compatibility changes which sacrifice efficiency, 
> subject to a case-by-case audit. These would be things like:
> - dict.iteritems() -> dict.items()
> - dict.items() -> list(dict.items())
> 4. Install Python 3 on macOS Sierra and Mojave bots
> 5. Convert peripheral automation scripts to Python 3 1-by-1 (scripts like 
> clean-webkit, merge-results-json, webkit-patch)
> 6. Convert testing scripts and webkitpy to Python 3 in a single change
> 
> The trouble I foresee us encountering with any scheme which attempts a 
> conversion which retains both Python 2.7 and Python 3 compatibility is code 
> like this:
> 
> for expectation_string, expectation_enum in 
> test_expectations.TestExpectations.EXPECTATIONS.iteritems():
> ...
> 
> In this code, the EXPECTATIONS dictionary is thousands of elements long. In 
> Python 2.7, iteritems() gives us an iterator instead of creating a new list, 
> like items() would. In Python 3, iteritems() doesn’t exist, but items() does, 
> and now gives us an iterator instead of creating a new list. The trouble here 
> is that, in this case, creating a new list will be very expensive, expensive 
> enough that we might manage to impact the testing run. There isn’t really an 
> elegant way around this problem if we want to support both Python 2.7 and 
> Python 3, other than defining different code paths for each language.

The official Python 3 transition documentation has a fairly elegant solution to 
this, actually??

https://legacy.python.org/dev/peps/pep-0469/

See "Migrating to the common subset of Python 2 and 3” — you define different 
iteritems() helpers in the two cases. Seems pretty reasonable to me.

> There are other small gotchas as well. For example, ‘%’ is no longer a 
> protected character, which can actually change the behavior of regexes. 
> That’s why I think it’s better to just try and directly convert things 
> instead of attempting to be compatible with both Python 2.7 and Python 3.
> 
> Jonathan
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Which WTF headers still should be in C++11?

2019-06-19 Thread Tim Horton
This was a hopefully-shortlived internal complication. I think this is already 
fixed and we could probably revert this change. Is that right, Alex?

> On Jun 19, 2019, at 7:38 PM, Fujii Hironori  wrote:
> 
> Hi,
> 
> wtf/CheckedArithmetic.h has been converted from C++14 to C++11.
> 
> Bug 195187 – Change CheckedArithmetic to not use std::enable_if_t.
> https://bugs.webkit.org/show_bug.cgi?id=195187 
> 
> Which WTF headers still should be in C++11?
> 
> - Fujii
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Requesting feedback about EWS comments on Bugzilla bugs

2019-06-15 Thread Tim Horton


> On Jun 15, 2019, at 21:13, Aakash Jain  wrote:
> 
> 
> Hi Everyone,
> 
> I am gathering feedback about EWS - specially about the comments which EWS 
> makes on the Bugzilla bugs. Currently, the comments are not very 
> user-friendly/polished/readable and are sometimes very noisy (e.g.: 72 
> comments and 36 attachments by EWS in 
> https://bugs.webkit.org/show_bug.cgi?id=177484). I am working on improving 
> them and looking for specific ideas/feedback.
> 
> Few ideas which I am considering:
> 
> 1) Do not upload archive (for layout-test-results) on bugzilla, instead 
> upload it to another server, unzip it and post a link to the results.html.
> Pros:
> a) Engineers won't have to download the attachment, unzip it, look for 
> failures, and then delete it from their disk. They can simply click the url 
> to view the results. 

This would be nice!

> b) This approach will also reduce 2 comments per failure to 1 comment. 
> Currently there are two comments per failure, one for failure details, second 
> for bugzilla attachment.
> 
> 2) Aggregate comments from multiple queues.
> Pros: less noise
> Cons: comments would get delayed while waiting for results from other queues. 
> (Also might be little complex to implement)

Maybe post a comment that says just “some queues failed” when the first one 
fails, with a link to page on another server that lists the failures (and 
pending queues)? That way you can still have only one comment and do your 
coalescing, but it doesn’t have to wait for all to be done before commenting?

> 3) Improve the text of the comments to make them more readable (specific 
> ideas are welcome).
> 
> 4) When a patch becomes 'obsolete', tag the corresponding EWS comments as 
> 'obsolete', so that they will be hidden.

This would be great.

> 5) Do not comment on bugzilla bug at all, instead send email to the author of 
> the patch.
> Pros: less noisy, also this will allow to include more detailed information 
> about the failure in email.
> Cons: reviewers would have to click status-bubbles to see the failures, 
> failure information is not immediately present in the comments.

I feel like “I have to click the status bubbles” isn’t so bad. That’s what we 
do for build failures (they don’t comment) and nobody complains about that...

> What do you guys think?
> 
> Thanks
> Aakash
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Unified source builds and adding or removing files...

2019-03-19 Thread Tim Horton
We already had this discussion; see the thread titled "Unified sources have 
broken our #include hygiene"

> On Mar 19, 2019, at 3:48 PM, Said Abou-Hallawa  wrote:
> 
> I have been working on patches that require adding and removing cpp files 
> from WebCore/Sources.txt. Almost every time I add or remove a file, I hit 
> undefined symbol compilation error in some unrelated source or header file. 
> Because a group of source files are compiled in one unified source file, some 
> dependencies get hidden.  The same symbol is defined in another source or 
> header file. Once sources are moved to different unified sources, the problem 
> gets uncovered.
> 
> For example the file svg/graphics/filters/filters/SVGFEImage.h uses the class 
> TreeScope without forward declaring it or including its header file. Oddly 
> the source file svg/graphics/filters/filters/SVGFEImage.cpp compiles in the 
> trunk right now. If a file is added to or removed from WebCore/Sources.txt 
> such that this source is moved to another unified source, the compiler will 
> give an error that TreeScope is not defined.
> 
> Another example is  > which fixes a compilation error 
> on GTK port. The same file was compiling fine on macOS but it failed on GTK.
> 
> Can we fix this issue? The fixes for such errors look very mysterious in the 
> patches. It can also cause build breaks because the ports do not compile the 
> same files.
> 
> One naive solution is to have the EWS bots compile without the unified source 
> builds. In this case, all the header and source files must have the required 
> forward declaration and/or the header file inclusion. So adding or removing 
> files should not affect the ability to compile any other source file.
> 
> Thanks,
> Said
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Code Style: Opinion on returning void

2019-02-07 Thread Tim Horton


> On Feb 7, 2019, at 12:47 PM, Daniel Bates  wrote:
> 
> Hi all,
> 
> Something bothers me about code like:
> 
> void f();
> void g()
> {
> if (...)
> return f();
> return f();
> }

⸘do people do this‽

> I prefer:
> 
> void g()
> {
> if (...) {
> f();
> return
> }
> f();
> }
> 
> Does it bother you? For the former? For the latter? Update our style guide?

+1 to a style guide update

> Opinions, please.
> 
> Dan
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] WebKit Commit Queue blocked by build errors in ToT

2018-12-07 Thread Tim Horton


> On Dec 7, 2018, at 08:55, Chris Fleizach  wrote:
> 
> To confirm, this is what is expected? Is WebKit the root namespace?
> 
> namespace WebKit {
> using namespace WebCore;
> using namespace WebKit;

No, that’s fine. It would be outside of that.

> 
> Thanks
> 
> 
>> On Dec 7, 2018, at 8:45 AM, Chris Fleizach  wrote:
>> 
>> Looks like there's quite a few of these. Will try to get them all
>> 
>>> On Dec 7, 2018, at 8:37 AM, Tim Horton  wrote:
>>> 
>>> Someone in UnifiedSource28-mm.mm is “using namespace WebCore” outside of 
>>> the root namespace.
>>> 
>>>>> On Dec 7, 2018, at 08:31, Chris Fleizach  wrote:
>>>>> 
>>>> I'm hitting this tougher one now. Namespace conflict between Rect in 
>>>> Carbon and WebCore::Rect
>>>> 
>>>> Any ideas?
>>>> Thanks for your help
>>>> 
>>>> n file included from 
>>>> /Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/WebKit2/unified-sources/UnifiedSource28-mm.mm:6:
>>>> In file included from 
>>>> /Volumes/Data/EWS/WebKit/Source/WebKit/UIProcess/Plugins/mac/PluginInfoStoreMac.mm:32:
>>>> In file included from 
>>>> /Volumes/Data/EWS/WebKit/Source/WebKit/Shared/Plugins/Netscape/NetscapePluginModule.h:34:
>>>> In file included from 
>>>> /Volumes/Data/EWS/WebKit/WebKitBuild/Release/WebCore.framework/PrivateHeaders/npruntime_internal.h:28:
>>>> In file included from 
>>>> /Volumes/Data/EWS/WebKit/WebKitBuild/Release/WebCore.framework/PrivateHeaders/npapi.h:82:
>>>> In file included from 
>>>> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/System/Library/Frameworks/Carbon.framework/Headers/Carbon.h:29:
>>>> In file included from 
>>>> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/System/Library/Frameworks/Carbon.framework/Frameworks/HIToolbox.framework/Headers/HIToolbox.h:35:
>>>> In file included from 
>>>> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/System/Library/Frameworks/Carbon.framework/Frameworks/HIToolbox.framework/Headers/HIToolbar.h:26:
>>>> In file included from 
>>>> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/System/Library/Frameworks/Carbon.framework/Frameworks/HIToolbox.framework/Headers/Menus.h:22:
>>>> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/System/Library/Frameworks/Carbon.framework/Frameworks/HIToolbox.framework/Headers/Appearance.h:1373:3:
>>>>  error: reference to 'Rect' is ambiguous
>>>>   Rectbounds;
>>>>   ^
>>>> In file included from 
>>>> /Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/WebKit2/unified-sources/UnifiedSource28-mm.mm:1:
>>>> In file included from 
>>>> /Volumes/Data/EWS/WebKit/Source/WebKit/WebKit2Prefix.h:45:
>>>> In file included from 
>>>> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/System/Library/Frameworks/CoreFoundation.framework/Headers/CoreFoundation.h:43:
>>>> In file included from 
>>>> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/System/Library/Frameworks/CoreFoundation.framework/Headers/CFBase.h:77:
>>>> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/usr/include/MacTypes.h:550:41:
>>>>  note: candidate found by name lookup is 'Rect'
>>>> typedef struct Rect Rect;
>>>> ^
>>>> In file included from 
>>>> /Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/WebKit2/unified-sources/UnifiedSource28-mm.mm:1:
>>>> In file included from 
>>>> /Volumes/Data/EWS/WebKit/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:95:
>>>> /Volumes/Data/EWS/WebKit/WebKitBuild/Release/WebCore.framework/PrivateHeaders/Rect.h:60:7:
>>>>  note: candidate found by name lookup is 'WebCore::Rect'
>>>> class Rect final : public RectBase, public RefCounted {
>>>>   ^
>>>> 
>>>> 
>>>>> On Dec 6, 2018, at 4:44 PM, Chris Fleizach  wrote:
>>>>> 
>>>>> On it
>>>>> 
>>>>>> On Dec 6, 20

Re: [webkit-dev] WebKit Commit Queue blocked by build errors in ToT

2018-12-07 Thread Tim Horton
Someone in UnifiedSource28-mm.mm is “using namespace WebCore” outside of the 
root namespace.

> On Dec 7, 2018, at 08:31, Chris Fleizach  wrote:
> 
> I'm hitting this tougher one now. Namespace conflict between Rect in Carbon 
> and WebCore::Rect
> 
> Any ideas?
> Thanks for your help
> 
> n file included from 
> /Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/WebKit2/unified-sources/UnifiedSource28-mm.mm:6:
> In file included from 
> /Volumes/Data/EWS/WebKit/Source/WebKit/UIProcess/Plugins/mac/PluginInfoStoreMac.mm:32:
> In file included from 
> /Volumes/Data/EWS/WebKit/Source/WebKit/Shared/Plugins/Netscape/NetscapePluginModule.h:34:
> In file included from 
> /Volumes/Data/EWS/WebKit/WebKitBuild/Release/WebCore.framework/PrivateHeaders/npruntime_internal.h:28:
> In file included from 
> /Volumes/Data/EWS/WebKit/WebKitBuild/Release/WebCore.framework/PrivateHeaders/npapi.h:82:
> In file included from 
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/System/Library/Frameworks/Carbon.framework/Headers/Carbon.h:29:
> In file included from 
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/System/Library/Frameworks/Carbon.framework/Frameworks/HIToolbox.framework/Headers/HIToolbox.h:35:
> In file included from 
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/System/Library/Frameworks/Carbon.framework/Frameworks/HIToolbox.framework/Headers/HIToolbar.h:26:
> In file included from 
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/System/Library/Frameworks/Carbon.framework/Frameworks/HIToolbox.framework/Headers/Menus.h:22:
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/System/Library/Frameworks/Carbon.framework/Frameworks/HIToolbox.framework/Headers/Appearance.h:1373:3:
>  error: reference to 'Rect' is ambiguous
>   Rectbounds;
>   ^
> In file included from 
> /Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/WebKit2/unified-sources/UnifiedSource28-mm.mm:1:
> In file included from 
> /Volumes/Data/EWS/WebKit/Source/WebKit/WebKit2Prefix.h:45:
> In file included from 
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/System/Library/Frameworks/CoreFoundation.framework/Headers/CoreFoundation.h:43:
> In file included from 
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/System/Library/Frameworks/CoreFoundation.framework/Headers/CFBase.h:77:
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/usr/include/MacTypes.h:550:41:
>  note: candidate found by name lookup is 'Rect'
> typedef struct Rect Rect;
> ^
> In file included from 
> /Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/WebKit2/unified-sources/UnifiedSource28-mm.mm:1:
> In file included from 
> /Volumes/Data/EWS/WebKit/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:95:
> /Volumes/Data/EWS/WebKit/WebKitBuild/Release/WebCore.framework/PrivateHeaders/Rect.h:60:7:
>  note: candidate found by name lookup is 'WebCore::Rect'
> class Rect final : public RectBase, public RefCounted {
>   ^
> 
> 
>> On Dec 6, 2018, at 4:44 PM, Chris Fleizach  wrote:
>> 
>> On it
>> 
 On Dec 6, 2018, at 4:44 PM, Andy Estes  wrote:
 
 
 
 On Dec 6, 2018, at 4:37 PM, Chris Fleizach  wrote:
 
 
 
> On Dec 6, 2018, at 4:37 PM, Ryan Haddad  wrote:
> 
> Chris,
> 
> I'm assuming that this is in reference to the patch in 
> https://bugs.webkit.org/show_bug.cgi?id=192373. My guess is that 
> something about the changes to 
> Source/WebKit/WebKit.xcodeproj/project.pbxproj is causing an issue with 
> unified sources.
> 
> The commit-queue itself has been landing other patches without issue and 
> the build isn't broken on trunk bots. CC'ing Tim in case he can help 
> point out the issue.
> 
 
 Yea must be. Any ideas why this wouldn’t build? I moved this .h/.mm to 
 another folder (from Mac -> Cocoa) 
>>> 
>>> Looks like we need to forward-declare class WebKit::SafeBrowsingWarning in 
>>> WebViewImpl.h. Can you try that in your patch and see if that fixes it?
>>> 
>>> Andy
>> 
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
> 
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Unified sources have broken our #include hygiene

2018-09-01 Thread Tim Horton



> On Sep 1, 2018, at 17:14, Simon Fraser  wrote:
> 
> Unified sources allow you to get away without #including all the files you 
> need in a .cpp file, because some earlier file in the group has probably 
> already included them for you.
> 
> This means that when you add a new file to the build, and the unified sources 
> get shuffled around, you end up with a long list of build breakages, some 
> platform-specific, that you can only fix by repeating EWS trials. Here's an 
> example: https://bugs.webkit.org/show_bug.cgi?id=189223. That patch added on 
> new file in Source/WebCore/rendering, which required unrelated #include 
> changes in at least:
> 
> rendering/RenderBlockFlow.cpp:
> rendering/RenderFrame.cpp:
> rendering/RenderImage.cpp:
> 
> How can we solve this? Should we have an EWS that builds non-unified?

Yes!

> Can we somehow have the style checker do #include enforcement?

Certainly not the style checker we have now!

> Simon
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Experimental features enabled by default

2018-06-27 Thread Tim Horton
Alright, how’s it looking now? :)

> On Jun 26, 2018, at 19:06, Tim Horton  wrote:
> 
> 
> 
>> On Jun 26, 2018, at 18:55, Michael Catanzaro  wrote:
>> 
>> 
>> Hi,
>> 
>> We're a lot closer after r233227!
>> 
>> There is still IsSecureContextAttributeEnabled. Shall we switch the default 
>> value of that one to use DEFAULT_EXPERIMENTAL_FEATURES_ENABLED instead of 
>> true?
> 
> Not sure, waiting for other people to get back to me. There’s some indication 
> that this one, indeed, should follow DEFAULT_EXPERIMENTAL_FEATURES_ENABLED, 
> but I want to make sure :)
> 
>> We should also move CSSAnimationTriggersEnabled to the experimental features 
>> section of the file, and add the category tag. FullScreenEnabled and 
>> EncryptedMediaAPIEnabled should also be moved down in the file.
> 
> Will have to peek at these!
> 
>> Then we can just update the comment to indicate that custom values like 
>> DEFAULT_SERVICE_WORKERS_ENABLED are fine as long as they're thoughtful and 
>> consider ENABLE(EXPERIMENTAL_FEATURES), and that will take care of the other 
>> cases.
> 
> Makes sense.
> 
>> Michael
>> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Experimental features enabled by default

2018-06-26 Thread Tim Horton


> On Jun 26, 2018, at 18:55, Michael Catanzaro  wrote:
> 
> 
> Hi,
> 
> We're a lot closer after r233227!
> 
> There is still IsSecureContextAttributeEnabled. Shall we switch the default 
> value of that one to use DEFAULT_EXPERIMENTAL_FEATURES_ENABLED instead of 
> true?

Not sure, waiting for other people to get back to me. There’s some indication 
that this one, indeed, should follow DEFAULT_EXPERIMENTAL_FEATURES_ENABLED, but 
I want to make sure :)

> We should also move CSSAnimationTriggersEnabled to the experimental features 
> section of the file, and add the category tag. FullScreenEnabled and 
> EncryptedMediaAPIEnabled should also be moved down in the file.

Will have to peek at these!

> Then we can just update the comment to indicate that custom values like 
> DEFAULT_SERVICE_WORKERS_ENABLED are fine as long as they're thoughtful and 
> consider ENABLE(EXPERIMENTAL_FEATURES), and that will take care of the other 
> cases.

Makes sense.

> Michael
> 
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Experimental features enabled by default

2018-06-20 Thread Tim Horton
Resending from the right address.

> On Jun 20, 2018, at 11:12 AM, Michael Catanzaro  wrote:
> 
> Hi,

Apple folks have been chatting about this elsewhere, so it’s a humorous 
coincidence to see this here at the same time.

> I noticed in glancing over WebPreferences.yaml that we have a large number of 
> experimental features enabled by default. There is a comment header 
> indicating that the only allowed values are false or 
> DEFAULT_EXPERIMENTAL_FEATURES_ENABLED, so true is disallowed, but that's 
> being ignored in many cases. It's become a bit difficult to notice the 
> comment header because the experimental feature list has grown quite large in 
> the past year or two.
> 
> Background: DEFAULT_EXPERIMENTAL_FEATURES_ENABLED is enabled by default in 
> the XCode build (because Apple disables experimental features on branches, 
> not trunk), disabled by default in the CMake build (to prevent experimental 
> features from sneaking into GTK and WPE releases), and finally enabled by 
> build-webkit (so experimental features are always enabled for bots and 
> developers).
> 
> I'm planning to change the default values of the following features from true 
> (not allowed for experimental features) to 
> DEFAULT_EXPERIMENTAL_FEATURES_ENABLED:
> 
> CacheAPIEnabled
> ConstantPropertiesEnabled
> CrossOriginWindowPolicySupportEnabled
> IsSecureContextAttributeEnabled
> StorageAccessAPIEnabled
> SubresourceIntegrityEnabled
> RestrictedHTTPResponseAccess
> CrossOriginResourcePolicyEnabled
> StorageAccessPromptsEnabled
> 
> But I wonder if this is OK for all of the above features, as no doubt the 
> intention behind using true was to make the feature always enabled.

It’s definitely not OK, no.

I think we have some sorting out of this to do, I think it would be appreciated 
if you could hold off for a little bit. Sorry for the mess!

> So if any of the above features are no longer experimental and should be 
> enabled by default on all ports, please speak up so they can be graduated out 
> of the experimental features category. My recommended sanity-check is that if 
> the feature is ready for all Safari users and not just Tech Preview users, 
> then it's probably no longer experimental.
> 
> A couple more oddities:
> 
> I'll mark CSSAnimationTriggersEnabled as experimental, since it uses 
> DEFAULT_EXPERIMENTAL_FEATURES_ENABLED and is clearly intended to be.
> 
> For DisabledAdaptationsMetaTagEnabled, I'm not sure. We could change the 
> default value from DISABLED_ADAPTATIONS_META_TAG_ENABLED to 
> DEFAULT_EXPERIMENTAL_FEATURES_ENABLED, and add a PLATFORM(WATCHOS) condition. 
> I think this should be OK for watchOS. Alternatively, I wonder if it might be 
> better to consider it non-experimental and simply set the default to true 
> (with a PLATFORM(WATCHOS) condition)?
> 
> Michael
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Update WebKit style to better accommodate accepted Obj-C block syntax

2017-10-03 Thread Tim Horton
Whatever we decide, we should probably also get it in 
https://webkit.org/code-style-guidelines/

> On Oct 3, 2017, at 4:51 PM, Megan Gardner  wrote:
> 
> 
> Currently, the webkit style checker does not allow ^{ at the beginning of 
> Obj-C blocks. It demands that there is a space between the ^ and the {. This 
> is not how obj-c code is usually styled, and there is plenty of code already 
> in webkit that is formatted with the carrot and and brace together. This 
> seems to have been changed recently with 
> https://bugs.webkit.org/show_bug.cgi?id=161247. I think we should revisit 
> this, as ^ { is much less common that ^{ and is canonical style in pure Obj-C.
> 
> Megan Gardner
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Are CSS constant properties and viewport fit features still experimental?

2017-09-22 Thread Tim Horton


> On Sep 22, 2017, at 12:08, Michael Catanzaro  wrote:
> 
> Hi,
> 
> I noticed today that in our experimental features list (in 
> WebPreferencesDefinitions.h), CSS constant properties and viewport fit are 
> both enabled unconditionally.
> 
> Should I move them to the non-experimental section of the file and leave them 
> on unconditionally, or are they really still experimental? If they're still 
> experimental then they shouldn't be enabled by default for non-Apple ports. 
> (This doesn't affect Apple ports as those ports disable experimental features 
> in stable branches instead.)

I’m guessing viewport-fit doesn’t affect non-Apple ports… do you support ?

I would say it’s probably reasonable to move viewport-fit to the 
non-experimental features section, and turn constant properties off for other 
ports (they’re still experimental/in-flux).

> Michael
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] RFC: Removing feature flag defaults in build-webkit

2017-09-21 Thread Tim Horton


> On Sep 21, 2017, at 13:25, Michael Catanzaro  wrote:
> 
> On Thu, Sep 21, 2017 at 2:26 PM, Tim Horton  wrote:
>> Sure. And someday I think we should go down that path, but generating the 
>> xcconfigs will be trickier (but also a positive because people already have 
>> trouble with them) because they involve more bizarre conditions. But not 
>> today.
> 
> I strongly support removing FeatureList.pm since it's useless for keeping 
> build definitions in sync between XCode and CMake, and it complicates things 
> unnecessarily for both XCode and CMake.

I don’t intend to remove it entirely yet, because it enables the build-webkit 
arguments to enable/disable features, just the default values. But it’s a start.

> There is one very minor complication. GTK and WPE currently use the 
> FeatureList.pm features as "developer features" since our bots all use the 
> build-webkit script, so we have some features enabled there that we don't 
> have enabled in WebKitFeatures.cmake. I think right now that is just MSE and 
> WebRTC. This means that we'll need to adjust our buildbot configurations to 
> manually enable these features at the same time that FeatureList.pm is 
> removed to avoid breaking all these tests.

OK. Is that something that you could prepare a patch for? I think technically 
that could land before the FeatureList.pm change, right? That would be quite 
helpful.

> Michael
> 

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] RFC: Removing feature flag defaults in build-webkit

2017-09-21 Thread Tim Horton


> On Sep 21, 2017, at 12:04, Konstantin Tokarev  wrote:
> 
> 
> 
> 21.09.2017, 22:01, "Tim Horton" :
>>>  On Sep 21, 2017, at 11:57, Konstantin Tokarev  wrote:
>>> 
>>>  21.09.2017, 21:54, "Tim Horton" :
>>>>  Hi, all!
>>>> 
>>>>  Would anybody mind if build-webkit stopped being a source of default 
>>>> values of feature flags?
>>>> 
>>>>  We have defaults for feature flags in FeatureDefines.xcconfig for Xcode, 
>>>> WebKitFeatures.cmake (and Options*.cmake for platform overrides) for 
>>>> CMake, and FeatureDefines.h for everyone. This is already a mess. But it 
>>>> gets worse!
>>>> 
>>>>  Right now, for both the Xcode and CMake builds, build-webkit overrides 
>>>> those with its own set of defaults (in FeatureList.pm). There is also 
>>>> currently an argument to fall back to the CMake defaults 
>>>> (--default-cmake-features) and ignore build-webkit’s defaults.
>>>> 
>>>>  It doesn’t make sense that build-webkit enables a different set of 
>>>> features than a plain ol’ ‘xcodebuild’ or ‘cmake’ in the root directory. 
>>>> It seems like we’ve mostly tried to keep them in sync, but that’s silly 
>>>> busywork for seemingly little benefit.
>>>> 
>>>>  If we do this, the effective changes would be as follows:
>>>> 
>>>>>  1) The default values of feature flags would be specified in all of the 
>>>>> usual places except FeatureList.pm
>>>>>  2) build-webkit --help will no longer list the default value of feature 
>>>>> flags
>>>>>  3) It will be easier (but still not easy, yet) to reason about the 
>>>>> default value of feature flags, and keep them in sync between different 
>>>>> ways people build
>>>>>  4) --default-cmake-features will go away, because it will always be true
>>>> 
>>>>  And things that wouldn’t change:
>>>> 
>>>>>  1) You will still be able to use --FEATURE, --no-FEATURE, and --minimal 
>>>>> arguments to build-webkit
>>>> 
>>>>  Any objections or things I’ve overlooked?
>>> 
>>>  I think it would be more useful to have a single file (e.g. JSON) with 
>>> options and their default values, and generate all other lists from it.
>> 
>> I agree, but it is a much, much bigger project to get us to 1 list than to 
>> reduce us by 1 list.
> 
> In the past at least Qt port was generating its feature list from 
> FeatureList.pm with a really simple script
> 
> https://trac.webkit.org/browser/webkit/trunk/Tools/qmake/dump-features.pl?rev=156692

Sure. And someday I think we should go down that path, but generating the 
xcconfigs will be trickier (but also a positive because people already have 
trouble with them) because they involve more bizarre conditions. But not today.

>> 
>>>>  —Tim
>>>>  ,
>>>> 
>>>>  ___
>>>>  webkit-dev mailing list
>>>>  webkit-dev@lists.webkit.org
>>>>  https://lists.webkit.org/mailman/listinfo/webkit-dev
>>> 
>>>  --
>>>  Regards,
>>>  Konstantin
> 
> -- 
> Regards,
> Konstantin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] RFC: Removing feature flag defaults in build-webkit

2017-09-21 Thread Tim Horton


> On Sep 21, 2017, at 11:57, Konstantin Tokarev  wrote:
> 
> 
> 
> 21.09.2017, 21:54, "Tim Horton" :
>> Hi, all!
>> 
>> Would anybody mind if build-webkit stopped being a source of default values 
>> of feature flags?
>> 
>> We have defaults for feature flags in FeatureDefines.xcconfig for Xcode, 
>> WebKitFeatures.cmake (and Options*.cmake for platform overrides) for CMake, 
>> and FeatureDefines.h for everyone. This is already a mess. But it gets worse!
>> 
>> Right now, for both the Xcode and CMake builds, build-webkit overrides those 
>> with its own set of defaults (in FeatureList.pm). There is also currently an 
>> argument to fall back to the CMake defaults (--default-cmake-features) and 
>> ignore build-webkit’s defaults.
>> 
>> It doesn’t make sense that build-webkit enables a different set of features 
>> than a plain ol’ ‘xcodebuild’ or ‘cmake’ in the root directory. It seems 
>> like we’ve mostly tried to keep them in sync, but that’s silly busywork for 
>> seemingly little benefit.
>> 
>> If we do this, the effective changes would be as follows:
>> 
>>> 1) The default values of feature flags would be specified in all of the 
>>> usual places except FeatureList.pm
>>> 2) build-webkit --help will no longer list the default value of feature 
>>> flags
>>> 3) It will be easier (but still not easy, yet) to reason about the default 
>>> value of feature flags, and keep them in sync between different ways people 
>>> build
>>> 4) --default-cmake-features will go away, because it will always be true
>> 
>> And things that wouldn’t change:
>> 
>>> 1) You will still be able to use --FEATURE, --no-FEATURE, and --minimal 
>>> arguments to build-webkit
>> 
>> Any objections or things I’ve overlooked?
> 
> I think it would be more useful to have a single file (e.g. JSON) with 
> options and their default values, and  generate all other lists from it.

I agree, but it is a much, much bigger project to get us to 1 list than to 
reduce us by 1 list.

>> 
>> —Tim
>> ,
>> 
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
> 
> 
> -- 
> Regards,
> Konstantin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


[webkit-dev] RFC: Removing feature flag defaults in build-webkit

2017-09-21 Thread Tim Horton
Hi, all!

Would anybody mind if build-webkit stopped being a source of default values of 
feature flags?

We have defaults for feature flags in FeatureDefines.xcconfig for Xcode, 
WebKitFeatures.cmake (and Options*.cmake for platform overrides) for CMake, and 
FeatureDefines.h for everyone. This is already a mess. But it gets worse!

Right now, for both the Xcode and CMake builds, build-webkit overrides those 
with its own set of defaults (in FeatureList.pm). There is also currently an 
argument to fall back to the CMake defaults (--default-cmake-features) and 
ignore build-webkit’s defaults.

It doesn’t make sense that build-webkit enables a different set of features 
than a plain ol’ ‘xcodebuild’ or ‘cmake’ in the root directory. It seems like 
we’ve mostly tried to keep them in sync, but that’s silly busywork for 
seemingly little benefit.

If we do this, the effective changes would be as follows:

1) The default values of feature flags would be specified in all of the usual 
places except FeatureList.pm
2) build-webkit --help will no longer list the default value of feature flags
3) It will be easier (but still not easy, yet) to reason about the default 
value of feature flags, and keep them in sync between different ways people 
build
4) --default-cmake-features will go away, because it will always be true

And things that wouldn’t change:

1) You will still be able to use --FEATURE, --no-FEATURE, and --minimal 
arguments to build-webkit

Any objections or things I’ve overlooked?

—Tim___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Proposal: Remove CSS color-correction property

2015-07-01 Thread Tim Horton

> On Jul 1, 2015, at 23:36, Dirk Schulze  wrote:
> 
> Hi Darin,
> 
>> On Jul 1, 2015, at 5:16 PM, Darin Adler  wrote:
>> 
>> Hi folks.
>> 
>> WebKit has a CSS property named color-correction. It’s still prefixed, so 
>> some would call it -webkit-color-correction and I don’t think it’s yet been 
>> proposed as a CSS standard.
>> 
>> Apple engineers added this a while back so that WebKit could continue 
>> interpret webpage and image colors as device native color space for 
>> performance and to match content from legacy plug-ins. The property allowed 
>> some web content to specify sRGB to get predictable results when correctness 
>> mattered more than performance.
>> 
>> If I’m not mistaken, this optimization is no longer effective on either of 
>> the Apple platforms. I’m pretty sure we always do the color correction. I 
>> suspect no one needs this feature.
> 
> Do you mean that WebKit uses sRGB by default now? I thought the default would 
> still be DeviceRGB.

DeviceRGB became equivalent to sRGB (thus causing WebKit to do color correction 
as if all colors are tagged as sRGB) on OS X a few releases ago. It no longer 
means “the same color space as the display”.

> Greetings,
> Dirk
> 
>> 
>> I suggest we remove the property.
>> 
>> Does anyone know a good reason not to remove it? I won’t necessarily land 
>> the code to remove it right away, but I’d like to get agreement on this now 
>> so I can do that later.
>> 
>> — Darin
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] WEBCORE_EXPORT

2015-02-23 Thread Tim Horton
They also seem like prime candidates for stylebot rules.

> On Feb 23, 2015, at 3:31 PM, Darin Adler  wrote:
> 
> Hooray!
> 
> Lets document these somewhere so people don’t have to read the webkit-dev 
> archives to learn these rules. Maybe in the header that defines 
> WEBCORE_EXPORT? Maybe in a webpage somewhere on webkit.org?
> 
> — Darin
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Abandoned Tests

2015-01-22 Thread Tim Horton

> On Jan 22, 2015, at 14:02, Benjamin Poulain  wrote:
> 
> On 1/21/15 8:48 PM, Brent Fulgham wrote:
>> I’ve been reviewing the various Windows layout tests, and have run across a 
>> few tests that seem to be leftovers from the Chromium project. I think all 
>> ports currently skip these tests:
>> 
>> 1. fast/dom/title-directionality-removeChild.html
>> 2. fast/dom/title-directionality.html
> 
> They come from http://trac.webkit.org/changeset/84199
> That's very much useless. The test was for chromium public API.
> 
> Let's remove!
> 
>> 3. fast/events/drag-image-filename.html
> 
> Looks useful. Probably worth fixing if possible.
> 
>> 4. fast/autoresize
> 
> That's from https://bugs.webkit.org/show_bug.cgi?id=73631
> Let's remove.

[from the right address so may be it will go to the list]

No, I think we should fix these (or reimplement them). We use the FrameView 
autosizing code! And testing it would be awesome.

>> 5. fast/overflow/scrollbar-restored-and-then-locked.html
> 
> Useless too.
> 
>> 
>> If no one is using these tests, or plans to, I would like to remove them and 
>> the disconnected bits of code in DRT/WKTR that were originally meant to 
>> drive this code.
>> 
>> If I don’t hear anything by this time next week, I’ll remove the code.
>> 
>> Thanks,
>> 
>> -Brent
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
>> 
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Is commit-queue broken ?

2014-11-26 Thread Tim Horton
I think the necessary emails have been sent to resolve it, but everyone is on 
vacation :(

I can't commit manually either.

> On Nov 26, 2014, at 19:33, Gyuyoung Kim  wrote:
> 
> Hello,
> 
> It looks WebKit queue doesn't work now. Does anyone check it ?
> 
> Gyuyoung.
> 
> 
> 
> -- 
> Sent from Gyuyoung Iphone
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Port-specific OpenType support

2014-09-14 Thread Tim Horton

On 2014.09.14, at 01:11, Dirk Schulze  wrote:

> 
> On Sep 11, 2014, at 11:03 PM, Martin Robinson  wrote:
> 
>> On Thu, Sep 11, 2014 at 11:44 AM, Myles C. Maxfield  
>> wrote:
>>> My question to WebKit-Dev is: Are there any ports who:
>>> 1) Want continued support for SVG fonts, and
>>> 2) Whose platforms do not support OpenType fonts natively?
>> 
>> WebKitGTK+ supports OpenType natively and I as far as I know we have
>> no problems dropping support for SVG fonts. If SVG fonts are supported
>> by the Mac port though, we will probably still want to maintain
>> support for our own port, if possible.
> 
> SVG Fonts live in the non-platform WebCore space. Dropping support for one 
> platform but keeping the code around for another wouldn’t be of any benefit 
> for WebCore as a whole.

Agreed (but I don’t think the original email was suggesting that; pretty sure 
Myles was just gauging whether there were any ports that don’t have OT support 
but need SVG fonts, which would preclude switching over to the translation 
approach).

> Adobe’s TypeKit team dropped support for SVG Fonts a couple of months ago and 
> got very view complains. OT should be supported for WebKitGTK+ from the 
> beginning and is a better replacement for WebCore’s SVG Font implementation 
> in any possible way. It is much less likely that content for WebKitGTK+ uses 
> SVG Fonts than for content for iOS. And even Apple seems to be willing to 
> drop support now.

As mentioned recently, we probably can’t drop support outright, but if Myles’ 
experiment with translating straightforward (no color/animation/etc.) SVG fonts 
to OpenType and rendering them that way pans out (which it seems like it’s 
going to), we can remove a good deal of scary code and do it that way instead 
(that should cover most-if-not-all of the cases that block us from removing 
support entirely).

> Greetings,
> Dirk
> 
>> 
>> --Martin
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] TestWebKitAPI doesn't initialize WKWebView from command line

2014-08-26 Thread Tim Horton

On 2014.08.26, at 04:44, Daniel Lazarenko  wrote:

> Hello,  
> 
> I’m trying to run the WKWebView API tests using this command:
> ./TestWebKitAPI --gtest_filter=_WKDownload.DownloadRequest
> This binary is located in webkit/WebKitBuild/Debug, I’ve built it using Xcode 
> beta 6 on Yosemite.

You probably want to either use the 'run-api-tests' wrapper, or specify the 
DYLD_FRAMEWORK_PATH environment variable to point to your build products. 
Otherwise you’re probably launching TestWebKitAPI with the system version of 
WebKit.

> This test fails, if I run it from command line.
> It passes, if I run it from Xcode (I can set up the process to run and edit 
> arguments if I edit scheme).
> 
> I’m new to this code, and such behaviour is quite surprising for me.
> 
> To be sure, I’ve placed “puts()” statements before, after and inside a call 
> to [WKWebView initWithFrame:]. This call happens in runTest function in 
> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/Download.mm . If I run it from command 
> line, the puts() inside initWithFrame is not printing anything out.
> 
> What’s the difference? Why does it fail outside Xcode debugger?  
> 
> It also fails if the Xcode debugger is attached to the process that is run 
> from command line.
> 
> Here’s the change I’ve made:
> 
> diff --git a/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm 
> b/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm
> index d6492df..321ac1f 100644
> --- a/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm
> +++ b/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm
> @@ -212,6 +212,7 @@ WKWebView* fromWebPageProxy(WebKit::WebPageProxy& page)
> 
> - (instancetype)initWithFrame:(CGRect)frame
> {
> + puts("WKWebView initWithFrame");
> return [self initWithFrame:frame 
> configuration:adoptNS([[WKWebViewConfiguration alloc] init]).get()];
> }
> 
> diff --git a/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/Download.mm 
> b/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/Download.mm
> index ecadb6d..a442930 100644
> --- a/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/Download.mm
> +++ b/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/Download.mm
> @@ -120,7 +120,9 @@ TEST(_WKDownload, DownloadDelegate)
> 
> static void runTest(id  navigationDelegate, id 
> <_WKDownloadDelegate> downloadDelegate, NSURL *url)
> {
> + puts("before initWithFrame");
> RetainPtr webView = adoptNS([[WKWebView alloc] 
> initWithFrame:NSMakeRect(0, 0, 800, 600)]);
> + puts("after initWithFrame");
> [webView setNavigationDelegate:navigationDelegate];
> [[[webView configuration] processPool] _setDownloadDelegate:downloadDelegate];
> 
> 
> 
> 
> 
> --
> With best regards,
> Daniel Lazarenko
> Developer
> Opera Software ASA
> 
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Please avoid inheriting concrete types over WebCore/WebKit boundary

2014-08-22 Thread Tim Horton

> On Aug 22, 2014, at 11:07, Antti Koivisto  wrote:
> 
> Hi,
> 
> To make callbacks from WebCore to WebKit(2) we have generally used approach 
> where WebCore exports an abstract client interface which is then implemented 
> on the WebKit side.
> 
> More recently there has been some proliferation of a pattern where we inherit 
> directly from a concrete WebCore type and specialize it on the WebKit side, 
> in some cases with a completely different implementation:
> 
> class WebResourceLoadScheduler : public WebCore::ResourceLoadScheduler {
> class GraphicsLayerCARemote final : public WebCore::GraphicsLayerCA {
> etc.
> 
> I don't think this is a good pattern. It makes understanding the code harder 
> as you can no longer reason about it within the WebCore only (the WebCore 
> side implementation you are looking at may be effectively dead code). It also 
> blurs the library boundary (which is still useful at least as long as we 
> support both WebKit and WebKit2). 
> 
> I think we should avoid this pattern in new code and possibly refactor some 
> of the existing examples.

Agreed, I think as the remote layer tree project went on we realized that this 
(all of the WebKit2-side *Remote subclasses) was a huge mistake, and just 
haven’t fixed it yet (not sure how straightforward it will be to fix, either, 
but we should try).

> 
>antti
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Questions about ports

2014-08-06 Thread Tim Horton

> On Aug 5, 2014, at 23:48, Jacques-Olivier  wrote:
> 
> Hi everyone,
> 
> I have a couple of questions regarding the different ports of WebKit:
> 
> 1) I suppose the port used by Safari Mac OS X is the Apple’s mac port (built 
> by default)
> Is there a planing of when a version of WebKit is being publicly released in 
> Safari Mac?

There is, of course, but said planning does not occur in public.

> 2) Which port is being used by Safari iOS and how to build it?

The iOS port is integrated into WebKit trunk. You’ll see lots of PLATFORM(IOS) 
around.

> Is there a release planning?

Not publicly, same as Mac.

> What is the flag to build this port?
> How to test it?

It is not currently possible for third parties to build/test this port.

> 3) Which port is being used to power third party applications on iOS? and how 
> to build it?
> Is there a release planning?
> What is the flag to build this port?
> How to test it?

All the same as above.

> Regards,
> J-O H
> 
> --
> Haché Jacques-Olivier
> R&D Engineer at Temasys Communications Pte Ltd
> Fr : 06 45 85 48 80
> Sg : 9086 3673 
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Comment on the bug & email author/reviewer before reverting a patch

2014-07-09 Thread Tim Horton

> On Jul 9, 2014, at 12:10 PM, Maciej Stachowiak  wrote:
> 
> 
> Could we teach webkitbot to do an appropriate notification with a waiting 
> period? Either as part of rollout or add a new command to do it.

It already does. The “waiting period” is defined by when the person who asked 
for the rollout sets the cq+ bit on the rollout patch.

>  - Maciej
> 
>> On Jul 9, 2014, at 11:40 AM, Ryosuke Niwa  wrote:
>> 
>> Yes.  The point is to do these things before telling webkitbot to rollout a 
>> patch.
>> 
>> - R. Niwa
>> 
>> 
>> On Wed, Jul 9, 2014 at 11:07 AM, Simon Fraser  wrote:
>> On Jul 9, 2014, at 7:43 AM, Ryosuke Niwa  wrote:
>> 
>>> Hi all,
>>> 
>>> This is a friendly remainder that you should
>>> comment on the associated bug
>>> email the patch author and the reviewer who reviewed the patch
>>> before rolling out / reverting a patch.
>> 
>> If you use webkitbot to roll out, is this necessary?
>> 
>> Simon
>> 
>> 
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] PSA: please sort WebCore.exp.in when you change it

2014-07-07 Thread Tim Horton

> On Jul 7, 2014, at 1:51 PM, Bem Jones-Bey  wrote:
> 
> 
> On Jul 7, 2014, at 11:55 , Tim Horton  wrote:
> 
>> 
>>> On Jul 7, 2014, at 11:41 AM, Simon Fraser  wrote:
>>> 
>>> We have a handy script that sorts the export file; please use it whenever 
>>> you modify WebCore.exp.in:
>>> 
>>> ./Tools/Scripts/sort-export-file Source/WebCore/WebCore.exp.in
>> 
>> It’s also smart enough to require no arguments at all, and just sorts all of 
>> them!
> 
> Any reason that we don't have check-webkit-style fail if these files aren't 
> properly sorted?

I don’t think there’s a *reason* besides a lack of implementation. Seems like 
it would be a good thing :)
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] PSA: please sort WebCore.exp.in when you change it

2014-07-07 Thread Tim Horton

> On Jul 7, 2014, at 11:41 AM, Simon Fraser  wrote:
> 
> We have a handy script that sorts the export file; please use it whenever you 
> modify WebCore.exp.in:
> 
> ./Tools/Scripts/sort-export-file Source/WebCore/WebCore.exp.in

It’s also smart enough to require no arguments at all, and just sorts all of 
them!

> It’s smart enough to understand #ifdef groupings and everything!
> 
> Simon
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] PDF: context menu and view mode

2014-04-06 Thread Tim Horton

On Apr 6, 2014, at 2:00 AM, Rüdiger Cordes  wrote:

> Hello list,
>  
> this is my first mail and I want so say "Hi" to you.
>  
> The reason why I entered this list:
> I would like to know why the context menu for opened
> PDFs in Safari has changed after r153570 in r153759.

The context menu for PDFs is not implemented in WebKit, so this list isn’t the 
right place to register complaints — any bugs regarding it should be filed in 
Apple’s bug tracker (bugreport.apple.com).

Thanks!

—Tim

> I have a web application and it is important for me and
> my 400 customers to have the old context menu.
>  
> Also annoying is the way PDFs are displayed.
>  
> Everytime they are displayed at full window width.
> Having a 30" Cinema Display you could imagine
> that I don't want to have a microscope view to PDFs.
>  
> When I change the view to original size or two pages
> it is not remembered. This is also a lack of comfort.
>  
> I am generating PDFs with FPDF and need to control
> many times what I have programmed/generated.
> And when everytime the PDF is displayed in full width
> mode I have to switch to an appropriate view for
> viewing my work every time!
>  
> I hope that you understand my situation and it would
> be nice to have a solution for it.
>  
> Thanks in advance,
> Rüdiger
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] EWS doesn't lie!

2014-02-10 Thread Tim Horton

On Feb 10, 2014, at 12:51 AM, youenn fablet  wrote:

> Is it by design that only mac bots run regression tests?

Certainly not.

> Technical issue?

I think so; look at http://build.webkit.org/dashboard/; the Mac port is the 
only one that’s even close to green enough to make running the tests on EWS 
worthwhile (i.e. insufficiently noisy that new failures are possible to 
distinguish).

> Lack of resources?

Could be this too, but I can’t speak for the other ports. It takes a good 
number of machines to keep up (and Mac starts falling behind as soon as someone 
introduces a flaky test because of EWS’s slightly odd machinery).

> 
> 2014-01-30 9:05 GMT+01:00 Alexey Proskuryakov :
> Hi WebKit hackers,
> 
> It sometimes happens that people land patches despite EWS detecting layout 
> test regressions, especially when these seem too unlikely to believe.
> 
> In my experience, EWS has been very stable recently, and if tester bubbles 
> are red, it almost certainly means that the patch is faulty. Even if they 
> don't turn red, but remain yellow for a long time, there is a good chance 
> that the patch introduces flaky failures, so EWS can't make up its mind about 
> exactly which tests regressed.
> 
> If you click on a yellow bubble, that takes you to a page with additional 
> details, where you can see which tests are failing. I'd like to look into 
> improving how this information is presented at some point in the future, yet 
> even now, it shouldn't be too time consuming to check what's going on.
> 
> For reference, we currently have mac and mac-wk2 EWS bots running regression 
> tests in release mode. Other bots only verify that the patch builds, and 
> don't run tests. No bots use debug mode as far as I know, so debug-only build 
> failures and assertions will not be detected. Please run tests locally to 
> catch as many of those as possible.
> 
> - WBR, Alexey Proskuryakov
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] MathML

2013-12-20 Thread Tim Horton

On Dec 20, 2013, at 4:57 PM, Peter Frane  wrote:

> Why did my email bounce? I'm a subscribed. Thanks.

It didn’t.

> From: pfran...@hotmail.com
> To: webkit-dev@lists.webkit.org
> Date: Sat, 21 Dec 2013 08:15:07 +0800
> Subject: [webkit-dev] MathML
> 
> To whom it may concern,
> 
> I'm the developer of ReforMath, a multi-platform C/C++ library for rendering 
> MathML. You can check the screenshots here: http://reformath.webnode.com/ and 
> http://reformath.weebly.com/.
> 
> I'd like to know whether there are MathML-related opportunities for me in 
> your development team, paid or voluntary. 

This is the development mailing list of the Open Source WebKit project, so 
there are no specific paid opportunities here. That said, if you take a look at 
https://bugs.webkit.org I’m sure you can find MathML-related tasks which you 
can voluntarily contribute to — patches are welcome!

> Looking forward to hearing from you.
> 
> Sincerely,
> 
> Peter Frane, Jr.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Test flakiness.

2013-12-03 Thread Tim Horton

On Dec 3, 2013, at 1:51 PM, Samuel White  wrote:

> Hey everyone,
> 
> I've noticed the following build message:
> 
> Unexpected flakiness: text-only failures (1)
>  platform/mac/accessibility/search-predicate-element-count.html [ Failure 
> Pass ]
> 
> Oddly, when this test is run ALONE (locally of course), it always passes. 
> Yet, sometimes when run with other tests (say all accessibility tests), it 
> fails. This suggests to me that this test should be considered 'flaky' as 
> order matters (which is odd). What are the criteria for marking a test flaky? 
> Thanks.

[from the right address]

It would be ideal if you could figure out *why* the test is flaky. What state 
are the tests that run before it setting that isn’t getting reset? Can you 
narrow it down to a pair of tests?

> - Sam
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Layout Tests Failure

2013-10-09 Thread Tim Horton
On 2013.10.09, at 19:18, Gurpreet Kaur  wrote:

> address space needed by 'datetime.dll' (0x211) is already occupied

http://cygwin.com/faq-nochunks.html#faq.using.fixing-fork-failures

and 

http://cygwin.com/faq-nochunks.html#faq.using.bloda

might be helpful. I’ve seen this before.___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] WebGL :: Win32 => build successful and rendering assumed so => but white page

2013-10-03 Thread Tim Horton
On 2013.10.03, at 09:06, Hugo Machefer  wrote:

> Dear all. Might not the best place  -here-  to describe my issue [Win32], but 
> I don't know where exactly this shall be submitted, as a novice by WebKit. To 
> recap, I managed to compile  sources by Microsoft Visual 
> Studio 2010 using CoreGraphics (ie without Cairo) in Debug mode. To activate 
> WebGL, additionally, I have set: 

The AppleWin port (the one you describe) does not currently support WebGL. Much 
of the underlying mechanism is implemented, but there’s still some work 
required to actually display the result.

> #define ENABLE_WEBGL 1 
> #define WTF_USE_3D_GRAPHICS 1 
> #define WTF_USE_OPENGL 1 
> #define WTF_USE_OPENGL_ES_2 1 
> #define WTF_USE_EGL 1 
> #define WTF_USE_GRAPHICS_SURFACE 1
> 
> I hope/guess this is the right approach => could anyone confirm ? Then LINK 
> turns out to be "fortunate", by adding these libraries : 
> translator_common.lib; libEGL.lib; libGLESv2.lib; preprocessor.lib; 
> translator_glsl.lib; translator_hlsl.lib to WebKit sub-project.
> 
> However, execution fails by http://get.webgl.org. Indeed, HTML content 
> pretends : 
> Your browser supports WebGL
> However: a blank area is displayed in place of expected movic 3D based 
> cube... 
> 
> Apologize in case this concern forsooth comes to break any rule by 
> lists.webkit.org , due to Windows matters...
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Webkit 2 drawing

2013-06-04 Thread Tim Horton

On Jun 4, 2013, at 10:18 PM, Vishvesh Community  wrote:

> Hi All,
>   I have been going through the webkit 2 code. But I am not able to 
> findout how the bitmaps are shared between the webprocess and the UI process. 
> Can anybody point me to the appropriate class or share some details on it?

It depends on the port. Which port are you looking at?

(Mac WebKit2 uses remotely-hosted CoreAnimation layers, FWIW, so it’s kind of 
“magic” from WebKit2’s perspective).

You should look at and around the DrawingArea implementations.

> Thanks and Regards,
> Vishvesh
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Buildsystem cleanup

2013-04-08 Thread Tim Horton

On Apr 8, 2013, at 3:44 PM, Patrick Gansterer  wrote:

>> I wasn’t aware that the build worked without a VS2010 pThreads build. I was 
>> running into issues because of the pThreads dll having embedded manifest 
>> problems earlier. Did you do anything special to circumvent that issue?
> 
> I committed many patches to get rid of pthread as a whole on Windows in the 
> past. Maybe I use a special flag which disables the "remaining" pthread calls 
> on Windows.
> 
>> It won’t take much to switch it over but the problem was originally that I 
>> couldn’t get it to build without a newer compile of the pthreads library. 
>> Thus we were waiting for the pthreads folks to release a newer pthreads dll 
>> before doing the switch over.
> 
> Why not just remove the remaining pthread calls on Windows (if there are) and 
> disable USE(PTHREADS)? I'm willing to do the work here too, to get rid of 
> pthreads on Windows.

Regardless of the cmake issue, ^^ this seems like a valid and useful project.


> -- Patrick
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Re-enabling pixel tests

2013-04-04 Thread Tim Horton
We should probably sort out some of our known and solvable OS-version-dependent 
color profile bugs *before* attempting any mass rebaselines, to avoid having to 
do them twice.

On Apr 4, 2013, at 7:37 PM, Benjamin Poulain  wrote:

> On Thu, Apr 4, 2013 at 7:26 PM, Ryosuke Niwa  wrote:
> Now that all Chromium contributors are gone to work on Blink, I'd expect we 
> will have much lower rate of commits. We've also converted quite few tests to 
> text-only or ref-tests over the years.
> 
> Can we do some mass rebaselines and re-enable pixel tests on Mac port now?
> 
> It's really bad to have no pixel test coverage in WebKit.
> 
> I would really like that.
> 
> Benjamin
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Please don't land patches without rebaselining tests for at least one platform

2013-03-22 Thread Tim Horton

On Mar 22, 2013, at 1:34 PM, Ryosuke Niwa  wrote:

> On Thu, Mar 21, 2013 at 6:43 PM, Ryosuke Niwa  wrote:
> On Thu, Mar 21, 2013 at 6:39 PM, Silvia Pfeiffer  
> wrote:
> On Fri, Mar 22, 2013 at 12:30 PM, Ryosuke Niwa  wrote:
> On Thu, Mar 21, 2013 at 6:05 PM, Maciej Stachowiak  wrote:
> On Mar 21, 2013, at 5:38 PM, Glenn Adams  wrote:
>> On Thu, Mar 21, 2013 at 6:11 PM, Ryosuke Niwa  wrote:
>> On Thu, Mar 21, 2013 at 5:10 PM, Glenn Adams  wrote:
>> 
>> On Thu, Mar 21, 2013 at 5:55 PM, Ryosuke Niwa  wrote:
>> On Thu, Mar 21, 2013 at 4:50 PM, Glenn Adams  wrote:
>> That's my platform, so I have to manage with it.
>> 
>> I do have a Retina MBP too but I don't use it to work on the rendering 
>> engine precisely because of this issue.  It's expected that every 
>> contributor has access to a machine where he/she can run layout tests.  
>> Retina MBP is not such a machine.
>> 
>> Well, it's been working for me.
>> 
>> The fact you appears to be contributing patches without appropriate 
>> rebaselines seems to indicate that it's not working for us.
>> 
>> Oh, please point out a case of "without appropriate rebaseline". Please 
>> point out in the documentation where "appropriate rebaseline" is defined. I 
>> think you are making unwarranted assumptions here. If you can't define or 
>> understand a process where I can contribute using a MBP Retina, then I think 
>> you are imposing an arbitrary, unwarranted restriction on the community. I 
>> have been contributing successfully, ergo, it is working.
>> 
>> Many are contributing WebCore layout and rendering patches using a wide 
>> variety of platforms, not all of which match your platform assumptions. It 
>> is not reasonable to claim they aren't contributing positively or that their 
>> contributions don't work.
> 
> We should definitely make it possible to contribute using a Retina system. 
> Apple's flagship laptops offer Retina displays, and it would be crazy to rule 
> them out as development machines. I'd imagine one day we may want the 
> canonical Mac pixel results to be *only* retina.
> 
> Yes, we should but it isn't today.
> 
> Perhaps one possibility is to make it possible to generate non-Retina pixel 
> results on a Retina system. That seems eminently doable to me, unless there's 
> something I am missing.
> 
> Yeah, Alexey and I were talking about this earlier. We need a some way to 
> force CAGraphics, etc… to behave as if we're in non-Retina MBP. We definitely 
> don't want to check in Retina pixel results.
> 
> Where can I sign up to make this a higher priority. ;-)
> 
> Post a patch on https://bugs.webkit.org/show_bug.cgi?id=93673.
> 
> Tim (thorton) kindly took time to fix this problem in 
> http://trac.webkit.org/changeset/146650 at least for render tree dumps. Pixel 
> tests still do fail for obvious reasons but this is a huge improvement 
> nonetheless.

Obvious reasons == "they fail on 1x machines too", right?

> Thanks Tim!

Sure!

> - R. Niwa
> 

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Proposal for coding guidelines: Do not use fall-through switch cases inside #ifdef's

2012-08-18 Thread Tim Horton

On Aug 18, 2012, at 8:35 PM, Jake  wrote:

> Just a guess but shouldn't the #if line be:
> 
> #if defined(ENABLE_CSS3_TEXT_DECORATION) && 
> defined(ENABLE_CSS3_TEXT_DECORATION)

No, this is supposed to be "if ENABLE_CSS3_TEXT_DECORATION is defined and it is 
defined to true". The line is correct as it stands.

> -Jake
> 
> On Sat, Aug 18, 2012 at 8:20 PM, Tim Horton  wrote:
> 
> 
> On Aug 17, 2012, at 11:57 AM, Bruno Abinader  wrote:
> 
> > On Fri, Aug 17, 2012 at 2:24 PM, Joe Mason  wrote:
> >> What do you mean by "precompiler"?  The preprocessor?  Or is this a 
> >> precompiled headers thing?
> >
> > Indeed, s/precompiler/preprocessor :)
> >
> >>
> >> Are you sure there wasn't a typo in the "case 
> >> CSSPropertyWebkitTextDecorationLine:" line which caused a perfectly normal 
> >> syntax error when CSS3_TEXT_DECORATION was defined?  Or maybe 
> >> CSSPropertyWebkitTextDecorationLine was not defined?
> >
> > There is no typo as far as I've checked. The following was added to
> > Source/WebCore/css/CSSPropertyNames.in:
> >
> > ...
> > #if defined(ENABLE_CSS3_TEXT_DECORATION) && ENABLE_CSS3_TEXT_DECORATION
> > -webkit-text-decoration-line
> > #endif
> > ...
> >
> >> I dislike this.  Repeated code should be avoided. Fallthrough is a widely 
> >> used and accepted technique to do that, ever since the days of C. I can't 
> >> believe there's a compiler that doesn't handle this correctly.
> >
> > +1 on that. I've re-checked the build bot output and it seems the
> > issue is actually caused because CSSPropertyWebkitTextDecorationLine
> > gets undefined. So I'm starting to believe the fail reason is
> > something else skipping from my eyes.
> 
> Without looking at the code, I vaguely think that 
> CSSPropertyWebkitTextDecorationLine comes from a generated file that might 
> not rebuild when it's supposed to. You should try a clean build and see -- 
> there've been a few problems with this recently (I ran into it when switching 
> flexbox and regions/exclusions on and off a lot a few months ago, and a 
> colleague ran into it again recently in a similar situation).
> 
> > The actual bug related to this
> > issue is https://bugs.webkit.org/show_bug.cgi?id=94108 .
> >
> >>
> >>> Where each switch case is handled individually (if you're curious
> >>> about it, it is fixed in bug 90493). Said this, I would like to
> >>
> >> https://bugs.webkit.org/show_bug.cgi?id=90493 is "[chromium] Don't archive 
> >> build files generated by VS2010", and doesn't seem to be related.
> >
> > My bad :/ The right bug is 94093:
> > https://bugs.webkit.org/show_bug.cgi?id=94093 .
> >
> > --
> > Bruno de Oliveira Abinader
> > ___
> > webkit-dev mailing list
> > webkit-dev@lists.webkit.org
> > http://lists.webkit.org/mailman/listinfo/webkit-dev
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
> 

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Proposal for coding guidelines: Do not use fall-through switch cases inside #ifdef's

2012-08-18 Thread Tim Horton


On Aug 17, 2012, at 11:57 AM, Bruno Abinader  wrote:

> On Fri, Aug 17, 2012 at 2:24 PM, Joe Mason  wrote:
>> What do you mean by "precompiler"?  The preprocessor?  Or is this a 
>> precompiled headers thing?
> 
> Indeed, s/precompiler/preprocessor :)
> 
>> 
>> Are you sure there wasn't a typo in the "case 
>> CSSPropertyWebkitTextDecorationLine:" line which caused a perfectly normal 
>> syntax error when CSS3_TEXT_DECORATION was defined?  Or maybe 
>> CSSPropertyWebkitTextDecorationLine was not defined?
> 
> There is no typo as far as I've checked. The following was added to
> Source/WebCore/css/CSSPropertyNames.in:
> 
> ...
> #if defined(ENABLE_CSS3_TEXT_DECORATION) && ENABLE_CSS3_TEXT_DECORATION
> -webkit-text-decoration-line
> #endif
> ...
> 
>> I dislike this.  Repeated code should be avoided. Fallthrough is a widely 
>> used and accepted technique to do that, ever since the days of C. I can't 
>> believe there's a compiler that doesn't handle this correctly.
> 
> +1 on that. I've re-checked the build bot output and it seems the
> issue is actually caused because CSSPropertyWebkitTextDecorationLine
> gets undefined. So I'm starting to believe the fail reason is
> something else skipping from my eyes.

Without looking at the code, I vaguely think that 
CSSPropertyWebkitTextDecorationLine comes from a generated file that might not 
rebuild when it's supposed to. You should try a clean build and see -- there've 
been a few problems with this recently (I ran into it when switching flexbox 
and regions/exclusions on and off a lot a few months ago, and a colleague ran 
into it again recently in a similar situation).

> The actual bug related to this
> issue is https://bugs.webkit.org/show_bug.cgi?id=94108 .
> 
>> 
>>> Where each switch case is handled individually (if you're curious
>>> about it, it is fixed in bug 90493). Said this, I would like to
>> 
>> https://bugs.webkit.org/show_bug.cgi?id=90493 is "[chromium] Don't archive 
>> build files generated by VS2010", and doesn't seem to be related.
> 
> My bad :/ The right bug is 94093:
> https://bugs.webkit.org/show_bug.cgi?id=94093 .
> 
> -- 
> Bruno de Oliveira Abinader
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev