Re: [webkit-dev] WebKit's position on @property

2020-04-24 Thread Antti Koivisto
We like declarative @property and will definitely consider implementing it.
We like CSS.registerProperty less since it is a performance footgun.


   antti

On Fri, Apr 24, 2020 at 10:25 AM Anders Hartvoll Ruud 
wrote:

> Thanks for the response. I did read through that issue at some point, but
> I didn't detect an underlying endorsement. :-)
>
> On Fri, Apr 24, 2020 at 12:33 AM Simon Fraser 
> wrote:
>
>> In general we are supporting. We've given some feedback here: <
>> https://github.com/w3c/css-houdini-drafts/issues/940>
>>
>> Simon
>>
>> On Apr 23, 2020, at 2:44 AM, Anders Hartvoll Ruud 
>> wrote:
>>
>> Hi,
>>
>> In Blink there's currently an intent to ship
>> 
>>  @property from CSS Properties & Values
>> .
>> I've noticed that there's an in-progress implementation of
>> CSS.registerProperty in WebKit, and I'm wondering if you're also
>> considering implementing @property, or you have any thoughts on it in
>> general.
>>
>> Anders
>>
>> ___
>> 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] [Styling] Space between [] and () in C++ lambdas

2019-11-02 Thread Antti Koivisto
On Fri, Nov 1, 2019 at 10:50 PM Yusuke Suzuki  wrote:

>
> > On Nov 1, 2019, at 11:53, Michael Catanzaro 
> wrote:
> >
> > On Fri, Nov 1, 2019 at 11:19 am, Ryosuke Niwa  wrote:
> >> Namely, some people write a lambda as:
> >> auto x = [] () { }
> >> with a space between [] and () while others would write it as:
> >> auto x = []() { }
> >
> > : I omit the () when there are no parameters, as in these examples.
> >
> > No preference on spacing.
>
> I like having a space here, because this rule is simpler to me.
> If we always have a space between them, this is clear that the above case
> is written in `[] { }` instead of `[]{ }`.
>

I prefer not having the redundant space in [](). It also makes logical
sense to me to keep the lambda signature together. I started using lambdas
with space there, dropped it later, and suffered no adverse consequences.

As for existing practice, WebCore favors spaceless ]( about 2:1 but across
the entire WebKit it is closer to 1:1.

We always put space before { } block, I don't think that is really in
question here, or creating any inconsistencies.


   antti


>
> -Yusuke
>
> >
> >
> > ___
> > 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] Introducing WTF::makeUnique and WTF::makeUniqueWithoutFastMallocCheck

2019-08-23 Thread Antti Koivisto
On Mon, Aug 19, 2019 at 10:25 AM Yusuke Suzuki  wrote:

> Hello WebKit folks!
>
> I would like to announce that I’ve just landed the patch which introduces
> `WTF::makeUnique` and `WTF::makeUniqueWithoutFastMallocCheck` in
> https://trac.webkit.org/changeset/248846.
> They are drop-in-replacement to std::make_unique, and we should not use
> std::make_unique after that patch is introduced.
> I’m planning to add cpplint check for `std::make_unique` to avoid the use
> of that.
>
> The motivation behind this change is the following.
>
> 1. Our typical way of allocating heap memory is three-fold. Using
> containers (Vector etc.), RefCounted, and std::unique_ptr.
> 2. Containers and RefCounted are covered well by FastMalloc.
> 3. But std::unique_ptr case, we missed using FastMalloc in many places so
> far.
>
> Even in very recently written code, we missed FastMalloc annotation. For
> example, we sometimes create a data structure just like a struct, and
> allocate it with make_unique.
>
> struct XXXData {
> ...
> };
>
> m_data = std::make_unique();
>
> We missed WTF_MAKE_STRUCT_FAST_ALLOCATED annotation in XXXData so
> frequently so that the allocation of XXXData ends up being allocated from
> system-malloc.
>
> This WTF::makeUnique adds one `static_assert` over std::make_unique: the
> static_assert ensures T is FastMalloced or IsoHeap-allocated.
> Otherwise, we see compile-error.
>

Could WTF::makeUnique simply use FastMalloc by default? We could then
remove most of these messy annotations.

This would require replacing std::unique_ptr with a type that knows how to
free the objects correctly (bring back OwnPtr!) but that doesn't seem like
a big deal. It wouldn't play well with mixed use of OwnPtr and new/delete
but that should be avoided in any case.


   antti


> This mechanism surprisingly found so many classes that do not have
> WTF_MAKE_FAST_ALLOCATED / WTF_MAKE_STRUCT_FAST_ALLOCATED in our code base.
>
> If the type T comes from ThirdParty and if we cannot annotate T with
> FAST_ALLOCATED, we can use WTF::makeUniqueWithoutFastMallocCheck explicitly
> as a fallback.
>
> More detailed explanation behind why we took this design (instead of
> allocating FastMalloced-memory automatically when using makeUnique()
> etc.) is described in ChangeLog in
> https://trac.webkit.org/changeset/248846/webkit.
> I already annotated missed structs / classes with WTF_MAKE_FAST_ALLOCATED
> in https://trac.webkit.org/changeset/248762. So, now I think 99% of
> allocations in WebKit-itself are handled well by FastMalloc.
>
> -Yusuke
> ___
> 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 anyone maintaining Nicosia scrolling code?

2019-05-28 Thread Antti Koivisto
On Sat, Feb 23, 2019 at 1:30 PM  wrote:

> On Sat, Feb 23, 2019, at 2:47 AM, Simon Fraser wrote:
> > There’s a bunch of code in Source/WebCore/page/scrolling/nicosia/ that
> > I keep breaking with scrolling tree refactoring. Most of it it stubs.
> > Is anyone maintaining this code, because it obviously doesn’t work, and
> > I’d like to remove the maintenance burden.
> >
>
> I added the stubs there, and was planning to start adding actual
> implementations there in the following weeks, now that a new release cycle
> was just entered.
>
> I wouldn't mind disabling or removing the code temporarily in order to
> enable you an easier time refactoring around it.


Please do. It is unreasonable to push maintenance burden of this dead code
to people who are working on async scrolling. See
https://bugs.webkit.org/show_bug.cgi?id=198292 for an example.


   antti


> But given that the async scrolling capability is something we plan to add
> support for in the WPE and GTK ports, I'd appreciate some input about the
> scope and the timeframe of your work and possibility of sharing as much of
> the implementation as possible between different ports.
>
> Cheers,
> Zan
>
> > 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] Rename AtomicString to AtomString

2019-01-30 Thread Antti Koivisto
Sounds good to me.


  antti

On Wed, Jan 30, 2019 at 6:33 PM Darin Adler  wrote:

> So, did we reach consensus that we should rename AtomicString to
> AtomString?
>
> — 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] Watch out for std::optional's move constructor

2018-12-18 Thread Antti Koivisto
On Tue, Dec 18, 2018 at 3:18 AM Geoffrey Garen  wrote:

> Again, the C++ standard does not say that moving from an object leaves the
> object in an undefined state.
>
> The C++ standard does say:
>
> Objects of types defined in the C++ standard library may be moved from
> (12.8). Move operations may be explicitly specified or implicitly
> generated. Unless otherwise specified, such moved-from objects shall be
> placed in a valid but unspecified state.
>
>
> A few ways this differs from the claim that moving from an object leaves
> the object in an undefined state:
>
> (1) The standard makes no mention of objects in general, only of objects
> in the C++ standard library;
>
> (2) Even for objects in the C++ standard library, the standard makes no
> mention of objects in general, only of objects that are not “otherwise
> specified”;
>

For example, std::unique_ptr does have a fully specified moved-from state
that can be relied on.


   antti


> (3) The standard avoids undefined-ness entirely and instead specifies a
> *valid* but unspecified state.
>
> I think it is accurate to say that the C++ standard specifies that *some* 
> objects
> *in the standard library* have *valid but unspecified* state when moved
> from.
>
> I think it’s also accurate to say that a function that accepts an rvalue
> reference as an argument does not promise to move from that rvalue
> reference.
>
> I think both of these statements are compatible with specifying what
> happens *in WebKit libraries* when we* do move from *an object.
>
> Geoff
>
> On Dec 17, 2018, at 5:04 PM, Alex Christensen 
> wrote:
>
> We can definitely make our own WTF::Optional instead of using
> std::optional and change its move constructor/operator= and I personally
> think that would not be worth it but if I’m in the minority I’ll deal with
> it.  We cannot diverge from the C++ standards that say that moving from an
> object leaves the object in an undefined state, and right now in WebKit we
> are relying quite a lot on that undefined state.  I think that is the
> bigger problem that Chris was trying to solve.
>
> On Dec 17, 2018, at 4:32 PM, Ryosuke Niwa  wrote:
>
> Let me add this.
>
> The situation we have here is analogous to having a member function
> "move()" which leave std::optional in undefined state as in:
>
> std::optional a;
> std::optional b = a.move();
>
> would leave a in some undefined state. I don't care what C++ standards say
> or what STL does. That's insane.
> Every object should remain in a well defined state after performing an
> operation.
>
> - R. Niwa
>
>
> On Mon, Dec 17, 2018 at 4:22 PM Ryosuke Niwa  wrote:
>
>> It's true that WTFMove or std::move doesn't do anything if the moved
>> variable is not used because WTFMove / std::move is just a type cast.
>>
>> However, that behavior is orthogonal from the issue that calling WTFMove
>> / std::move on std::optional, and the returned value is assigned to another
>> std::optional, the original std::optional will be left a bad state.
>>
>> I completely disagree with your assessment that this calls for the use of
>> std::exchange.
>>
>>
>> On Mon, Dec 17, 2018 at 3:55 PM Alex Christensen 
>> wrote:
>>
>>> Let me give a concrete example on why, even with our nice-to-use WTF
>>> types, the state of a C++ object is undefined after being moved from:
>>>
>>> #include 
>>> #include 
>>> #include 
>>>
>>> class Test : public RefCounted { };
>>>
>>> void useParameter(RefPtr&& param)
>>> {
>>>   RefPtr usedParam = WTFMove(param);
>>> }
>>>
>>> void dontUseParameter(RefPtr&&) { }
>>>
>>> int main() {
>>>   RefPtr a = adoptRef(new Test);
>>>   RefPtr b = adoptRef(new Test);
>>>   std::cout << "a null? " << !a << std::endl;
>>>   std::cout << "b null? " << !b << std::endl;
>>>   useParameter(WTFMove(a));
>>>   dontUseParameter(WTFMove(b));
>>>   std::cout << "a null? " << !a << std::endl;
>>>   std::cout << "b null? " << !b << std::endl;
>>>   return 0;
>>> }
>>>
>>> // clang++ test.cpp -I Source/WTF -L WebKitBuild/Debug -l WTF -framework
>>> Foundation -L /usr/lib -l icucore --std=c++17 && ./a.out
>>>
>>> // a null? 0
>>>
>>>
>>> // b null? 0
>>>
>>>
>>> // a null? 1
>>>
>>>
>>> // b null? 0
>>>
>>>
>>>
>>>
>>> As you can see, the internals of callee dontUseParameter (which could be
>>> in a different translation unit) affects the state of the local variable b
>>> in this function.  This is one of the reasons why the state of a moved-from
>>> variable is intentionally undefined, and we can’t fix that by using our own
>>> std::optional replacement.  If we care about the state of a moved-from
>>> object, that is what std::exchange is for.  I think we should do something
>>> to track and prevent the use of moved-from values instead of introducing
>>> our own std::optional replacement.
>>>
>>> On Dec 17, 2018, at 2:47 PM, Ryosuke Niwa  wrote:
>>>
>>> Yeah, it seems like making std::optional more in line with our own
>>> convention provides more merits than downsides here. People are using
>>> WTFMove as 

Re: [webkit-dev] Removing support for CSS regions

2017-08-02 Thread Antti Koivisto
This is a blocker for making the render tree more secure and for
architectural progress there in general. The feature is extremely invasive
and has design issues. I think the security benefits of removing it alone
are worth taking a small regression risk.


  antti

On Wed, Aug 2, 2017 at 10:00 AM, Ryosuke Niwa  wrote:

> On Tue, Aug 1, 2017 at 11:40 PM, Andreas Kling  wrote:
> >
> >> On 2 Aug 2017, at 01:03, Ryosuke Niwa  wrote:
> >>
> >> On Mon, Jul 31, 2017 at 1:49 AM, Andreas Kling 
> wrote:
> >>> Some time has passed, and it seems that adoption of CSS regions on the
> web is not gonna happen.
> >>>
> >>> Blink has long since removed their support.
> >>> Firefox never supported it AFAIK.
> >>> (The new) IE has some amount of support behind a prefix, but no plans
> to unprefix AFAIK.
> >>>
> >>> I think it’s time we remove the code from WebKit, and relieve
> ourselves of the maintenance burden.
> >>> This should also open up numerous opportunities for clean-up and
> optimization.
> >>>
> >>> If you know of any reason to keep the feature, such as a major website
> or WebKit client depending on it, do speak up now!
> >>
> >> Since we've been shipping CSS regions for a while, I think the first
> >> step we should take would be disabling the feature on trunk, and put
> >> that into STP and other ports' releases so that we can easily revert
> >> the change if we find out any Web content to be broken when the
> >> feature is disabled.
> >
> >> Unless there is evidence of at least one major site or client depending
> on CSS regions, I don’t agree that such a slow removal process is necessary.
> >
> > IMO doing that would only further increase the maintenance burden
> incurred by the feature, since we’d have to add tons of runtime checks
> throughout the codebase.
>
> Given my concern is the compatibility, not the maintenance cost, what
> is the evidence that nobody is relying on this feature?
>
> It seems a little crazy to remove a feature that has been available
> (prefixed) since Safari 6.2 without any prior warning or gathering any
> usage metrics at all.
>
> Also see: https://trac.webkit.org/wiki/DeprecatingFeatures
>
> > I would feel differently if we were pioneering this removal, but since
> we’ve already seen it succeed in Blink I’m far less concerned.
>
> I'm more concerned about iOS and macOS apps that use WebKit than the
> Web content in the wild.
>
> - 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] !!Tests for equality comparison

2017-04-28 Thread Antti Koivisto
This is a good change as long as we are just relaxing the rule rather than
mandating ==. I think ! makes a lot of sense when testing for emptiness:

if (!container.size())
   ...

if (!count)
   ...

but == should be used for testing things where 0 is just another number,
like indexes:

if (index == 0)
   ...


   antti


On Fri, Apr 28, 2017 at 11:00 AM, Keith Miller 
wrote:

> Is there any opposition to relaxing this rule? Speak now or forever hold
> your piece! (not really but I would be curious to hear opposition).
>
> Cheers,
> Keith
>
> On Apr 27, 2017, at 10:32 PM, Carlos Garcia Campos 
> wrote:
>
> El jue, 27-04-2017 a las 16:06 -0700, JF Bastien escribió:
>
> Hello C++ fans!
>
> The C++ style check currently say:
> Tests for true/false, null/non-null, and zero/non-zero should all be
> done without equality comparisons
>
> I totally agree for booleans and pointers… but not for integers. I
> know it’s pretty much the same thing, but I it takes me slightly
> longer to process code like this:
>
> int numTestsForEqualityComparison = 0:
> // Count ‘em!
> // …
> if (!numTestsForEqualityComparison)
>   printf(“Good job!”);
>
> I read it as “if not number of tests for equality comparison”. That's
> weird. It takes me every slightly longer to think about, and I’ve
> gotten it wrong a bunch of times already. I’m not trying to check for
> “notness", I’m trying to say “if there were zero tests for equality
> comparison”, a.k.a.:
>
> if (numTestsForEqualityComparison == 0)
>   printf(“Good job!”);
>
> So how about the C++ style let me just say that? I’m not suggesting
> we advise using that style for integers everywhere, I’m just saying
> it should be acceptable to check zero/non-zero using equality
> comparison.
>
>
> I agree, it's also quite confusing when using strcmp, because !strcmp
> means the strings are equal. It's not only more difficult to read, I've
> seen patches with wrong strcmp checks because of that.
>
>
> I also think this could be solved by #defining a success a C call positive
> result that is 0 (e.g. CCallSuccess), regardless of the choice we make here.
>
>
>
> !!Thanks (i.e. many thanks),
>
> JF
>
> p.s.: With you I am, fans of Yoda comparison, but for another day
> this will be.
> ___
> 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] SVN trouble

2017-02-24 Thread Antti Koivisto
Hi,

Looks like https://bugs.webkit.org/show_bug.cgi?id=168774 caused some sort
of SVN problem. Please hold commits until this is resolved.


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


Re: [webkit-dev] WebCore/platform standalone library

2017-01-11 Thread Antti Koivisto
Why is the PAL namespace inside the WebCore namespace? Couldn't it just be
a top-level namespace (even if it currently happens to live in the WebCore
project)?

#include  would be more consistent with existing headers than
.


  antti

On Wed, Jan 11, 2017 at 7:24 AM, Myles C. Maxfield 
wrote:

> After 18 months of no progress, Don Olmstead and I are getting the band
> back together!
>
> We’ve uploaded a patch to https://bugs.webkit.org/show_bug.cgi?id=143358 which
> incorporates feedback from many different stakeholders (and as such, the
> direction is a little different than where I was going with this in the
> beginning).
>
> First of all, this isn’t a new project; instead, it’s a new target inside
> the WebCore project. The target creates a static library which gets linked
> into WebCore, which means that the enforcement mechanism can’t be done by
> the linker. Instead, the layering will be enforced by a Python script,
> triggered as an extra build step, which checks the symbol names inside the
> .a file as well as #include directives in source code.
>
> We opted for WebCore to include files using “#include ” instead
> of just including Foo.h. Similarly, we are putting symbols inside the PAL
> namespace, which is a child of the WebCore namespace. Therefore, inside
> WebCore, you use PAL things by specifying “PAL::Foo”.
>
> The first thing to move into PAL is the “crypto” subfolder, which is a
> good candidate because it’s small, simple, yet also has platform-dependent
> implementations.
>
> We would love your feedback on this approach to help make the dream a
> reality!
>
> Thanks,
> Myles and Don
>
> On Mar 22, 2015, at 4:40 PM, Gavin Barraclough 
> wrote:
>
> On Mar 22, 2015, at 4:35 AM, Maciej Stachowiak  wrote:
>
> Web Abstraction Toolbox (it’s hard to tell the difference between wat and
> WTF sometimes…)
>
>
> +1
>
>
> ___
> 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] Naming preference: SetForScope vs. TemporaryChange

2016-12-26 Thread Antti Koivisto
On Mon, Dec 26, 2016 at 5:29 AM, Maciej Stachowiak  wrote:

>
> ScopedChange(tachyonFlux, 2.0);
>

I wish there was a compiler warning against this. I have caused at least
one bug by doing this exact thing.

(a temporary gets deleted at the end of the expression)


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


Re: [webkit-dev] RFC: stop using std::chrono, go back to using doubles for time

2016-05-23 Thread Antti Koivisto
I think the actual issue here was that std::chrono::duration can't
represent infinite duration and some of our code has been hacking around
that with std::chrono::duration::max(). Clock time + duration is not going
to overflow if the duration is natural (a finite constant or produced by
diffing time_points).

I have found that using Optional in cases where
the duration legitimately can be unspecified/infinite works well. In most
cases it can't be.

If we decide to write our own time type system lets make sure it covers all
the existing use of std::chrono without losing any of the static typing.
But maybe simply disallowing std::chrono::duration::max() in the style
checker would solve the immediate problem here?


   antti


On Mon, May 23, 2016 at 4:41 AM, Filip Pizlo  wrote:

> Hi everyone!
>
> I’d like us to stop using std::chrono and go back to using doubles for
> time.  First I list the things that I think we wanted to get from
> std::chrono - the reasons why we started switching to it in the first
> place.  Then I list some disadvantages of std::chrono that we've seen from
> fixing std::chrono-based code.  Finally I propose some options for how to
> use doubles for time.
>
> *Why we switched to std::chrono*
>
> A year ago we started using std::chrono for measuring time.  std::chrono
> has a rich typesystem for expressing many different kinds of time.  For
> example, you can distinguish between an absolute point in time and a
> relative time.  And you can distinguish between different units, like
> nanoseconds, milliseconds, etc.
>
> Before this, we used doubles for time.  std::chrono’s advantages over
> doubles are:
>
> *Easy to remember what unit is used:* We sometimes used doubles for
> milliseconds and sometimes for seconds.  std::chrono prevents you from
> getting the two confused.
>
> *Easy to remember what kind of clock is used:* We sometimes use the
> monotonic clock and sometimes the wall clock (aka the real time clock).
> Bad things would happen if we passed a time measured using the monotonic
> clock to functions that expected time measured using the wall clock, and
> vice-versa.  I know that I’ve made this mistake in the past, and it can be
> painful to debug.
>
> In short, std::chrono uses compile-time type checking to catch some bugs.
>
> *Disadvantages of using std::chrono*
>
> We’ve seen some problems with std::chrono, and I think that the problems
> outweigh the advantages.  std::chrono suffers from a heavily templatized
> API that results in template creep in our own internal APIs.  std::chrono’s
> default of integers without overflow protection means that math involving
> std::chrono is inherently more dangerous than math involving double.  This
> is particularly bad when we use time to speak about timeouts.
>
> *Too many templates:* std::chrono uses templates heavily.  It’s overkill
> for measuring time.  This leads to verbosity and template creep throughout
> common algorithms that take time as an argument.  For example if we use
> doubles, a method for sleeping for a second might look like
> sleepForSeconds(double).  This works even if someone wants to sleep for a
> nanoseconds, since 0.01 is easy to represent using a double.  Also,
> multiplying or dividing a double by a small constant factor (1,000,000,000
> is small by double standards) is virtually guaranteed to avoid any loss of
> precision.  But as soon as such a utility gets std::chronified, it becomes
> a template.  This is because you cannot have
> sleepFor(std::chrono::seconds), since that wouldn’t allow you to represent
> fractions of seconds.  This brings me to my next point.
>
> *Overflow danger:* std::chrono is based on integers and its math methods
> do not support overflow protection.  This has led to serious bugs like
> https://bugs.webkit.org/show_bug.cgi?id=157924.  This cancels out the
> “remember what unit is used” benefit cited above.  It’s true that I know
> what type of time I have, but as soon as I duration_cast it to another
> unit, I may overflow.  The type system does not help!  This is insane:
> std::chrono requires you to do more work when writing multi-unit code, so
> that you satisfy the type checker, but you still have to be just as
> paranoid around multi-unit scenarios.  Forgetting that you have
> milliseconds and using it as seconds is trivially fixable.  But if
> std::chrono flags such an error and you fix it with a duration_cast (as any
> std::chrono tutorial will tell you to do), you’ve just introduced an
> unchecked overflow and such unchecked overflows are known to cause bugs
> that manifest as pages not working correctly.
>
> I think that doubles are better than std::chrono in multi-unit scenarios.
> It may be possible to have std::chrono work with doubles, but this probably
> implies us writing our own clocks.  std::chrono’s default clocks use
> integers, not doubles.  It also may be possible to teach std::chrono to do
> overflow protection, but that would make me 

Re: [webkit-dev] Windows Build Now on VS2015

2015-08-14 Thread Antti Koivisto
Does this mean we can start using (most) C++14 features?


  antti


On Wed, Aug 12, 2015 at 6:13 AM, Brent Fulgham bfulg...@apple.com wrote:

 Hi Floks,

 We’ve finished updating the various Windows builds to VS2015. Full
 regression tests are now completing on these new builds, and seem to be
 comparable in terms of stability and correctness.

 Please let me know if you encounter any new issues on Windows. I know that
 EA encountered some JavaScript issues in a prior revision, but I haven’t
 been able to replicate this problem (at least yet).

 VS2015 is churning out a number of new build warnings, which I hope to
 address over the coming weeks. It will also likely have some useful static
 analysis results in the Windows-specific portions of the code that we
 should evaluate.

 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


Re: [webkit-dev] Filling the features.json files

2015-04-08 Thread Antti Koivisto
On Wed, Apr 8, 2015 at 7:09 AM, Benjamin Poulain benja...@webkit.org
wrote:

 Hi WebKit,

 The two features.json files are in the tree (WebCore/features.json and
 JavaScriptCore/features.json). The style checker now checks the basic
 structure of the files.


Could you also explain what these files are used for?


  antti



 I have added some basic things in there to show the basic structure. I
 would like help setting up those lists.

 If you have worked on a feature in the last 12 months, and you want to
 give it the visibility it deserves, please add it to features.json.

 An other type of feature that would be useful for those files are features
 that are not as popular as they should (image-set, srcset,
 sticky-positioning, etc).

 The only mandatory fields are name and status.

 Committers can edit those files without reviews. Other contributors can
 make patches and ping me for review.

 Any question? Email me or poke me on IRC.

 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] WebCore/platform standalone library

2015-03-20 Thread Antti Koivisto
Reusable Os Fitting Layer


   antti

On Fri, Mar 20, 2015 at 11:26 AM, Simon Fraser simon.fra...@apple.com
wrote:


 On Mar 20, 2015, at 11:03 AM, Edward O'Connor eocon...@apple.com wrote:

  This almost makes me want to suggest a jokey name for Platform. I can’t
 off the top of my head think of a good expansion of OMG, though. Or BBQ.
 
  I am not a pro at this, but here are a few tries: Lower-level Object
 Library. Algorithm Reuse Framework. New Framework for WebCore, New System
 Framework for WebCore.

 Platform Obfuscation Source.


 Platform Interface and Testing Abstraction.


 General Independent Framework (pronounced jiff, of course).

 Low-Level Abstract Platform would also be a logical choice.


 Low-level Object Library.

 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] WebKit Meeting: Network Cache session

2015-03-17 Thread Antti Koivisto
I added link to the slide to
https://trac.webkit.org/wiki/March%202015%20Meeting .


   antti

On Tue, Mar 17, 2015 at 8:28 AM, Adam Bergkvist adam.bergkv...@ericsson.com
 wrote:

 Hi

 Perhaps I should have posted this to the webkit-meeting list, but it
 seems pretty dead.

 Anyways.. does anyone have any notes or slides from the Network Cache
 session that they would like to share? I missed that session at the
 meeting.

 Antti or Chris?

 BR
 Adam
 ___
 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] WorkQueue for Windows?

2015-03-07 Thread Antti Koivisto
On Sat, Mar 7, 2015 at 9:02 PM, Brent Fulgham bfulg...@gmail.com wrote:

 Hi Antti,

 Take a look at https://bugs.webkit.org/show_bug.cgi?id=142432. I
 haven't landed this yet, because I was hoping to get some kind of test for
 this. However, it seems to introduce no problems, so we could go ahead and
 land it.


That was fast, thanks! Looks good as far as I can tell. Basic QOS support
(thread priorities) would also be a useful addition at some point.

I don't think there is any specific test coverage (though WorkQueue is used
heavily in WK2). At least a basic unit test under
Tools/TestWebKitAPI/Tests/WTF would be nice.


 Question: why do we have a separate WorkQueue implementation in
 DumpRenderTree *and* WebKitTestRunner? Seems like we could do some
 consolidation...


This might be because the code used to live in WK2 and was only recently
moved to WTF. It would indeed be good to get rid of the duplication.


  antti

-Brent


 On Mar 6, 2015, at 10:55 AM, Anders Carlsson ander...@apple.com wrote:

 I think we used to have a WorkQueue implementation for Windows WK2 -
 anyone should be able to dig it up.

 (Alternatively, we can just make an implementation using std::thread).

 - Anders

 On Mar 6, 2015, at 10:46 AM, Antti Koivisto koivi...@iki.fi wrote:


 Hi,


 I'd like to start using WTF::WorkQueue abstraction in WebCore code.
 However we are currently missing a Windows implementation (WorkQueue used
 to be part of WK2). Anyone interested in making one? I assume it wouldn't
 be a difficult task for someone who knows the platform.



   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


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


[webkit-dev] WorkQueue for Windows?

2015-03-06 Thread Antti Koivisto
Hi,

I'd like to start using WTF::WorkQueue abstraction in WebCore code. However
we are currently missing a Windows implementation (WorkQueue used to be
part of WK2). Anyone interested in making one? I assume it wouldn't be a
difficult task for someone who knows the platform.


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


[webkit-dev] virtual and override

2015-02-20 Thread Antti Koivisto
We have traditionally marked virtual overrides with the 'virtual' keyword
for documentation purposes. Nowadays we also have the compiler checked
'override'. Using both at the same time seems redundant.

Should we change the coding style so that exactly one of the 'virtual' and
'override' must be present for all virtual functions?


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


Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-24 Thread Antti Koivisto
I don't think this is really 32bit vs 64bit platform issue. The vast
majority of 64bit systems our users have (that is iOS devices) can't use
memory buffers sized anywhere near the 32bit limit even in theory. Also
when using Vector auto-grow capabilities (which is really the point of
using a vector instead of just allocating a buffer) you need way more
memory than the actual data size. Growing a Vectoruint8_t beyond 4GB has
peak allocation of 9GB.

Are there any examples of Vectors in the current code base where we would
usefully fix an actual problem even in medium term by switching to 64bit
storage sizes? I don't think they exists. Cases like file uploads should
stream the data or use some mapped-file backed buffer type that is not a
Vector.

With this in mind I think the right direction is to make the Vector API
match the implementation and just start using unsigned indexes everywhere.


   antti

On Fri, Nov 21, 2014 at 2:59 AM, Maciej Stachowiak m...@apple.com wrote:


 On Nov 20, 2014, at 4:51 PM, Maciej Stachowiak m...@apple.com wrote:


 On Nov 20, 2014, at 9:26 AM, Alexey Proskuryakov a...@webkit.org wrote:


 19 нояб. 2014 г., в 14:58, Alexey Proskuryakov a...@webkit.org написал(а):

 These and related uses are all over the place - see also Vectors
 in FormDataBuilder, data returned from
 FrameLoader::loadResourceSynchronously, plug-in code that loads from
 network, SharedBuffer etc.


 Another way to say this is: we need to deal with large data arrays
 throughout loading code. We do not really need full size vectors in most
 other code - it's sort of OK for HTML parser or for image decoder to fail
 catastrophically when there is too much data fed to it.

 This is somewhat questionable design, but if we are going to stick to it,
 then magnitude checks should be centralized, not sprinkled throughout the
 code. We should not make this check explicitly when feeding a network
 resource to the parser, for example.

 A 64-bit API for Vector solves this nearly flawlessly. We do not perform
 the checks manually every time we use a Vector, Vector does them for us.

 Other options are:

 - uint64_t everywhere. This way, we'll solve practical problems with large
 resources once and for all. Also, this may prove to be necessary to solve
 even YouTube/Google Drive uploads, I do not know that yet.

 - size_t everywhere. Same effect on 64-bit platforms, while 32-bit ones
 will still be limited. I like this option, because it won't make us pay the
 memory and performance cost on old crippled 32-bit machines, which are
 unlikely to be used for manipulating huge volumes of data anyway.


 We probably want YouTube upload of large files to work on 32-bit machines.
 Though presumably if you want to upload a file larger than the virtual
 address space, you need to represent it in some way other than a Vector.


 Thinking about this more - I think file sizes should probably be an off_t,
 not a size_t, so on 32-bit platforms some care must still be taken in the
 conversion between file sizes and vector sizes.

 In general, I like the idea of Vector having a size_t API and a choice of
 smaller or larger internal size. We might also want to change other
 containers with size-related interfaces to match.

 Regards,
 Maciej



 ___
 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] size_t vs unsigned in WTF::Vector API ?

2014-11-20 Thread Antti Koivisto
Or Vector and ByteBuffer.

All these cases requiring very large buffers seem to be about untyped data.


   antti

On Thu, Nov 20, 2014 at 9:40 PM, Filip Pizlo fpi...@apple.com wrote:

 That looks like a pretty good performance win.

 I'd advocate for SmallVector and Vector, then.

 -Filip


 On Nov 20, 2014, at 11:38 AM, Chris Dumez cdu...@apple.com wrote:

 The corresponding Blink bug did contain some performance data:
 CrBug#229226 https://code.google.com/p/chromium/issues/detail?id=229226

 Kr,
 --
 Chris Dumez - Apple Inc.
 Cupertino, CA




 On Nov 20, 2014, at 11:32 AM, Geoffrey Garen gga...@apple.com wrote:

 I wonder what the downsides are to this approach.  Footprint of Vector?


 It looks like the original change was motivated by shrinking Vector:

 https://bugs.webkit.org/show_bug.cgi?id=97268

 Sadly, it didn’t include any data on the observed benefit :(.

 Geoff
 ___
 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] size_t vs unsigned in WTF::Vector API ?

2014-11-19 Thread Antti Koivisto
I agree with Phil, we should just use unsigned consistently. For very large
memory allocations Vector is probably not the right type to use.


  antti

On Wed, Nov 19, 2014 at 10:32 PM, Filip Pizlo fpi...@apple.com wrote:

 Whatever we do, the clients of Vector should be consistent about what type
 they use.  I'm actually fine with Vector internally using unsigned even if
 the API uses size_t, but right now we have lots of code that uses a mix of
 size_t and unsigned when indexing into Vectors.  That's confusing.

 If I picked one type to use for all Vector indices, it would be unsigned
 rather than size_t.  Vector being limited to unsigned doesn't imply 4GB
 unless you do Vectorchar.  Usually we have Vectors of pointer-width
 things, which means 32GB on 64-bit systems (UINT_MAX * sizeof(void*)).
 Even in a world where we had more than 32GB of memory to use within a
 single web process, I would hope that we wouldn't use it all on a single
 Vector and that if we did, we would treat that one specially for a bunch of
 other sensible reasons (among them being that allocating a contiguous slab
 of virtual memory of that size is rather taxing).  So, size_t would buy us
 almost nothing since if we had a vector grow large enough to need it, we
 would be having a bad time already.

 I wonder: are there cases that anyone remembers where we have tried to use
 Vector to store more than UINT_MAX things?  Another interesting question
 is: What's the largest number of things that we store into any Vector?  We
 could use such a metric to project how big Vectors might get in the future.

 -Filip


 On Nov 19, 2014, at 12:20 PM, Chris Dumez cdu...@apple.com wrote:

 Hi all,

 I recently started updating the WTF::Vector API to use unsigned types
 instead of size_t [1][2], because:
 - WTF::Vector is already using unsigned type for size/capacity internally
 to save memory on 64-bit, causing a mismatch between the API and the
 internal representation [3]
 - Some reviewers have asked me to use unsigned for loop counters iterating
 over vectors (which looks unsafe as the Vector API, e.g. size(), returns a
 size_t).
 - I heard from Joseph that this type mismatch is forcing us (and other
 projects using WTF) to disable some build time warnings
 - The few people I talked to before making that change said we should do it

 However, Alexey recently raised concerns about this change. it doesn't
 strike him as the right direction. 4Gb is not much, and we should have
 more of WebKit work with the right data types, not less.”.
 I did not initially realize that this change was controversial, but now
 that it seems it is, I thought I would raise the question on webkit-dev to
 see what people think about this.

 Kr,
 --
 Chris Dumez - Apple Inc.
 Cupertino, CA


 [1] http://trac.webkit.org/changeset/176275
 [2] http://trac.webkit.org/changeset/176293
 [3] http://trac.webkit.org/changeset/148891

 ___
 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] Networking process scope

2014-11-10 Thread Antti Koivisto
On Mon, Nov 10, 2014 at 12:28 PM, youenn fablet youe...@gmail.com wrote:

 Hi,

 I looked at the Networking process recently and am wondering what is
 in scope/out of scope of this process.

 Currently, the networking process is used to load resources from:
 - URLs resolved using platform specific libraries (CF, libsoup...),
 including data URLs
 - Blob URLs
 The networking process is not used for (at least):
 - appcache resources URLs
 - archive resources URLs

 Two options come to mind:
 1. Use networking process for all URLs that require actual networking.
 That would mean moving data  blob URL handling within WebProcess,
 probably unifying this loading with appcache/archive resource loading.


A load should go to the network process if it needs to do actual
networking. Web processes should not need that capability at all (there are
a few things that still require this at the moment I think). Another reason
might be a case where multiple web processes need to see the same view of
some non-network resources.

Otherwise it makes sense to handle loads as close to the request source as
possible to minimize overhead and complexity. At least data: could be
handled in the web process just fine. I think blob: too.


   antti


 2. Use networking process for all URLs. That would mean moving
 appcache/archive resource loading into Networking process at some
 point.

 Any thoughts?

 Regards,
Youenn

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


Re: [webkit-dev] Disk cache

2014-11-01 Thread Antti Koivisto
On Sat, Nov 1, 2014 at 7:03 AM, Alex Christensen 
alex.christen...@flexsim.com wrote:

 Would anything special need to be done for Windows?


Is there some Windows port that uses network process? As I mentioned
enabling the feature would require Windows specific backend implementation.
Just compiling it out requires no action.


   antti



 Alex

 On Oct 31, 2014, at 11:02 AM, Antti Koivisto koivi...@iki.fi wrote:

 Hello,

 I'm planning to add an experimental HTTP cache implementation to WebKit (
 https://bugs.webkit.org/show_bug.cgi?id=30322). The main motivations are:

 - Improving performance by bringing the cache closer. For example we can
 serialize WebKit response objects directly instead of converting to/from
 platform types.
 - Making future innovation around caching easier. Closer coordination
 between cache and WebKit opens new optimization possibilities.

 The cache lives in the network process. Most of the code is
 cross-platform. The storage backend uses libdispatch IO though it wouldn't
 be hard to add a generic posix one too.

 The code will be behind NETWORK_CACHE feature flag.


   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] Disk cache

2014-11-01 Thread Antti Koivisto
On Sat, Nov 1, 2014 at 10:13 AM, Carlos Garcia Campos carlo...@webkit.org
wrote:

 El vie, 31-10-2014 a las 19:02 +0200, Antti Koivisto escribió:
  Hello,
 
 
  I'm planning to add an experimental HTTP cache implementation to
  WebKit (https://bugs.webkit.org/show_bug.cgi?id=30322).

 Great news!

   The main motivations are:
 
 
  - Improving performance by bringing the cache closer. For example we
  can serialize WebKit response objects directly instead of converting
  to/from platform types.
  - Making future innovation around caching easier. Closer coordination
  between cache and WebKit opens new optimization possibilities.
 
 
  The cache lives in the network process. Most of the code is
  cross-platform. The storage backend uses libdispatch IO though it
  wouldn't be hard to add a generic posix one too.

 Why is it limited to the network process? wouldn't it be possible to use
 it also in the web process when shared secondary process model is used?


The cache ties to NetworkResourceLoader which lives in the network process.
In principle it would be possible to integrate with the web process side
resource loader too. However I don't want to support multiple
configurations during development.

It would be good if all WK2 ports would eventually switch to using the
network process. The current multitude of configurations makes networking
related code more confusing and less hackable than it needs to be.


   antti



 
  The code will be behind NETWORK_CACHE feature flag.
 
 
 
 
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

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


[webkit-dev] Disk cache

2014-10-31 Thread Antti Koivisto
Hello,

I'm planning to add an experimental HTTP cache implementation to WebKit (
https://bugs.webkit.org/show_bug.cgi?id=30322). The main motivations are:

- Improving performance by bringing the cache closer. For example we can
serialize WebKit response objects directly instead of converting to/from
platform types.
- Making future innovation around caching easier. Closer coordination
between cache and WebKit opens new optimization possibilities.

The cache lives in the network process. Most of the code is cross-platform.
The storage backend uses libdispatch IO though it wouldn't be hard to add a
generic posix one too.

The code will be behind NETWORK_CACHE feature flag.


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


Re: [webkit-dev] Keep ResourceBuffer?

2014-10-23 Thread Antti Koivisto
Note that ResourceBuffer is virtual and wraps a ShareableResource on
WebKit2 side. ShareableResource exists to do memory mapped IPC.

The current factoring is confusing and cleaning this up is definitely a
good idea. However it will require a bit more than just changing
ResourceBuffers
to SharedBuffers.


   antti

On Thu, Oct 23, 2014 at 9:42 AM, Darin Adler da...@apple.com wrote:

 Hi folks.

 I’ve noticed that we have an unused abstraction in the loader code.
 ResourceBuffer is a cover for a SharedBuffer, with comments and remnants of
 an older effort to make it some kind of abstraction. But at the moment it’s
 just overhead and no abstraction.

 Does anyone object to my eliminating ResourceBuffer for now and changing
 code to use SharedBuffer directly instead?

 — 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] Keep ResourceBuffer?

2014-10-23 Thread Antti Koivisto
In that case remove away. Did I mention this code is confusing?


   antti

On Thu, Oct 23, 2014 at 8:11 PM, Darin Adler da...@apple.com wrote:


  On Oct 23, 2014, at 10:13 AM, Antti Koivisto koivi...@iki.fi wrote:
 
  Note that ResourceBuffer is virtual and wraps a ShareableResource on
 WebKit2 side. ShareableResource exists to do memory mapped IPC.

 Really? I don’t see any code actually creating the class in WebKit2. There
 is a class named WebResourceBuffer, but no instance of it is ever created.
 Also, not enough of the functions in ResourceBuffer are virtual to make
 this actually work.

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


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

2014-08-22 Thread Antti Koivisto
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.


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


Re: [webkit-dev] Proposal: Remove ENABLE(SVG)

2014-01-29 Thread Antti Koivisto
Yes, let's remove the flag. It is too easy to break the no-SVG build and
the guards uglify the code.


   antti


On Wed, Jan 29, 2014 at 2:13 AM, Sam Weinig wei...@apple.com wrote:

 Hi Everyone,

 While we are discussing removing #ifdefs that everyone has enabled, I'd
 like to propose removing ENABLE(SVG), as every port has SVG enabled.  The
 only argument I have heard for keeping it around is to keep a minimal
 build working, but I don't think the clutter of the #ifdefs is worth that.

 -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] When to use auto? (I usually consider it harmful)

2014-01-07 Thread Antti Koivisto
On Mon, Jan 6, 2014 at 11:49 PM, Geoffrey Garen gga...@apple.com wrote:


 (2) ApplyStyleCommand.cpp:

 auto children = elementChildren(*dummySpanAncestor);
 for (auto child = children.begin(), end = children.end(); child !=
 end; ++child) {
 *if (isSpanWithoutAttributesOrUnstyledStyleSpan(*child))*
 *toRemove.append(*child);*
 }


You are looking at a pretty old revision. In ToT this looks like

for (auto child : childrenOfTypeElement(*dummySpanAncestor)) {
if (isSpanWithoutAttributesOrUnstyledStyleSpan(child))
toRemove.append(child);
}

  antti



 Thanks,
 Geoff

 ___
 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] When to use auto? (I usually consider it harmful)

2014-01-05 Thread Antti Koivisto
On Fri, Jan 3, 2014 at 9:28 PM, Geoffrey Garen gga...@apple.com wrote:

 I strongly object to varying coding style arbitrarily based on author or
 project file. That's a recipe for an unreadable polyglot codebase.


I wasn't advocating a general coding style free-for-all. I imagine there
could be valid subsystem specific reasons for not deploying 'auto' widely.

 auto cell = toRenderTableCell(*renderer); // right
  RenderTableCell cell = toRenderTableCell(*renderer);  // wrong

 Not sure.

 These lines of code do not include a verbatim type name, and so they are
 not friendly to cmd-click/select-and-search. Changing the function
 signature to “toRenderTableCell”, or something like that, might help.


Cmd-click on toRenderTableCell takes you to RenderTableCell.h. I'm
unconvinced this is a practical problem.


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


Re: [webkit-dev] When to use auto? (I usually consider it harmful)

2014-01-03 Thread Antti Koivisto
On Thu, Jan 2, 2014 at 11:12 PM, Geoffrey Garen gga...@apple.com wrote:

 Hi folks.

 Recently, I’ve seen patches go by using the C++11 “auto” keyword. For
 example, let me pick on Andreas:

  http://trac.webkit.org/changeset/161143
 
  +auto newRenderer = textNode.createTextRenderer(style);
  +ASSERT(newRenderer);
 
  ….
 
  +parentRenderer-addChild(newRenderer.leakPtr(), nextRenderer);

 I think this use of “auto” is net harmful.


I agree that in a case where a smart pointer of unclear type (when the new
renderer smart pointers are deployed everywhere the situation might be
different) is returned it might be better not to use auto.

However I also feel the harm here is debatable enough that people working
on/reviewing the code should make decisions instead of there being a
project level dictate. It is a new thing, the appropriate usage hasn't yet
settled and people should be allowed to experiment.

I think an appropriate style guideline for “auto” would say something like:

 - Use “auto to declare a disgusting templated iterator type in a loop
 - Use “auto… - to define a template-dependent return type in a class
 template
 - In all other cases, use an explicit type declaration


I think this is a too limiting set of rules. I have found that in many
cases auto improves code readability by having less gunk obscuring the
logic. It has also made refactoring easier.

I'm not sure how much rules we really need beyond use 'auto' when it
improves code. I gather from this thread that especially people working on
JSC are not comfortable using it so they shouldn't.

If we start making rules I'd add the following:

- Use auto when the type name is already mentioned on a line:

auto cell = toRenderTableCell(*renderer); // right
RenderTableCell cell = toRenderTableCell(*renderer);  // wrong

for (auto source : descendantsOfTypeHTMLSourceElement(*this)) // right
for (const HTMLSourceElement source :
descendantsOfTypeHTMLSourceElement(*this)) // wrong

auto properties = std::make_uniquePropertiesVector();  //right
std::unique_ptrPropertiesVector properties =
std::make_uniquePropertiesVector(); //wrong

This rule is already widely deployed and I think the code readability has
improved.

- Use auto when the type is irrelevant. This covers things like iterators
and adapter classes:

auto sourceDescendants = descendantsOfTypeHTMLSourceElement (*this));

It might also cover smart pointer usage like your example and multiline
expansions of setSize(foo-size()).

- Use auto when type is obvious for people with basic familiarity with a
subsystem:

auto style = renderer.style();


 Thoughts?



   antti



 Thanks,
 Geoff
 ___
 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] The SrcN responsive images proposal

2013-10-23 Thread Antti Koivisto
On Tue, Oct 22, 2013 at 11:50 PM, Tab Atkins Jr. jackalm...@gmail.comwrote:

 The parsing aspect isn't particularly new - parsing data-* attributes
 presents the same problem.  You just need to filter the list of


data-* attributes are not parsed.


 attributes on the element to look for things with a src- prefix.  I've
 heard direct feedback from Yoav, implementing in Blink, that it's not
 a big problem.


I didn't say it is a big problem. I said it is ugly.


  antti


 ~TJ

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


Re: [webkit-dev] The SrcN responsive images proposal

2013-10-20 Thread Antti Koivisto
Ignoring other aspects of this, the idea of making attribute name an
enumeration is somewhat distasteful. It will require ugly special parsing.
The platform has plenty of attribute values that are lists already.


  antti


On Sun, Oct 20, 2013 at 4:00 AM, Maciej Stachowiak m...@apple.com wrote:

 My initial impression is that it seems a bit overengineered.

  - Maciej

 On Oct 19, 2013, at 4:39 PM, Yoav Weiss y...@yoav.ws wrote:

 Hello WebKiteers,

 I'd like to get the project's opinion on a recent responsive image
 proposal by Tab Atkins dubbed srcN:
 http://tabatkins.github.io/specs/respimg/Overview.html

 This proposal covers all the major responsive images use-cases: DPR based
 resource selection, viewport dimensions based resource selection and
 art-direction.
 It does so while avoiding previous implementor concerns regarding the
 picture element and its complexity, and while providing better authoring
 tools for viewport based resource selection than previous proposals.

 The srcN proposal does come to replace the srcset DPR switching syntax
 which recently landed in WebKit (an effort I was a part of). OTOH, srcN's
 x-based URL syntax replicates srcset's DPR syntax, so I believe large parts
 of that code can be reused in srcN's implementation.

 Will the project welcome patches implementing the srcN proposal? (Possibly
 behind a flag)

 Thanks,
 Yoav

 ___
 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] The SrcN responsive images proposal

2013-10-20 Thread Antti Koivisto
On Sun, Oct 20, 2013 at 11:08 PM, Benjamin Poulain benja...@webkit.orgwrote:

 On 10/20/13, 9:07 AM, Antti Koivisto wrote:
  Ignoring other aspects of this, the idea of making attribute name an
  enumeration is somewhat distasteful. It will require ugly special
  parsing. The platform has plenty of attribute values that are lists
 already.

 I was thinking the same thing last night. In addition to weirdness on
 the engine side, it looks like a nightmare for authoring/tooling.


 Is there a precedent for this strange notation?


Also CSS selectors only support matching exact attribute names. There is no
way to write universal elements with some srcN attribute query for
example. This might not be important for practical uses here but it does
demonstrate that the approach is a poor fit to the platform.


  antti


 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] Proposal: remove style scoped

2013-09-26 Thread Antti Koivisto
I think it is a feature that we may want to have sooner or later. We will
probably want to change the implementation quite a bit though and removing
the dead code for now makes sense.


   antti


On Thu, Sep 26, 2013 at 3:42 PM, Benjamin Poulain benja...@webkit.orgwrote:

 Dear WebKit,

 We have an unmaintained implementation of the scoped style feature in
 the tree. (http://www.whatwg.org/specs/**web-apps/current-work/**
 multipage/semantics.html#attr-**style-scopedhttp://www.whatwg.org/specs/web-apps/current-work/multipage/semantics.html#attr-style-scoped
 )
 It is getting in the way of improvements so I would like to remove it for
 now.

 From what I have found online, only Firefox ships the feature and it is
 not popular.

 Does anyone plan to finish the feature and ship it? If not, I will remove
 it soon-ish.
 We can always add the feature later if it becomes useful.

 Cheers,
 Benjamin
 __**_
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 https://lists.webkit.org/**mailman/listinfo/webkit-devhttps://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] Removing DIALOG_ELEMENT for now

2013-08-29 Thread Antti Koivisto
We current have an incomplete, unmaintained implementation of the HTML5
dialog element in our code base. No one is building it as far as I can
see and it has bit-rotted to the point it probably wouldn't work anyway.

I think we should remove the code for now. It should be possible to come up
with a nicer implementation when we decide to bring it back.

https://bugs.webkit.org/show_bug.cgi?id=120467


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


Re: [webkit-dev] Ref counting questions

2013-05-22 Thread Antti Koivisto
TreeShared is essentially an implementation detail of the DOM tree (the
only other client besides Node is SVGElementInstance).  It shouldn't be
used for anything else.

TreeShared keeps Nodes alive when they are part of a tree (have a parent
node) even when they have zero refcount. This way nodes don't need to
explicitly ref and deref their children.


  antti


On Wed, May 22, 2013 at 7:26 PM, Bem Jones-Bey bjone...@adobe.com wrote:

 Hey all,

 I've read the document at http://www.webkit.org/coding/RefPtr.html, but I
 still have some questions about using RefPtrs.

 The first one is listed in the Improving this document section of the
 aforementioned documentation:
 • Perils of programming with TreeShared.
 What does that mean? What gotchas should I be aware of? (Heck, more
 details on any of the things in improving this document would be helpful,
 and I'll do my best to update the document with anything I learn if that's
 desired.)

 Also, I've noticed that there's both TreeShared and RefCounted, and they
 seem fairly similar to my uninitiated eye. What's the difference between
 the two and when should one use one or the other? Also, are there other
 templates/classes that I should be aware of?

 Thanks,
 Bem

 ___
 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] We should delete ENABLE_SHADOW_DOM

2013-05-16 Thread Antti Koivisto
I think it would be better to remove this code in controlled pieces along
with all the underlying implementation (which is mostly not behind the
flag) rather than removing it wholesale. It is good to understand what we
are removing and that gets difficult if the API level (the only thing
consistently behind SHADOW_DOM) is not present. I was also considering
leaving a basic implementation along the lines of
http://lists.w3.org/Archives/Public/public-webapps/2013AprJun/0387.html in
place (mostly to keep details element working).


   antti



On Thu, May 16, 2013 at 8:09 PM, Ryosuke Niwa rn...@webkit.org wrote:

 Hi,

 It appears that not a single port enables SHADOW_DOM build flag at this
 point, and WebKit doesn't even compile with that option enabled. (There are
 a few dozens of build failures).

 Is anyone intending to maintain the feature on trunk?  If not, we should
 simply get rid of code behind this build flag for now; keeping unmaintained
 code that doesn't even compile isn't healthy for the project of our size.

 - 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] What do we do with various Web component features?

2013-04-29 Thread Antti Koivisto
I agree that we should remove most of the Shadow DOM related code (the
portions supporting distribution especially). It is invasive, has
significant architectural issues (including use of a confusing mutating
iterator) and forces generic code many places where it would not be needed
otherwise. It is essentially blocking us from improving DOM data
structures. The code has been developed organically in parallel with the
spec and I feel that a future new implementation could be cleaner.


   antti



On Sat, Apr 27, 2013 at 2:38 PM, Benjamin Poulain benja...@webkit.orgwrote:

 On Fri, Apr 26, 2013 at 9:46 PM, Ryosuke Niwa rn...@webkit.org wrote:

 There appears to be a lot of Web component related features in
 WebKit that used to be maintained by Chromium contributors;
 specifically those related to Shadow DOM and node distributions.

 What do we do with them? The specification is still under the
 construction and our implementation is getting outdated day by day.

 I certainly see the value of the proposed specification, and I would like
 it to be shipped by WebKit browsers. However, having unmaintained code that
 is this invasive in WebCore is very harmful in short term.

 Is anyone stepping up to maintain the code, or should we consider
 removing them for the time being?


 All that unmaintained code has a cost we should not have to pay.

 I am in favor of removing it and bring the code back later when someone
 decide to support that spec.

 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] Parallel JavaScript: Why a separate ParallelArray types

2013-04-13 Thread Antti Koivisto
On Sat, Apr 13, 2013 at 8:26 AM, Filip Pizlo fpi...@apple.com wrote:


 Also, you could do a libdispatch style, where there is no lock per se
 but instead you just have queues that you put tasks on.  I like that, too.
  It's also formally equivalent to both locks and message queues, but much
 more similar to threads and locks in practice.


I always thought libdispatch style concurrency would be a great fit for the
web. It is slightly higher level and probably easier to use than locks and
threads, without being particularly limiting. You also get thread
management and platform event integration (timers etc) in the same package.
The programming style (with blocks C extension) even looks like Javascript
style.


   antti


 -Filip


 Regards,
 Maciej



 ___
 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] StyleBuilder vs StyleResolver

2013-04-12 Thread Antti Koivisto
On Fri, Apr 12, 2013 at 7:36 AM, Dirk Schulze dschu...@adobe.com wrote:

 Hi,

 The style of CSS properties is either set in StyleBuilder/CSSProperty or
 in StyleResolver (alias CSSStyleSelector).

 StyleResolver has a giant switch statement to handle all CSS property
 values and set the style. It is the historical way to build the style.

 StyleBuilder was introduced ~2 years ago. Instead of a giant switch to
 handle all property styles, it has a concept of template to combine CSS
 property handling.

 In these last two years new properties were mainly added to StyleBuilder,
 older properties were left alone in StyleResolver. The concept of
 StyleBuilder was always controversial[1][2]. A lot of people had concerns
 that StyleBuilder is less readable and makes it harder to understand the
 code.

 I personally am more worried that we still have two ways to set the style.
 I think it is bad to keep half of the properties in StyleResolver and the
 other half in StyleBuilder. We may use the general spring cleanup to
 revalidate the concept of StyleBuilder and StyleResolver and decide to use
 the one or the other concept.

 Any thoughts?


Fully agreed. I'm still sad I couldn't stop this refactoring initially. So
much wasted effort.

Having property applying code in a separate class instead of piling it back
to StyleResolver still makes sense though and StyleBuilder is a good name.
Maybe something like this would be a good strategy?

rename StyleBuilder - DeprecatedStyleBuilder
create new StyleBuilder
move the giant switch and the related functions from StyleResolver
to StyleBuilder
move individual properties from DeprecatedStyleBuilder to StyleBuilder
until nothing remains
delete DeprecatedStyleBuilder


   antti


 Greetings,
 Dirk

 [1] https://bugs.webkit.org/show_bug.cgi?id=54707
 [2] https://bugs.webkit.org/show_bug.cgi?id=102844
 ___
 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] StyleBuilder vs StyleResolver

2013-04-12 Thread Antti Koivisto
On Fri, Apr 12, 2013 at 4:50 PM, Dirk Schulze dschu...@adobe.com wrote:


 On Apr 12, 2013, at 1:08 AM, Antti Koivisto koivi...@iki.fi wrote:

  On Fri, Apr 12, 2013 at 7:36 AM, Dirk Schulze dschu...@adobe.com
 wrote:
  Hi,
 
  The style of CSS properties is either set in StyleBuilder/CSSProperty or
 in StyleResolver (alias CSSStyleSelector).
 
  StyleResolver has a giant switch statement to handle all CSS property
 values and set the style. It is the historical way to build the style.
 
  StyleBuilder was introduced ~2 years ago. Instead of a giant switch to
 handle all property styles, it has a concept of template to combine CSS
 property handling.
 
  In these last two years new properties were mainly added to
 StyleBuilder, older properties were left alone in StyleResolver. The
 concept of StyleBuilder was always controversial[1][2]. A lot of people had
 concerns that StyleBuilder is less readable and makes it harder to
 understand the code.
 
  I personally am more worried that we still have two ways to set the
 style. I think it is bad to keep half of the properties in StyleResolver
 and the other half in StyleBuilder. We may use the general spring cleanup
 to revalidate the concept of StyleBuilder and StyleResolver and decide to
 use the one or the other concept.
 
  Any thoughts?
 
  Fully agreed. I'm still sad I couldn't stop this refactoring initially.
 So much wasted effort.
 
  Having property applying code in a separate class instead of piling it
 back to StyleResolver still makes sense though and StyleBuilder is a good
 name. Maybe something like this would be a good strategy?
 
  rename StyleBuilder - DeprecatedStyleBuilder
  create new StyleBuilder
  move the giant switch and the related functions from StyleResolver to
 StyleBuilder
  move individual properties from DeprecatedStyleBuilder to StyleBuilder
 until nothing remains
  delete DeprecatedStyleBuilder

 I agree that StyleBuilder is the better name. Do you really want to have
 two huge renamings? Can't we just move everything over to StyleResolver and
 then rename it to StyleBuilder? After all, we just removed one build system
 yet ;) However, I am fine with both.


StyleBuilder is an implementation detail of StyleResolver and is not
referenced from anywhere else. I don't changing its name qualifies as a
huge renaming.

The transition may take a while. I like strategy like this as it documents
both the current state and the end goal.


   antti


 I would definitely help to make this possible but would appreciate to get
 help from others as well.

 Here is a master bug for this change:
 https://bugs.webkit.org/show_bug.cgi?id=114508

 Greetings,
 Dirk


 
 
 antti
 
 
  Greetings,
  Dirk
 
  [1] https://bugs.webkit.org/show_bug.cgi?id=54707
  [2] https://bugs.webkit.org/show_bug.cgi?id=102844
  ___
  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] AncestorChainWalker substance and style thoughts

2013-02-19 Thread Antti Koivisto
The purpose of the NodeTraversal refactoring was simply to move the
existing traversal functions off the (rather bloated) Node and add Element
specific functions. I think longer term iterators are the way to go as they
allow decoupling our internal document tree data structures from the DOM
API.

The problem with ComposedShadowTreeWalker is not that it is an iterator but
that it is ill-defined and complex. It calls back to non-trivial, mutating
functions on the structure being traversed (ContentDistributor::distribute!).
Simply traversing a tree with it can apparently end up marking the tree for
style recalc.

I don't think adding NodeRenderingTraversal on top of this helped much
though it did clarify the case where ComposedShadowTreeWalker is actually
needed.


  antti

On Tue, Feb 19, 2013 at 4:18 AM, Hajime Morrita morr...@chromium.orgwrote:

 AncestorChainWalker follows the style of ComposedShadowTreeWalker,
 which also breaks conventions like mutations-should-be-verbs. We also fix
 its style for integrity.

 Non-trivial tree traversal API in general should, IMO, be modeled after
 recently introduced NodeTraversal.
 Having separate namespaces allows us to keep Node API minimal. Also it is
 stateless and simple.

 I made NodeRenderingTraversal in that way.
 AncestorChainWalker could be just merged to NodeRenderingTraversal.

 Speaking of ComposedShadowTreeWalker, I think it is worth having a class
 instead of namespace
 since it holds configuration parameters to decide, for example, whether it
 skips specific types of node.



 On Tue, Feb 19, 2013 at 10:37 AM, Darin Adler da...@apple.com wrote:

 I am trying to learn about code related to shadow DOM that needs to be
 understood by anyone working on an part of the DOM.

 I have some questions and concerns about AncestorChainWalker. If there is
 a document I should be reading that covers these, please point me to it.

 The parentNode and parentElement functions are just plain old functions
 that are used to walk the ancestor chain of the normal DOM. But the shadow
 DOM AncestorChainWalker is a class. Why? It seems that it could just be a
 function with two return values. Is it more efficient to have it be a
 class? Can the operation be correctly done without storing
 m_distributedNode in a data member?

 There is a function in AncestorChainWalker named parent. That name is a
 noun, so the function should be a const function that returns a value.
 Since it’s not, the function name should be a verb phrase, such as
 advanceToParent, or event just “advance” since it’s in the context of an
 ancestor chain walker.

 The function crossingInsertionPoint should be named
 isCrossingInsertionPoint as the data member is. But also, since the walker
 sits still on a single node, I don’t think it makes sense to talk about the
 position as “is crossing”. It should be “just crossed” or something like
 that instead. Unless an insertion point is like a bridge and is not itself
 a “true node”.

 The function that returns the current node in the ancestor chain is named
 “get”. That’s not a good name, and should be avoided if possible. It could
 be named “node” or “currentNode” instead.

 -- 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] Feature Announcement: Moving HTML Parser off the Main Thread

2013-01-10 Thread Antti Koivisto
When loading web pages we are very frequently in a situation where we
already have the source data (HTML text here but the same applies to
preloaded Javascript, CSS, images, ...) and know we are likely to need it
in soon, but can't actually utilize it for indeterminate time. This happens
because pending external JS resources blocks the main parser (and pending
CSS resources block JS execution) for web compatibility reasons. In this
situation it makes sense to start processing resources we have to forms
that are faster to use when they are eventually actually needed (like token
stream here).

One thing we already do when the main parser gets blocked is preload
scanning. We look through the unparsed HTML source we have and trigger
loads for any resources found. It would be beneficial if this happened off
the main thread. We could do it when new data arrives in parallel with JS
execution and other time consuming engine work, potentially triggering
resource loads earlier.

I think a good first step here would be to share the tokens between the
preload scanner and the main parser and worry about the threading part
afterwards. We often parse the HTML source more or less twice so this is an
unquestionable win.


  antti


On Thu, Jan 10, 2013 at 7:41 AM, Filip Pizlo fpi...@apple.com wrote:

 I think your biggest challenge will be ensuring that the latency of
 shoving things to another core and then shoving them back will be smaller
 than the latency of processing those same things on the main thread.

 For small documents, I expect concurrent tokenization to be a pure
 regression because the latency of waking up another thread to do just a
 small bit of work, plus the added cost of whatever synchronization
 operations will be needed to ensure safety, will involve more total work
 than just tokenizing locally.

 We certainly see this in the JSC parallel GC, and in line with traditional
 parallel GC design, we ensure that parallel threads only kick in when the
 main thread is unable to keep up with the work that it has created for
 itself.

 Do you have a vision for how to implement a similar self-throttling, where
 tokenizing continues on the main thread so long as it is cheap to do so?

 -Filip


 On Jan 9, 2013, at 6:00 PM, Eric Seidel e...@webkit.org wrote:

  We're planning to move parts of the HTML Parser off of the main thread:
  https://bugs.webkit.org/show_bug.cgi?id=106127
 
  This is driven by our testing showing that HTML parsing on mobile is
  be slow, and long (causing user-visible delays averaging 10 frames /
  150ms).
  https://bug-106127-attachments.webkit.org/attachment.cgi?id=182002
  Complete data can be found at [1].
 
  Mozilla moved their parser onto a separate thread during their HTML5
  parser re-write:
 
 https://developer.mozilla.org/en-US/docs/Mozilla/Gecko/HTML_parser_threading
 
  We plan to take a slightly simpler approach, moving only Tokenizing
  off of the main thread:
 
 https://docs.google.com/drawings/d/1hwYyvkT7HFLAtTX_7LQp2lxA6LkaEWkXONmjtGCQjK0/edit
  The left is our current design, the middle is a tokenizer-only design,
  and the right is more like mozilla's threaded-parser design.
 
  Profiling shows Tokenizing accounts for about 10x the number of
  samples as TreeBuilding.  Including Antti's recent testing (.5% vs.
  3%):
  https://bugs.webkit.org/show_bug.cgi?id=106127#c10
  If after we do this we measure and find ourselves still spending a lot
  of main-thread time parsing, we'll move the TreeBuilder too. :)  (This
  work is a nicely separable sub-set of larger work needed to move the
  TreeBuilder.)
 
  We welcome your thoughts and comments.
 
 
  1.
 https://docs.google.com/spreadsheet/ccc?key=0AlC4tS7Ao1fIdGtJTWlSaUItQ1hYaDFDcWkzeVAxOGc#gid=0
  (Epic thanks to Nat Duca for helping us collect that data.)
  ___
  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] CSSGroupingRule and a bit of refactoring?

2012-12-21 Thread Antti Koivisto
On Fri, Dec 21, 2012 at 3:33 AM, Tab Atkins Jr. jackalm...@gmail.comwrote:

 No, it's just a refactoring on the CSS side, so we don't have to
 repeat a bunch of stuff every time we have an at-rule that contains
 other rules.  It just makes the WebIDL easier and less error-prone.


It seems bit strange to add a web-exposed type if it is just a spec writing
helper. Each new type has non-zero cost (code size of generated bindings
etc).

Internally it obviously makes sense to share code, I'm just wondering why
we would want to expose this via API.


   antti



 ~TJ

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


Re: [webkit-dev] CSSGroupingRule and a bit of refactoring?

2012-12-13 Thread Antti Koivisto
We already have internal type corresponding to the new
CSSGroupingRule, StyleRuleBlock. Technically refactoring the CSSOM like
this would be fairly trivial.

It is not clear to me what value this new type adds though. In JS you don't
really care if types have a common base as long as they share similar
looking interface. Are there some concrete use cases?


  antti


On Thu, Dec 13, 2012 at 3:41 AM, Pablo Flouret pab...@motorola.com wrote:

 Hello webkit folk,

 I'm working on having @supports behave like a proper CSSSupportsRule [1].

 In the css3-conditional spec, CSSSupportsRule inherits from a
 CSSGroupingRule (CSSMediaRule does as well):
 http://dev.w3.org/csswg/css3-conditional/#the-cssgroupingrule-interface.

 I'm wondering if it's a good idea, as a first step, to do a bit of
 refactoring and rip the stuff that's now in CSSGroupingRule from
 CSSMediaRule (cssRules/insertRule/deleteRule), and have the supports
 and media rules inherit from it.

 Incidentally, i see at least WebKitCSSKeyframesRule and
 WebKitCSSRegionRule also having similar but not quite identical
 re-implementations of these functions, but i guess whether these ones
 should also inherit from CSSGroupingRule might be a question for the
 standards mailing lists first (in any case, they would potentially
 benefit from such a refactoring).

 Opinions / advice / potential problems?

 Thanks!

 [1] http://wkbug.com/104822

 --
 pablo flouret
 motorola | webkit / browser team
 ___
 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] Prefix naming scheme for DOM-exposed functions

2012-12-10 Thread Antti Koivisto
On Fri, Dec 7, 2012 at 1:36 PM, Maciej Stachowiak m...@apple.com wrote:


 Would this involve creating a bindingsFoo() for every method foo() that is
 exposed to bindings? For example, would we have to add
 XMLHttpRequest::bindingsSend(), even though there's no real need for a
 special internal XMLHttpRequest::send()? Would getters and setters that map
 to JavaScript properties (but which do not reflect markup attributes) also
 need a bindings... version? For example, would we need
 HTMLMediaElement::bindingsMuted() and HTMLMediaElement::bindingsSetMuted()
 to wrap the regular muted() and setMuted()?


I see no internal clients for HTMLMediaElement::muted()/setMuted() so there
would be no need for internal versions for it. I suspect that in many cases
we would at most need an internal accessor (which could then properly
follow our naming conventions HTMLMediaElement::isMuted()).


 If the answer to these questions is yes, then I think this is too much
 complexity tax on all exposed methods and properties to make up for the
 benefit. It's likely only a minority of methods where it's highly desirable
 to have a specialized version for internal use.


Probably the only way to find out is try doing it and seeing how the patch
ends up looking like.


 As a side note, I don't see how this would address the concrete example
 given, that of firstElementChild likely becoming more efficient than
 firstChild. If we add bindingsFirstChild() and bindingsFirstElementChild(),
 how does this help WebCore developers know that they should use the
 internal firstElementChild() instead of the internal firstChild()? I expect
 both have to exist, because there really are cases where you need to
 traverse the whole DOM, not just elements, and even if we were to convert,
 firstElementChild() is not a drop-in replacement.


This would create more freedom for our internal document tree to diverge
from how it looks like through the DOM api. For example internal
firstChild() that returns Node* may no longer make sense at all if we
eliminate Text nodes.


  antti


 It also seems to me that internal methods that do the exact same thing as
 a bindings...() version but lack an ExceptionCode parameter, we'll still
 want to avoid excess code duplication, in some cases of tricky algorithms.
 I would not want a second copy of ContainerNode::insertBefore() and its
 helper methods that replaces exception checking with preflight checks that
 return false without setting an exeption code (I don't think you can just
 skip the checks entirely or you'd make a method that is extremely dangerous
 to call if you have not met very complex preconditions).

 I do agree with the goal of having efficient internal interfaces that are
 not constrained by what is exposed to the Web, but a blanket introduction
 of methods just for bindings does not seem like a good way to get there.

 Possible alternatives:
 - Use something in the IDL to call a bindings... variant only in cases
 where we know there is a materially better internal method.
 - Use the style checker to ban calling select exposed methods from
 hand-written WebCore code, and give the corresponding internal methods
 different names.

 These approaches could achieve the goals described for critical DOM
 methods without having to infect things like XHR::send() or
 HTMLMediaElement::setMuted().

 Regards,
 Maciej

 On Dec 7, 2012, at 9:27 AM, Darin Adler da...@apple.com wrote:

  Hi folks.
 
  Many of the APIs designed for use in the DOM are not efficient for use
 inside WebKit, or have designs that are better for JavaScript than for C++.
 Antti Koivisto and I have been discussing how to best communicate this to
 WebKit contributors so they don’t end up using inefficient idioms just
 because they are familiar with them from use in JavaScript code on
 websites. So far, our best idea for this is to add a prefix to function
 names that indicate they are functions for use by the bindings machinery.
 Thus, a function like appendChild would get a new name:
 
 void bindingsAppendChild(Node*, ExceptionCode);
 
  The internal function that’s used to add a child node would be designed
 for the best clarity, ease of use, and efficiency within WebKit
 implementation code, even when that does not match up with the DOM
 standard. And could be refactored over time as WebKit design changes
 without affecting the bindings.
 
  - It’s not clear what the best prefix is. I don’t like the prefix “dom”,
 since it’s a lowercased acronym and an overloaded not all that precise
 term. The prefix “bindings” is sort of silly because these functions are
 not themselves “bindings”, rather they are the non-language-specific
 functions designed to be bound to each language. However, I do like the
 idea of a brief non-acronym word. So, still looking for a great prefix.
 
  - When appropriate, these exposed functions can be short inline
 functions that turn around and call internal functions.
 
  - These functions aren’t needed

Re: [webkit-dev] DRT/WTR should clear the cache at the beginning of each test?

2012-11-02 Thread Antti Koivisto
On Wed, Oct 31, 2012 at 12:05 AM, Alexey Proskuryakov a...@apple.com wrote:

 This will mean that cache is always almost empty, and all resources in it
 are extremely fresh. I don't know if this would provide substantial
 additional test coverage over cleaning the cache all the time, or just
 completely disabling it in WebKitTestRunner.


Certain areas of coverage would improve. The code paths taken when a
resource is restored from the memory cache can be quite different from the
usual loading. Many operations (like script execution) happen synchronously
if the resource is found from the cache. We reuse various decoded forms
(bitmaps, stylesheets, jsc parse structures, likely more in the future).
All data is available in single chunk. It is possible to write tests that
detect these differences (and it is possible that some tests hit them
accidentally).

We would still lose coverage for things that depend on having lots of
resources around like cache pruning.


   antti


 - WBR, Alexey Proskuryakov


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


Re: [webkit-dev] On returning mutable pointers from const methods

2012-10-29 Thread Antti Koivisto
I don't think the original proposal was meant to apply to the basic
container types. Would this be a sensible rule to adopt for WebCore only
for example?

Like all our blanket rules, this one should be ignored when it doesn't
make sense. If that kind of cases are expected to be very rare then their
existence shouldn't be a show stopper for adopting the rule.


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


Re: [webkit-dev] DRT/WTR should clear the cache at the beginning of each test?

2012-10-28 Thread Antti Koivisto
We could clear the cache between tests but run each test twice in a row.
Second run will then happen with deterministically pre-populated cache.
That would both make things more predictable and improve our test coverage
for cached cases. Unfortunately it would also slow down testing
significantly, though less than 2x.


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


Re: [webkit-dev] DRT/WTR should clear the cache at the beginning of each test?

2012-10-26 Thread Antti Koivisto
On Wed, Aug 8, 2012 at 9:54 PM, Eric U er...@google.com wrote:

 On Wed, Aug 8, 2012 at 11:43 AM, Alexey Proskuryakov a...@webkit.org
 wrote:
  I can see some downsides to emptying the cache before each test:
 
  - we won't be getting any test coverage for cache behavior when it hits
  non-trivial size;

 Then let's add a cache test explicitly for this.  Otherwise we just
 have to hope it gets tested accidentally along the way.


Cache has subtle interactions with other things being tested (-flakiness).
More explicit cache tests would be nice but we can't hope the replicate all
the accidental testing we now get. We are going to lose a large chunk of
existing test coverage if we do this.


  antti



  - this may well make tests measurably slower;
 
  - this will be yet another cause of subtle difference between platforms,
 as
  some will undoubtedly have this unimplemented for a long time.

 Both good points, but probably worth it, given the reliability
 improvement in the tests IMO.

 Eric
 ___
 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] DRT/WTR should clear the cache at the beginning of each test?

2012-10-26 Thread Antti Koivisto
On Fri, Oct 26, 2012 at 6:09 PM, Ami Fischman fisch...@chromium.org wrote:

 The reality is that this test coverage today shows up as flakiness and
 so is ignored anyway, meaning we don't actually have useful coverage here.
  Even when flakiness is investigated, the fix is to cache-bust using
 unique URL params, which just means we lose the coverage you describe for
 that test, anyway.


When making cache related changes I have frequently found bugs from my
patches because some seemingly random test started failing and I
investigated. Without the test coverage some of those bugs would probably
now be in the tree.


  antti



 Brian notes in the bug that GTK  wk2 GTK+ are done.
 I believe that just leaves chromium  mac.
 Anyone wanting to step up to do mac, and, I guess, wk2 mac?

 -a

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


Re: [webkit-dev] On returning mutable pointers from const methods

2012-10-25 Thread Antti Koivisto
On Thu, Oct 25, 2012 at 1:48 PM, Andreas Kling kl...@webkit.org wrote:


 So, I propose that we allow only these two signature formats for raw
 pointers:

 - const Foo* foo() const;
 - Foo* foo();


I think this would be an excellent rule to adopt for new code. The
mutable/immutable state pattern is a powerful way to optimize data
structures that are rarely mutated. Correct use of const through the code
makes using it much easier and safer. With adaption of PassRef/OwnPtr I
can't think of any correct uses for Foo* foo() const.


   antti

Moreover, for methods that return references to objects where call sites
 are expected to take a strong reference, we should really be using
 PassRefPtrFoo as the return type.

 Thoughts? Objections? Am I missing something?

 -Kling

 ___
 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] Discussing bug 98539 - Refactor resource loading to allow for out-of-process loading and memory caching

2012-10-09 Thread Antti Koivisto
On Tue, Oct 9, 2012 at 4:21 AM, Maciej Stachowiak m...@apple.com wrote:

 One preliminary finding of ours is that different web pages fairly often
 load identical resource bodies from different URLs. We expect possible
 benefits from sharing the body data of resources in memory even if we
 cannot share the URL or response headers.


I did some preliminary profiling of this earlier. The data is not
necessarily very representative of the web as whole, I simply browsed
through a number of popular sites with an instrumented build (
https://bug-98539-attachments.webkit.org/attachment.cgi?id=167753). Here is
what I had in the WebCore memory cache in the end:

TOTAL duplicates count=443 size=852791 decodedSize=3426638
all cached resources count=2636 size=116463712

17% of cache entries are exact copies of previous entries. They use 4.3MB
out of total 116MB cache size (4%). The average size of duplicate entries
is much smaller than the average entry size (not surprisingly, transparent
1x1 images etc).

Some examples:

HTTP vs. HTTPS:

hash=5d90af size=91556 decodedSize=34834
https://ajax.googleapis.com/ajax/libs/jquery/1.6.2/jquery.min.js
hash=5d90af size=91556 decodedSize=32
http://ajax.googleapis.com/ajax/libs/jquery/1.6.2/jquery.min.js

(decodedSize for js resources is the size of the function offset cache
which can differ depending which functions have been parsed).

Ad scripts with deliberately unique URLs:

hash=6f6494 size=3111 decodedSize=32
http://ad.doubleclick.net/adj/teg.fmsq/j2ek;subs=n;wsub=n;sdn=n;dcopt=ist;pos=ldr_top;sz=728x90,970x90;tile=1;ord=76273718
?
hash=6f6494 size=3111 decodedSize=648
http://ad.doubleclick.net/adj/teg.fmsq/j2ek;subs=n;wsub=n;sdn=n;dcopt=ist;pos=ldr_top;sz=728x90,970x90;tile=1;ord=711567314
?
hash=6f6494 size=3111 decodedSize=32
http://ad.doubleclick.net/adj/teg.fmsq/j2ek;subs=n;wsub=n;sdn=n;dcopt=ist;pos=ldr_top;sz=728x90,970x90;tile=1;ord=908690779
?
hash=6f6494 size=3111 decodedSize=32
http://ad.doubleclick.net/adj/teg.fmsq/j2ek;subs=n;wsub=n;sdn=n;dcopt=ist;pos=ldr_top;sz=728x90,970x90;tile=1;ord=876087584
?

Same framework loaded by multiple unrelated sites:

hash=230c7d size=1958 decodedSize=0
http://static.iltalehti.fi/js/measure/spring.js
hash=230c7d size=1958 decodedSize=1536 http://yle.fi/global/spring/spring.js

(didn't see as many of these as expected though i'm sure there are sites
that run into this)

Copy-paste resources:

hash=8fb3e size=3965 decodedSize=13088
http://store.storeimages.cdn-apple.com/2149/store.apple.com/rs/source/store/base/nav/globalnav/css/bg/globalsearch_spinner.gif
hash=8fb3e size=3965 decodedSize=13088
http://store.storeimages.cdn-apple.com/2150/store.apple.com/rs/source/store/features/search/css/bg/spinner.gif
hash=8fb3e size=3965 decodedSize=13088
http://store.storeimages.cdn-apple.com/2150/store.apple.com/rs/source/store/base/nav/globalnav/css/bg/globalsearch_spinner.gif
hash=8fb3e size=3965 decodedSize=13088
http://images.apple.com/global/nav/images/globalsearch_spinner.gif

Versioning:

hash=cecbad size=184899 decodedSize=0
http://store.storeimages.cdn-apple.com/2150/store.apple.com/rs/applestore-rs-2.css
hash=cecbad size=184899 decodedSize=0
http://store.storeimages.cdn-apple.com/2149/store.apple.com/rs/applestore-rs-2.css

hash=47a8de size=20041 decodedSize=7870
http://js.t.sinajs.cn/open/analytics/js/suda.js?version=b4d67909ad6b5b7d
hash=47a8de size=20041 decodedSize=7870
http://tjs.sjs.sinajs.cn/open/analytics/js/suda.js

Multiple content servers:

hash=8e976b size=808 decodedSize=13088
http://i0.sinaimg.cn/home/deco/2008/0329/sinahome_0803_ws_003_new.gif
hash=8e976b size=808 decodedSize=71700
http://i3.sinaimg.cn/home/deco/2008/0329/sinahome_0803_ws_003_new.gif

Based on this I think this is definitely worth at least looking further.


  antti

You can also expect that we won't push forward blindly on this effort if
 data ultimately shows it to be a bad idea, or in general not worth the
 complexity.

 Regards,
 Maciej

 ___
 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] Discussing bug 98539 - Refactor resource loading to allow for out-of-process loading and memory caching

2012-10-09 Thread Antti Koivisto
On Tue, Oct 9, 2012 at 10:02 PM, Adam Barth aba...@webkit.org wrote:

 This is interesting data, but it seems to be related to whether we
 should make the MemoryCache content addressable rather than whether we
 should use shared memory to back the MemoryCache when there are
 multiple WebProcesses.


It is relevant when considering if and how to share cache data between
processes. It is also interesting in single process case. Brady's
refactoring should be helpful for both scenarios.


  antti


 Adam


  You can also expect that we won't push forward blindly on this effort if
  data ultimately shows it to be a bad idea, or in general not worth the
  complexity.
 
  Regards,
  Maciej
 
  ___
  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] Eliminate potential null pointer dereference?

2012-04-21 Thread Antti Koivisto
Sat, Apr 21, 2012 at 8:13 AM, John Yani van...@gmail.com wrote:

 2316if (selector-relation() != CSSSelector::SubSelector)
 2317break;
 2318selector = selector-tagHistory();
 2319};

 Now selector is null and we are trying to call tagHistory():


This is not possible. If selector-relation() == CSSSelector::SubSelector
then there will always be a subselector in tagHistory.


 2321for (selector = selector-tagHistory(); selector; selector =

 Which will result in segfault.


That would indicate a serious bug in CSS parser. The crash would allow us
to catch and fix the bug. Now the bug is hidden. We have also lost some
documentation (in form of code) on how our data structures look like. The
only sensible change here would have been ASSERT(selector) for
documentation purposes.

There is generally too much pointless drive-by refactoring going on in the
project. I think we should take harder line against these No new test /
code cleanup only type patches to reduce noise level.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] How to use ASSERT_NO_EXCEPTION

2011-12-15 Thread Antti Koivisto
On Thu, Dec 15, 2011 at 8:36 AM, Darin Adler da...@apple.com wrote:

ExceptionCode ec;
appendChild(newChild, ec);
ASSERT(!ec);


Often code like this indicates misuse of DOM API functions for internal
purposes. This is inefficient (due to exception related checking and other
spec mandated behaviors) and architecturally bad (ties our internal data
structures to DOM way of presenting them, see stylesheet related classes).

I'm not sure it is a good idea to make these cases less visible.


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


Re: [webkit-dev] Using C++ constant local variables in WebKit

2011-12-01 Thread Antti Koivisto
On Thu, Dec 1, 2011 at 4:26 AM, Ojan Vafai o...@chromium.org wrote:

 I don't mind using const in some cases, but I share Darin's concern of
 littering the code with consts. I'd prefer that

we come up with a fairly conservative guideline about when to use it. I'm
 not sure what that would look like. We could start with, what's special
 about this case, or string parsing in general that makes using const more
 valuable?


Littering the code with anything is bad. Has misuse of consts been a
significant practical problem? Perhaps we don't need a specific rule here.


   antti




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


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


Re: [webkit-dev] old-run-webkit-tests is officially deprecated.

2011-10-28 Thread Antti Koivisto
On Fri, Oct 28, 2011 at 1:00 AM, Eric Seidel e...@webkit.org wrote:

 If you're still using old-run-webkit-tests for your work, I would
 *love* to know and fix any issues you may have with NRWT.


The lack of locale independence is a showstopper for me,
https://bugs.webkit.org/show_bug.cgi?id=68691.


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


Re: [webkit-dev] Compile-time assertions for object sizes

2011-09-29 Thread Antti Koivisto
On Thu, Sep 29, 2011 at 9:40 PM, Andreas Kling kl...@webkit.org wrote:

 One idea is to add a file that would only be built on (for example) 64-bit
 Mac and then at least that bot would break if an object changes size. That's
 obviously not ideal though.


I like that approach as it allows you to explicitly assert against byte
counts. Other approaches are more abstract and so easier to dismiss when
checking in a bloaty patch. It also documents the byte sizes and makes
progressions explicit too.


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


Re: [webkit-dev] Implementing style scoped

2011-09-09 Thread Antti Koivisto
On Fri, Sep 9, 2011 at 12:28 AM, Roland Steiner
rolandstei...@chromium.orgwrote:


 ad 3.) Implementation of scoped selector matching works as follows:

 .) scoped style sheet rules are contained in a separate map scoping
 element address - RuleSet
 .) add a field to ParentStackFrame that identifies the
 closest ParentStackFrame that refers to a scoping element
 .) during matching only check RuleSets that are thusly identified in
 ParentStackFrame (plus a slow path that climbs the tree in case
 ParentStackFrame isn't applicable), plus global rules
 .) add a 'scope' parameter to the matching functions that identifies the
 scoping element that limits the matching.

 Note that with this implementation rules in scoped RuleSets that are not
 in scope aren't even looked at. Also note that the actual selector data is
 NOT modified for this. I.e., a selector in a scoped style sheet is not
 different from a global selector. This has several benefits:


Sounds good to me. We don't currently rely on ParentStackFrames to get any
semantics right, it is just for performance optimization. You will need to
take care that it is now functional (rebuild as needed) in all cases.

Inroducing root should be ok for fastCheckSelector path too, testing against
root element shouldn't be slower than testing against null.

I was anyway thinking of moving the parent stack to SelectorChecker. With
this it will actually be needed for querySelectorAll.


   antti



 .) no updating necessary if a style element is inserted or removed from
 the document, nor when the 'scoped' attribute is set/unset.
 .) paves the way for a trivial implementation of a similarly scoped
 querySelector[All].
 .) for components: several component instantiations can re-use the same
 template style sheet without the need to clone its rules.


 Please let me know your thoughts,

 - Roland

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


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


Re: [webkit-dev] Preload Scanner in WebKit

2011-02-11 Thread Antti Koivisto
On Fri, Feb 11, 2011 at 9:11 AM, Adam Barth aba...@webkit.org wrote:
 We run the preload scanner when the regular parser is blocked on the
 network.  CSS and other sub-resources don't block the main parser
 whereas scripts do.

If the main parser encounters an script tag (including inline scripts)
it will also block on any external CSS load that was triggered earlier
in the document.


   antti


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

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


Re: [webkit-dev] beforeload link (esp rel prefetch)

2011-01-13 Thread Antti Koivisto
2011/1/13 Darin Fisher da...@chromium.org:
 Supporting the Link header enables web servers to inject link tags without
 modifying the document, which can be useful, especially for intermediaries.

That sounds like a great reason not to support this feature. Why would
we want to make the web more unpredictable and harder to understand?

From prefetching perspective this has little or no value. When we get
the response header we are generally milliseconds away from parsing
the real document text and requesting the linked resources properly.


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


Re: [webkit-dev] HTML5 Web Links (RFC 5988)

2010-11-10 Thread Antti Koivisto
Even for prefetching this seem rather worthless as it won't allow
prefetching start significantly earlier over the resources specified
in the document source.

There is also (based on the experience in similar things) a high
chance that whatever is listed in the Link header won't match what the
document actually uses and so will end up slowing down the page load.


  antti

2010/11/9 Gavin Peters (蓋文彼德斯) gav...@chromium.org:
 Firefox supports it for rel=prefetch, and it was my thought to include that
 support in my patch.  That use is worthwhile: it allows server specification
 of prefetch resources, as opposed to author (as in HTML).l
 - Gavin

 On 9 November 2010 12:20, Maciej Stachowiak m...@apple.com wrote:

 On Nov 9, 2010, at 9:00 AM, Alexey Proskuryakov wrote:

 
  09.11.2010, в 03:51, Alex Milowski написал(а):
 
  Now that RFC 5988 is a proposed standard [1] and HTML5 references the
  Link: header [2]
 
 
  Note the way in which HTML5 references it: Some versions of HTTP
  defined a Link: header. That's about HTTP 1.0 only.
 
  Specifying stylesheets in HTTP headers seems like a most obvious
  misfeature to me, and other potential uses of the Link header field are so
  unimportant that RFC 5988 doesn't even bother to mention them in its
  introduction.

 It might be worth testing which other browsers support it. I know at least
 Firefox supports associating a stylesheet with Link. It does seem like a
 fairly ill-conceived feature, but not so much that it is worth being the
 sole holdout, if other browser engines all do it. And there are some
 plausible use cases - same document served with multiple stylesheets,
 without having to modify the document on the fly, just the headers.

 Regards,
 Maciej

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


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


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


Re: [webkit-dev] Tightening up smart pointer usage rules

2010-06-28 Thread Antti Koivisto
Is the plan also to banish the std::auto_ptr? It seems pointless and
confusing to allow both the OwnPtr and the auto_ptr.


  antti (who wants PwnPtr too)

On Mon, Jun 28, 2010 at 11:39 PM, Darin Adler da...@apple.com wrote:
 Hi folks.

 I’d like to use our smart pointers more consistently to decrease the chances 
 of memory leaks or other similar problems.

 My proposal is that we have a rule that all calls to new are immediately 
 followed by putting the object into a smart pointer. The main types of smart 
 pointer that matter for these purposes are:

    RefPtr
    OwnPtr
    OwnArrayPtr

 Today, we put a new reference counted object into a RefPtr by calling 
 adoptRef. The code looks something like this:

    PassRefPtrSharedObject createSharedObject()
    {
        return adoptRef(new SharedObject);
    }

 I propose we do the same thing with single ownership objects. It would look 
 like this:

    PassOwnPtrSingleOwnerObject createSingleOwnerObject()
    {
        return adoptPtr(new SingleOwnerObject);
    }

 Then it would be a programming mistake to call new without immediately 
 calling adoptPtr or adoptRef. As part of this effort I plan to do the 
 following:

    1) Add a debugging mode where we assert at runtime if we ref, deref, or 
 destroy a RefCounted object before doing adoptRef. Tracked in 
 https://bugs.webkit.org/show_bug.cgi?id=27672.

    2) Add an adoptPtr function that returns a PassOwnPtr, and a releasePtr 
 function that returns a raw pointer to the PassOwnPtr class.

    3) Add a PassOwnArrayPtr with similar adoptArrayPtr and releaseArrayPtr 
 functions.

    4) Add a strict mode to PassOwnPtr and OwnPtr which removes OwnPtr::set 
 entirely and removes the ability to construct a PassOwnPtr or OwnPtr from a 
 raw pointer directly, making it a compile time error to forget to use 
 adoptPtr.

    5) Once everything compiles with the strict mode, make the strict mode the 
 only mode.

    6) Add validator rules that make invocation of the new operator legal 
 only inside adoptRef and adoptPtr function calls.

 Code that used to say this:

    OwnPtrOtherObject m_otherObject;
    ...
    m_otherObject.set(new OtherObject);

 Will now say this:

    OwnPtrOtherObject m_otherObject;
    ...
    m_otherObject = adoptPtr(new OtherObject);

 And one thing that’s cool about that is it is quite natural for this to 
 become:

    OwnPtrOtherObject m_otherObject;
    ...
    m_otherObject = OtherObject::create();

 I thought I’d mention this to everyone on the list before getting doing 
 significantly more work on this. I think it’s going to work well. Any 
 questions or comments?

    -- Darin

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

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


Re: [webkit-dev] Proposal: Rect based HitTest for a better touch experience

2010-06-06 Thread Antti Koivisto
On Wed, Jun 2, 2010 at 11:22 PM, David Hyatt hy...@apple.com wrote:

 I really don't think hit testing needs to be modified to get what you want.  
 You can do a scattershot sampling using multiple candidate points within the 
 rect and apply whatever heuristics you want to choose a node.  I'm also not 
 convinced that we should be complicating core hit testing code in this way, 
 since I suspect this is one of those areas where mobile platforms will each 
 want to do their own slightly different thing anyway.

The problem with hit testing by multisampling is that it is
ridiculously slow. You need to take at least a dozen samples for
decent results. With a complex document on a mobile platform doing
those can take several hundreds of milliseconds. That is pretty far
from what is needed for good responsiveness. I think area hit testing
would make a lot of sense.

With WebKit2 it might actually be a good idea to track all event
sensitive areas in the document. This way at least partial (no
hit/maybe hit) testing could be done entirely in the UI process side.


  antti

 dave
 (hy...@apple.com)

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

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


Re: [webkit-dev] PreloadScanner aggressiveness

2010-01-11 Thread Antti Koivisto
On Fri, Jan 8, 2010 at 3:00 AM, Mike Belshe m...@belshe.com wrote:

Nice testing!

But for HTTP; the key seems to the pre-rendering-ready escape hatch in
 DocLoader::preload.  Removing this gives me most all of the benefit.  The
 comment says it pretty clearly:  Don't preload images or body resources
 before we have something to draw. This prevents preloads from body delaying
 first display when bandwidth is limited.  For SPDY, there is more benefit
 by continuing to preparse aggressively - I suspect this is due to the finer
 grained prioritization where it can continue to send requests up without
 impacting the clogged downlink channel.


Yeah, this is currently really optimized for best first display, not for
total load time.

In my testing that escape hatch was pretty important for first display if
the case was bandwidth limited (several seconds on some major sites on 3G).
It is not surprising that it may somewhat hurt total load time in high
bandwidth/high latency case.

Ideally we would have per-site estimate of the current bandwidth and latency
values available so we could tune things like this dynamically.

Any testing of changes here should consider first display times too, not
just total load time.


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


Re: [webkit-dev] http cache support - rfc2616

2008-11-17 Thread Antti Koivisto
WebKit implements significant parts of RFC 2616 caching logic for it's
internal memory cache and will implement more. See
https://bugs.webkit.org/show_bug.cgi?id=17998 for details.


   antti

2008/11/15 Darin Fisher [EMAIL PROTECTED]:
 The http caching logic for chromium lives here:
 http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_cache.cc?view=markup
 WebKit does not have code for a network stack.  Instead, each port provides
 a network stack, typically by using a system library.
 -Darin

 On Sat, Nov 15, 2008 at 9:01 AM, zaheer ahmad [EMAIL PROTECTED] wrote:

 hi,
 Does webkit or any component built with (e.g. chromium) has a full
 implementation of http caching- rfc2616. a quick search in the code base or
 the bug list does not suggest one. Looks like some of the pieces from html5
 application cache can be reused for such an implementation. kindly suggest.
 thanks,
 Zaheer
 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev



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


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


[webkit-dev] Windows build now requires QuickTime SDK

2007-12-21 Thread Antti Koivisto
Hi,

I just checked in the timed media support (video and audio) for
Windows. This means that Windows build now depends on the QuickTime
SDK which can be downloaded from here:

http://developer.apple.com/quicktime/download/

Note that QuickTime itself is not required to build, just the SDK.


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