Re: [webkit-dev] Enabling visual viewports by default

[David Bokan]

Hi Simon,

Blink had a problem with mixing viewports between APIs. I've summarized the
situation in this doc:
https://docs.google.com/document/d/1E6tBwLF1UlqSCMyIIemaRheJrtLncsNetV7y6we-muE/edit?usp=sharing

Basically, some APIs (window.scroll_) currently refer to visual while
others (getBoundingClientRect) to the layout viewport. This mixing has led
to a long tail of bugs. Conceptually, we'd prefer to make everything
relative to the layout viewport. We tried this by shipping "inert visual
viewport" in M48 but got push back from web devs and reverted it.

I don't know what your new implementation does w.r.t. these APIs but since
you're making a big change this would be a good time to settle on the same
behavior. What do you think of the "inert" model described in the doc (and
available in Chrome via chrome://flags/#inert-visual-viewport)?
Essentially, it locks the UA, as far as the page can tell, into the fully
zoomed-out state.

Edge engineers have expressed cautious support but were worried by the
compat impact. FWIW, we shipped for a whole milestone and didn't get any
reports of broken pages from users. We did get significant push back from
developers in http://crbug.com/571297 but it was mostly along the lines of
interop. If we could get all the engines on the same page I think it'd be a
significant improvement over the status quo.

In any case, we'll need to change one way or the other. Mixing the two
viewports in APIs is confusing devs and a source of bugs on existing pages.
The alternative is to make the "client" coordinates relative to the visual
viewport. Whatever we do, I'd like to improve interop by settling on the
same behavior between all the engines.

Thanks,
David

> Hi floks!
>
> I plan to enable visual viewports by default in WK1 and WK2 in the near 
> future.
>
> "visual viewports" is new behavior for position:fixed and sticky elements 
> under page zoom; they lay out relative to a "layout viewport" (which is the 
> size of the initial containing block), while the user pans around in the 
> "visual viewport". When the visual viewport hits the edge of the layout 
> viewport, it pushes this around.
>
> This is a better user experience, and also matches Chrome and Firefox 
> behavior more closely.
>
> Is there any platform which would object to having this new behavior by 
> default? I hope to remove the non-visual-viewport code at some point.
>
> The master bug for this is https://bugs.webkit.org/show_bug.cgi?id=164260 
> .
>
> Simon
>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Myles C. Maxfield]

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

Re: [webkit-dev] [webkit-reviewers] usage of auto

[Simon Fraser]

> On Jan 10, 2017, at 9:03 PM, Darin Adler  wrote:
> 
> This kind of discussion should be on webkit-dev, not webkit-reviewers. While 
> the reviewers may have more standing to decide about such things, we normally 
> want to discuss them in the open.

Agreed. Moving there.

> If you don’t like “auto”, please first ask yourself first whether it is 
> because of years of experience reading and writing C++ code without it.
> 
> I like auto, auto&, and auto* pretty much everywhere they can be used. (I 
> almost never like const auto& or const auto*.)
> 
> As Brady pointed out, I find that auto* helps make it clear something is a 
> pointer, and I often prefer that to auto when a pointer is involved.
> 
> Without auto, we often convert types unnecessarily, for example, we call a 
> function and put a pointer to a RenderElement into a RenderObject*. Or 
> convert integer types from one integer type to another unnecessarily. Or 
> convert a Ref to a RefPtr even though the function we are calling can never 
> return a null.
> 
> It’s easy to get the illusion that if you don’t use auto you can “see” the 
> types in the program, but this true only in a limited sense. You can’t see 
> the types of expressions and subexpressions, only of variables. And if you 
> use auto, you can see that there is no type conversion. But if you use an 
> explicit type, you can’t see whether the type on the left matches the type on 
> the right, so this actually hides the “no type conversion here” information.
> 
> It’s often irrelevant what the type is to understanding the code, more 
> important to know what a value represents rather than its type. Specific 
> types can be a distraction. For example, if I am iterating a collection and 
> adding together the result of calling the width function on each element, do 
> I need to state what kind of object is in the collection? It both seems 
> unimportant, and can be a distraction. Arguably, the type of the result of 
> the width function is important, but I don’t know a way to make *that* type 
> visible. I can make what type I put the result into visible, but that still 
> doesn’t tell me what the type of the return value of the width function is.
> 
> I agree with Filip it can be good to write out the name of the type when we 
> want to document what the type is. But for me, this almost never comes up in 
> the kind of programming that I do on the project.
> 
> I’d love to see examples where using auto substantially hurts readability so 
> we could debate them.

Some contrary examples in code that I’ve seen/reviewed:

auto countOfThing = getNumberOfThings();
ASSERT(countOfThing >= 0);  // Can’t tell by reading whether the ASSERT is 
assured at compile time if countOfThing is unsigned

auto thingLength = getLengthOfThing();
IntSize size(thingLength, 2); // Can’t tell by reading if thingLength is 
LayoutUnit or float and thus truncated here.

Another common issue in code I’m not familiar with is something like:

auto fancyStyleThing = styleMagic.styleThingForDoohicky()

where it maybe obvious to the author what the type of fancyStyleThing is, but 
without looking at StyleMagic::styleThingForDoohicky() I have no idea what it 
is, and Xocde doesn’t help me. You argue above that maybe I don’t need to know 
the exact type, but often I do if I’m trying to figure out how various 
components relate to each other, rather than studying the logic of one specific 
function.

Simon




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

Re: [webkit-dev] [webkit-reviewers] usage of auto

[Darin Adler]

> On Jan 10, 2017, at 9:46 PM, Simon Fraser  wrote:
> 
> auto countOfThing = getNumberOfThings();
> ASSERT(countOfThing >= 0);  // Can’t tell by reading whether the ASSERT is 
> assured at compile time if countOfThing is unsigned

I understand wanting to know, but I am not certain this is a bad thing.

> auto thingLength = getLengthOfThing();
> IntSize size(thingLength, 2); // Can’t tell by reading if thingLength is 
> LayoutUnit or float and thus truncated here.

Makes total sense to me. This is a bad pattern.

> Another common issue in code I’m not familiar with is something like:
> 
> auto fancyStyleThing = styleMagic.styleThingForDoohicky()
> 
> where it maybe obvious to the author what the type of fancyStyleThing is, but 
> without looking at StyleMagic::styleThingForDoohicky() I have no idea what it 
> is, and Xocde doesn’t help me. You argue above that maybe I don’t need to 
> know the exact type, but often I do if I’m trying to figure out how various 
> components relate to each other, rather than studying the logic of one 
> specific function.

If the type of fancyStyleThing was given here it would not tell you the type of 
styleThingForDoohicky; it would tell you what type we are assigning to but 
there could be a type conversion.

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

Re: [webkit-dev] [webkit-reviewers] usage of auto

[JF Bastien]

>
> auto thingLength = getLengthOfThing();
> IntSize size(thingLength, 2); // Can’t tell by reading if thingLength is
> LayoutUnit or float and thus truncated here.
>

The same is true for:
int thingLength = getLengthOfThing();
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Darin Adler]

> On Jan 10, 2017, at 9:24 PM, Myles C. Maxfield  wrote:
> 
> We opted for WebCore to include files using “#include ” instead of 
> just including Foo.h.

Will this work without ForwardingHeaders on macOS and iOS?

Given that WTF chose , what is the reasoning for PAL choosing the 
all capital ?

> 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”.

Given that WebCore is built on top of PAL, it seems a little strange to put PAL 
“inside” WebCore, but I guess OK.

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

Re: [webkit-dev] [webkit-reviewers] usage of auto

[Darin Adler]

> On Jan 10, 2017, at 9:49 PM, Darin Adler  wrote:
> 
>> On Jan 10, 2017, at 9:46 PM, Simon Fraser  wrote:
>> 
>> auto countOfThing = getNumberOfThings();
>> ASSERT(countOfThing >= 0);  // Can’t tell by reading whether the ASSERT is 
>> assured at compile time if countOfThing is unsigned
> 
> I understand wanting to know, but I am not certain this is a bad thing.

Sorry, let me say something different, but related:

int countOfThing = getNumberOfThings();

Can’t tell from the above code if getNumberOfThings() returns int or unsigned.

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

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

[Carlos Garcia Campos]

El mar, 10-01-2017 a las 21:24 -0800, Myles C. Maxfield escribió:
> After 18 months of no progress, Don Olmstead and I are getting the
> band back together!

Great news. 

> 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.

So, will it be possible to link to PAL but not the rest of WebCore?

> 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”.

And WebCore::PAL::Foo when used outside WebCore namespace, right?

> 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!

I'm not sure I agree with the approach because I don't know the reasons
 to make it a target inside WebCore, could you elaborate more on that,
please?

> Thanks,
> Myles and Don
> 
> > On Mar 22, 2015, at 4:40 PM, Gavin Barraclough  > om> 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] [webkit-reviewers] usage of auto

[Alex Christensen]

>> I’d love to see examples where using auto substantially hurts readability so 
>> we could debate them.
I once saw a RefPtr changed to auto in some generated code where it 
was unclear what the return type was.  For at least one generated instance the 
return type was Something* that needed a reference kept alive by the caller, so 
this change caused a subtle use-after-free bug.  See 
https://trac.webkit.org/changeset/201345 


Also when we change what a return type is but there are call sites that use 
auto, we may miss checking to see if everything is ok at a call site that 
compiles successfully even though it has different meaning.

I’ll admit auto has grown on me quite a bit since I wrote 
https://lists.webkit.org/pipermail/webkit-dev/2014-January/026000.html 


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

Re: [webkit-dev] [webkit-reviewers] usage of auto

[Darin Adler]

> On Jan 10, 2017, at 10:17 PM, Filip Pizlo  wrote:
> 
> while (Arg src = worklist.pop()) {
> HashMap::iterator iter = 
> mapping.find(src);
> if (iter == mapping.end()) {
> // With a shift it's possible that we previously built 
> the tail of this shift.
> // See if that's the case now.
> if (verbose)
> dataLog("Trying to append shift at ", src, "\n");
> currentPairs.appendVector(shifts.take(src));
> continue;
> }
> Vector pairs = WTFMove(iter->value);
> mapping.remove(iter);
> 
> for (const ShufflePair& pair : pairs) {
> currentPairs.append(pair);
> ASSERT(pair.src() == src);
> worklist.push(pair.dst());
> }
> }

Here is the version I would write in my favored coding style:

while (auto source = workList.pop()) {
auto foundSource = mapping.find(source);
if (foundSource == mapping.end()) {
// With a shift it's possible that we previously built the tail of 
this shift.
// See if that's the case now.
if (verbose)
dataLog("Trying to append shift at ", source, "\n");
currentPairs.appendVector(shifts.take(source));
continue;
}
auto pairs = WTFMove(foundSource->value);
mapping.remove(foundSource);
for (auto& pair : pairs) {
currentPairs.append(pair);
ASSERT(pair.source() == source);
workList.push(pair.destination());
}
}

You argued that specifying the type for both source and for the iterator helps 
reassure you that the types match. I am not convinced that is an important 
interesting property unless the code has types that can be converted, but with 
conversions that we must be careful not to do. If there was some type that 
could be converted to Arg or that Arg could be converted to that was a 
dangerous possibility, then I grant you that, although I still prefer my 
version despite that.

You also said that it’s important to know that the type of foundSource->value 
matches the type of pairs. I would argue that’s even clearer in my favored 
style, because we know that auto guarantees that are using the same type for 
both, although we don’t state explicitly what that type is.

Here’s one thing to consider: Why is it important to see the type of 
foundSource->value, but not important to see the type of shifts.take(source)? 
In both cases they need to be Vector.

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

Re: [webkit-dev] [webkit-reviewers] usage of auto

[Chris Dumez]

I usually like using auto / auto* as much as possible.

The one exception where I have found using auto confusing was for functions 
returning an std::optional. 

E.g.
auto value = maximum();
if (!value)
return;

I find that the check is confusing because it returns early if value is 0 in 
the case where maximum() returns an integer but checks if the value is set in 
the case the function returns an std::optional.

Chris Dumez

On Jan 10, 2017, at 9:51 PM, Darin Adler  wrote:

>> On Jan 10, 2017, at 9:49 PM, Darin Adler  wrote:
>> 
>>> On Jan 10, 2017, at 9:46 PM, Simon Fraser  wrote:
>>> 
>>> auto countOfThing = getNumberOfThings();
>>> ASSERT(countOfThing >= 0);  // Can’t tell by reading whether the ASSERT is 
>>> assured at compile time if countOfThing is unsigned
>> 
>> I understand wanting to know, but I am not certain this is a bad thing.
> 
> Sorry, let me say something different, but related:
> 
>int countOfThing = getNumberOfThings();
> 
> Can’t tell from the above code if getNumberOfThings() returns int or unsigned.
> 
> — 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] [webkit-reviewers] usage of auto

[Filip Pizlo]

Brady asked:

> Have you identified any situation where explicitly calling out the type in a 
> range-based for loop has been better than using the proper form of auto?

> Have you identified a situation where explicitly calling out a nasty 
> templated type, like in my example, added to readability over using auto?

Darin asked:

> I’d love to see examples where using auto substantially hurts readability so 
> we could debate them.


In many places, I agree that auto is better.  But there are a bunch of 
algorithms in the compiler and GC where knowing the type helps me read the code 
more quickly.  Here are a few examples.  (1) is a loop, (2) is loop-like, and 
(3) is a nasty templated type.

1) B3 compiler code cloning loop.

I find that this code is easier to read with types:

Value* clone = m_proc.clone(value);
for (Value*& child : clone->children()) {
if (Value* newChild = mappings[i].get(child))
child = newChild;
}
if (value->type() != Void)
mappings[i].add(value, clone);

cases[i]->append(clone);
if (value->type() != Void)
cases[i]->appendNew(m_proc, value->origin(), 
clone, value);

Here's another code cloning loop I found - sort of the same basic algorithm:

for (Value* value : *tail) {
Value* clone = m_proc.clone(value);
for (Value*& child : clone->children()) {
if (Value* replacement = map.get(child))
child = replacement;
}
if (value->type() != Void)
map.add(value, clone);
block->append(clone);
}

When reading this code, it's pretty important to know that value, clone, child, 
newChild, and replacement are all Values.  As soon as you know this piece of 
information the algorithm - its purpose and how it functions - becomes clear.  
You can infer this information if you know where to look - m_proc.clone() and 
clone->children() are give-aways - but this isn't as immediately obvious as the 
use of the type name.

If someone refactored this code to use auto, it would be harder for me to read 
this code.  I would spend more time reading it than I would have spent if it 
spelled out the type.  I like seeing the type spelled out because that's how I 
recognize if the loop is over blocks, values, or something else.  I like to 
spell out the type even when it's super obvious:

for (BasicBlock* block : m_proc) {
for (Value* value : *block) {
if (value->opcode() == Phi && candidates.contains(block))
valuesToDemote.add(value);
for (Value* child : value->children()) {
if (child->owner != block && 
candidates.contains(child->owner))
valuesToDemote.add(child);
}
}
}

Sticking to this format for compiler loops means that I spend less time reading 
the code, because I can recognize important patterns at a glance.

2) Various GC loops

The GC usually loops using lambdas, but the same question comes into play: is 
the value's type auto or is it spelled out?

forEachFreeCell(
freeList,
[&] (HeapCell* cell) {
if (false)
dataLog("Free cell: ", RawPointer(cell), "\n");
if (m_attributes.destruction == NeedsDestruction)
cell->zap();
clearNewlyAllocated(cell);
});

It's useful to know that 'cell' is a HeapCell, not a FreeCell or a JSCell.  I 
believe that this code will only compile if you say HeapCell.  Combined with 
the function name, this tells you that this is a cell that is free, but not 
necessarily on a free-list ('cause then it would be a FreeCell).  This also 
tells you that the cell wasn't necessarily a JSCell before it was freed - it 
could have been a raw backing store.  That's important because then we don't 
have a guarantee about the format of its header.

I think that spelling out the type really helps here.  In the GC, we often 
assert everything, everywhere, all the time.  Typical review comes back with 
suggestions for more assertions.  Types are a form of assertion, so they are 
consistent with how we hack the GC.

3) AirEmitShuffle

My least favorite part of compilers is the shuffle.  That's the algorithm that 
figures out how to move data from one set of registers to another, where the 
sets may overlap.

It has code like this:

while (Arg src = worklist.pop()) {
HashMap::iterator iter = 
mapping.find(src);
if (iter == mapping.end()) {
// With a shift it's possible that we previously built the 
tail of this shift.
// See if that's the case now.
if (verbose)
dataLog("Trying 

[webkit-dev] some problems and wishes

[Oleg Milovidov]

Dear Sirs!

I built WebKit2Gtk release 14.2 under Linux Mint 18 gcc compiler on
13/11/2016.

It works stable!

I use the product for embed web C++ application (Gtk-3).

I noticed some problems and I have some wishes with the product.

1. Size of downloading resource.

It is important for user to know the size of downloading resource. I know
that servers sometimes do not send this information. But sometimes other
browsers and curl can receive the size of resource, when WebKit2Gtk returns
0. I use curl in this case internally.

I wrote embed WebBrowser under MS Windows. IE WebBrowser Control always
knows the size of resource. Sometimes initially IE WebBrowser Control
returns 0, but when downloading starts, the IDownloadManager always know
total size of resource.

2. Scroll bars rendering often is strange. Scroll bars change own images.
Some old parts of page under transparent scroll bars are on them. Scroll
bars need to be invalidated?

3. Segmentation error when I was trying to read:

bool value = webkit_settings_get_allow_universal_access_from_file_urls(se
ttings);

4. Compilation of an extension was problem. I defined symbol
__WEBKIT2_H_INSIDE__. After that I can compile an extension with WebView
and manually included ,
 and others.

5. I was trying to use model: WEBKIT_PROCESS_MODEL_MULTIPLE_
SECONDARY_PROCESSES.

But second web page in the next notebook page do not start downloading. So
I was unsuccessful with the model.

6. User agent string. Well known in Russia mass media site
http://www.rbc.ru/ detected my browser as very old and there were serious
problem with rendering the site. When I change UAS to "Mozilla/5.0 (X11;
Linux) AppleWebKit/602.1 (KHTML, like Gecko) Chrome/54.0.2840.59 Safari/8"
problem disappeared.

But Google maps begun inform me with new UAS that «It is simplified mode.
Your browser do not support full version of Google maps». I consider that
Google maps rendering with Google Chrome is better that with WebKit2Gtk. So
Google maps is a good test from my point of view.

7. Font size for Web Inspector is too small for me. Some editable style
config for Inspector, including zoom control

will be good.


All I wrote may be not very serious remarks.

Thank you for the product!

I'll be wait new releases of WebKit2Gtk with inpatianci.


Best regards,

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