Re: [webkit-dev] Style guideline on initializing non-POD types via member initialization
I don’t like style rules that prevent doing useful things, and this seems like a useful thing. -Filip > On Mar 21, 2019, at 2:12 PM, Alex Christensen wrote: > > A more specific example of why I object is that I want to do things like add > a pointer to the thread an object was created on if ASSERT_DISABLED is false > so I can assert if things are done on invalid threads. If I do this in a > class like RefCounted, this rule would make me add a guarded initializer to > every RefCounted class. > >> On Mar 21, 2019, at 1:37 PM, Alex Christensen wrote: >> >> I object. I don’t find using { *this } in a header disorienting at all. I >> think it’s better than adding many duplicate lines in each constructor and >> risking forgetting one. I think if we were to remove all the >> m_attributeOwnerProxy initializers in WebKit it would add lots of >> duplication with little benefit. If it were a class with a default >> constructor we would have a high risk of forgetting a constructor somewhere. >> >>> On Mar 20, 2019, at 9:22 AM, Simon Fraser wrote: >>> On Mar 14, 2019, at 1:06 PM, Filip Pizlo wrote: I like to draw this distinction: is the initializer a constant? It’s not a constant if it relies on arguments to the constructor. “This” is an argument to the constructor. It’s also not a constant if it involves reading the heap. So, like you, I would want to see this code done in the constructor. But I’m not sure that my general rule is the same as everyone’s. >>> >>> This seems like a reasonable proposal to me: only use initializers when >>> their input is constant data. >>> >>> Any objections? >>> >>> Simon >>> -Filip > On Mar 14, 2019, at 12:59 PM, Simon Fraser wrote: > > I've seen some code recently that initializes non-POD members via > initializers. For example, SVGAElement has: > > AttributeOwnerProxy m_attributeOwnerProxy { *this }; > > I find this a little disorientating, and would normally expect to see > this in the constructor as m_attributeOwnerProxy(*this), as it makes it > easier to find places to set breakpoints, and the ordering of > initialization is easier to see. > > Are people OK with this pattern, or should we discourage it via the style > guidelines (and style checker)? > > 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 > > ___ > 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] Style guideline on initializing non-POD types via member initialization
A more specific example of why I object is that I want to do things like add a pointer to the thread an object was created on if ASSERT_DISABLED is false so I can assert if things are done on invalid threads. If I do this in a class like RefCounted, this rule would make me add a guarded initializer to every RefCounted class. > On Mar 21, 2019, at 1:37 PM, Alex Christensen wrote: > > I object. I don’t find using { *this } in a header disorienting at all. I > think it’s better than adding many duplicate lines in each constructor and > risking forgetting one. I think if we were to remove all the > m_attributeOwnerProxy initializers in WebKit it would add lots of duplication > with little benefit. If it were a class with a default constructor we would > have a high risk of forgetting a constructor somewhere. > >> On Mar 20, 2019, at 9:22 AM, Simon Fraser wrote: >> >>> On Mar 14, 2019, at 1:06 PM, Filip Pizlo wrote: >>> >>> I like to draw this distinction: is the initializer a constant? >>> >>> It’s not a constant if it relies on arguments to the constructor. “This” is >>> an argument to the constructor. >>> >>> It’s also not a constant if it involves reading the heap. >>> >>> So, like you, I would want to see this code done in the constructor. But >>> I’m not sure that my general rule is the same as everyone’s. >> >> This seems like a reasonable proposal to me: only use initializers when >> their input is constant data. >> >> Any objections? >> >> Simon >> >>> >>> -Filip >>> On Mar 14, 2019, at 12:59 PM, Simon Fraser wrote: I've seen some code recently that initializes non-POD members via initializers. For example, SVGAElement has: AttributeOwnerProxy m_attributeOwnerProxy { *this }; I find this a little disorientating, and would normally expect to see this in the constructor as m_attributeOwnerProxy(*this), as it makes it easier to find places to set breakpoints, and the ordering of initialization is easier to see. Are people OK with this pattern, or should we discourage it via the style guidelines (and style checker)? 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 ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Style guideline on initializing non-POD types via member initialization
I object. I don’t find using { *this } in a header disorienting at all. I think it’s better than adding many duplicate lines in each constructor and risking forgetting one. I think if we were to remove all the m_attributeOwnerProxy initializers in WebKit it would add lots of duplication with little benefit. If it were a class with a default constructor we would have a high risk of forgetting a constructor somewhere. > On Mar 20, 2019, at 9:22 AM, Simon Fraser wrote: > >> On Mar 14, 2019, at 1:06 PM, Filip Pizlo wrote: >> >> I like to draw this distinction: is the initializer a constant? >> >> It’s not a constant if it relies on arguments to the constructor. “This” is >> an argument to the constructor. >> >> It’s also not a constant if it involves reading the heap. >> >> So, like you, I would want to see this code done in the constructor. But I’m >> not sure that my general rule is the same as everyone’s. > > This seems like a reasonable proposal to me: only use initializers when their > input is constant data. > > Any objections? > > Simon > >> >> -Filip >> >>> On Mar 14, 2019, at 12:59 PM, Simon Fraser wrote: >>> >>> I've seen some code recently that initializes non-POD members via >>> initializers. For example, SVGAElement has: >>> >>> AttributeOwnerProxy m_attributeOwnerProxy { *this }; >>> >>> I find this a little disorientating, and would normally expect to see this >>> in the constructor as m_attributeOwnerProxy(*this), as it makes it easier >>> to find places to set breakpoints, and the ordering of initialization is >>> easier to see. >>> >>> Are people OK with this pattern, or should we discourage it via the style >>> guidelines (and style checker)? >>> >>> 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] RFC: dropping support for x87
Since there were no negative comments in the list, just another extra positive comment in the bug itself, we have landed this: https://trac.webkit.org/changeset/243293/webkit Xan On Mon, Mar 11, 2019 at 7:00 PM Xan wrote: > Hi, > as part of Igalia's work on 32bit support we spent some time trying to > iron out some bugs in JSC/intel 32bit builds. A bunch of them were related > to rounding errors (or more precisely, differences) caused by the extended > precision FP registers used by default in x87; which, in turn, is the > default in GCC when compiling with -m32. > > We discussed a number of solutions, but then we noticed that major > browsers have in recent times already dropped support for x87: > > Firefox: > https://support.mozilla.org/en-US/kb/your-hardware-no-longer-supported > Chrome: https://support.google.com/chrome/a/answer/7100626?hl=en > > This removes a small maintenance burden and prevents future bugs like the > ones we investigated. Also, in practice, intel hardware older than Pentium > 4-era machines is very rare. All this considered, we decided it made sense > to do the same in WebKit. > > In short, this email is a RFC to see if anyone in the WebKit community > objects to this change, or has any comment about it. The bug where we have > attached the patch for this change is: > https://bugs.webkit.org/show_bug.cgi?id=194853 > > Regards, > Xan > ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] MathML Refresh Heads up
On Thu, Mar 21, 2019 at 9:42 AM Frédéric Wang wrote: > On 20/03/2019 20:45, Ryosuke Niwa wrote: > > I agree on the goal but disagree on the suggestion. At minimum, we should > decide each case separately and try to collect some data before. > >> We can continue to agree to disagree on this point. But I strongly object >> to removing any features from MathML implementation of WebKit without >> having done comprehensive compatibility testing with various iOS and macOS >> apps that use MathML. >> >> I'm also happy with more testing with iOS / macOS apps. I was just > concerned by the "for now" and the "there isn't any easy way to do it" in > your initial emails, it seems you were not open to accept any change for an > undetermined period of time, just because of your ports... > Yes, in general, I don't think we should be any feature from WebKit unless there is a good evidence that the removal won't affect any website or apps. This topic comes up of wanting to remove a feature comes up every now and then, and my answer is always that we should never remove a feature. The corollary of this position is that we should never add a feature unless it's absolutely necessary because we can never remove it once it's added. > I guess one way to satisfy your desire without breaking WebKit in iOS and >>> macOS would be to add a runtime feature flag which disables the parts of >>> MathML that's not a part of core MathML, and then only enable this feature >>> in your own port. That would allow you to have the same set of features >>> between your products without breaking our ports. >>> >> > ...however that proposal from you last email sounds more constructive. > That would still be extra burden for us to manage deprecated MathML code > and the corresponding tests, but at least that will give us the opportunity > to start disabling features & run tests without them, to better see which > parts we can ignore when comparing code between web engines and even to > have beta testers providing feedback. So that seems a good trade off until > Apple is ready to accept the changes (or not). > > I really don't care what maintainers of Blink say or do about MathML >>> because they don't have MathML implementation right now. My primary concern >>> as I've stated multiple times is iOS and macOS apps that currently use >>> MathML. >>> >> Again, you are the one who brought up that topic in this thread. If you > really don't care about Blink maintainers or Google's opinion then please > just don't mention them. > Well, you're the one who brought up Chromium in the very first email: > As some of you may know, Igalia is working on MathML support in Chromium > this year I think it would have been best if you didn't mention it if you didn't talk to talk about. > These all seem like something out in the wild might be using. >>> https://github.com/search?q=mathml+fontweight&type=Code yields quite a >>> few examples in which fontweight content attribute is being used for >>> example. >>> >>> MathML is used as an exchange format in various tools, standards and >>> documents so that's not really surprising to get matches. However, the >>> MathML Core spec targets usage in web engines so what we need instead is to >>> check content that is actually served to web engines in general and to >>> WebKit in particular. >>> >>> Note: There are straighforward CSS polyfills for the attributes >>> previously mentioned. A JS polyfill would be needed for MathML nonzero >>> unitless lengths (if they are really used) but removing the possibility to >>> write "5" instead of "500%" is part of the general goal to align more with >>> HTML/CSS. Regarding fontweight, it is known to require some extra >>> code/tests in Gecko/Stylo in order to handle conflicts with mathvariant ( >>> WebKit doesn't handle such conflicts >>> https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/mathml/MathMLElement.cpp#L122 >>> ). >>> >> An existence of a bug in our code is not an evidence that the entire >> feature can be removed. >> > That was not my point. I was just trying to explain that there are more > issues involved when you analyze carefully each case, you cannot just rely > on generic claims, quick searches or unilateral approaches in order to make > a decision. Anyway that was just a side note, it's probably not worth > entering into details on this mailing list. > > Thank you, > > -- > Frédéric Wang > > ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] MathML Refresh Heads up
On 20/03/2019 20:45, Ryosuke Niwa wrote: > > I agree on the goal but disagree on the suggestion. At minimum, we > should decide each case separately and try to collect some data before. > > We can continue to agree to disagree on this point. But I strongly > object to removing any features from MathML implementation of > WebKit without having done comprehensive compatibility testing > with various iOS and macOS apps that use MathML. > I'm also happy with more testing with iOS / macOS apps. I was just concerned by the "for now" and the "there isn't any easy way to do it" in your initial emails, it seems you were not open to accept any change for an undetermined period of time, just because of your ports... > I guess one way to satisfy your desire without breaking WebKit > in iOS and macOS would be to add a runtime feature flag which > disables the parts of MathML that's not a part of core MathML, > and then only enable this feature in your own port. That would > allow you to have the same set of features between your > products without breaking our ports. > > ...however that proposal from you last email sounds more constructive. That would still be extra burden for us to manage deprecated MathML code and the corresponding tests, but at least that will give us the opportunity to start disabling features & run tests without them, to better see which parts we can ignore when comparing code between web engines and even to have beta testers providing feedback. So that seems a good trade off until Apple is ready to accept the changes (or not). > I really don't care what maintainers of Blink say or do about > MathML because they don't have MathML implementation right > now. My primary concern as I've stated multiple times is iOS > and macOS apps that currently use MathML. > Again, you are the one who brought up that topic in this thread. If you really don't care about Blink maintainers or Google's opinion then please just don't mention them. >> These all seem like something out in the wild might be >> using. https://github.com/search?q=mathml+fontweight&type=Code yields >> quite a few examples in which fontweight content attribute is >> being used for example. > > MathML is used as an exchange format in various tools, > standards and documents so that's not really surprising to get > matches. However, the MathML Core spec targets usage in web > engines so what we need instead is to check content that is > actually served to web engines in general and to WebKit in > particular. > > Note: There are straighforward CSS polyfills for the > attributes previously mentioned. A JS polyfill would be needed > for MathML nonzero unitless lengths (if they are really used) > but removing the possibility to write "5" instead of "500%" is > part of the general goal to align more with HTML/CSS. > Regarding fontweight, it is known to require some extra > code/tests in Gecko/Stylo in order to handle conflicts with > mathvariant ( WebKit doesn't handle such conflicts > > https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/mathml/MathMLElement.cpp#L122 > ). > > An existence of a bug in our code is not an evidence that the > entire feature can be removed. > That was not my point. I was just trying to explain that there are more issues involved when you analyze carefully each case, you cannot just rely on generic claims, quick searches or unilateral approaches in order to make a decision. Anyway that was just a side note, it's probably not worth entering into details on this mailing list. Thank you, -- Frédéric Wang ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Licensing terms for content from developer.apple.com
Hi again, On Tue, 19 Feb 2019 18:54:43 +0200, Adrian Perez de Castro wrote: > Lately we have been working on enabling the content extensions feature > in the GTK and WPE WebKit ports [1], and we would like to include in the > generated API documentation a reference of the JSON source format which > WebKit consumes. > > There is one page [2] at “developer.apple.com” which already includes > a description, and I was wondering if it would be possible to copy some > part from it. What is the licensing situation for the contents in that > page? Would it be acceptable to copy the tables with the possible values > for the different objects, and some of the paragraphs, in our documentation? Is there anyone who can comment on this? In a few weeks I'll take silence as a sign that it would be better that we write our documentation for the content extension JSON format from scratch =) Cheers, -Adrián > --- > [1] https://bugs.webkit.org/show_bug.cgi?id=167941 > [2] > https://developer.apple.com/documentation/safariservices/creating_a_content_blocker?language=objc pgpo7pNl4HrSv.pgp Description: PGP signature ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev