Skip to content

Meeting 2015 07 13

Jump to bottom
Lars Bergstrom edited this page Jul 13, 2015 · 1 revision

Agenda items

Last week

Attending

  • mainishearth, cimes, larsberg, lalehb, jack, jdm, pcwalton, simonsapin, ms2ger

New employee!

  • jack: ms2ger is officially a Mozilla employee today!

Surrogates in the DOM

  • simonsapin: Now, SM uses UTF16 for JS strings and they can contain surrogates. We (in Servo) use the rust builtin string type, which is UTF8 and cannot contain surrogates. When we encounter one in Rust, we panic and crash the browser. We could either use a different string type or replace them with a replacement character.
  • jack: pcwalton and I both commented in the bug that we think crashing is bad here. How should we fix this? pcwalton's warn! with a replacement character seemed good. But we don't know if doing that is web compatible. There's not a good way to know that without doing it and some instrumentation.
  • pcwalton: We should at least print it to the console. If you're browsing with Servo, you will have a console open (more than likely) so we should always know if it a site is relying on it.
  • jack: Objections to this strategy?
  • pcwalton: I believe we came into this because jesse's DOM fuzzers were blowing up.
  • simonsapin: jesse said he was hitting this error a lot and my PR was to unblock that.
  • pcwalton: panic seems like it blocks people. And fuzzing is valuable for us... I would like to make life as easy for fuzzers as possible.
  • jack: ms2ger, objections to this proposal?
  • jdm: Maybe pref it? and panic instead?
  • pcwalton: Why do this who would turn it on?
  • larsberg: I assumed that, like most prefs, the default is to do the web compatible thing and then pref to do non-web standard stuff.
  • pcwalton: Why leave a panic in?
  • jdm: We don't know what the web compat story is here, just that we're breaking things. If we aren't encountering this in the wild (i.e., just ACID3 and fuzzing), it seems easier to postpone making a firm decision on this by allowing people to control the behavior for ACID3 and fuzzing.
  • pcwalton: Seems like advantage of panic vs. warn is you're forced to do something about it. Will it be filing a bug or ragequitting? With warn you get the same ability to do telemetry but it's less obvious that it happened.
  • manish: Maybe panic in debug mode?
  • ms2ger: Given that no real content has ever triggered this, I'd rather keep the panic rather than have a warning that won't be noticed
  • pcwalton: We have to do something long-term other than panic. Otherwise you can just write a site that causes servo to panic.
  • jack: What can we do that's more noticable than warn but less than servo?
  • simonsapin: Add telemetry.
  • jack: Or the itried star.
  • pcwalton: I guess I'm nervous about it.
  • larsberg: There's panic vs. warn and web compat vs. not. I don't care what we do on panic vs. warn, but get really fidgety when we say we're going to do something that's not web compat. I'm fine removing the panic.
  • jack: How is this not web compat?
  • larsberg: Presumably, other browsers do something with unpaired surrogates.
  • pcwalton: I just don't want to leave in crashes.
  • manishearth: Maybe popup alert to submit a bug about it?
  • pcwalton: Not saying we should warn, submit to terminal, and forget about it. Should have better handling for warn (UI, telemetry, etc.).
  • larsberg: So, do we even know what we're supposed to do in this scenario with unpaired surrogates?
  • manishearth: Unspec'd. Most browsers will try to fixup & continue, probably.
  • simonsapin: A few years ago it was unclear. Now, the spec says to preserve surrogates. But, it's not clear this is required for web compatibility.
  • manishear: I doubt there are web sites relying on this...
  • ms2ger: So my hypothesis is that we'll never hit this panic in the real world
  • ms2ger: So that would tell us that the behaviour isn't constrained, and browsers can change away from the "unpaired surrogates everywhere" design
  • ms2ger: The spec says "use wtf-8", basically
  • pcwalton: Yeah, I think most sites do basic sanitization. I just don't want it to be possible for sites to cause a panic.
  • ms2ger: I absolutely agree that we shouldn't panic when we ship
  • Ms2ger: And I'd really like to have the lossy-conversion behaviour
  • pcwalton: I suspect what we'd like to do is not panic, get the spec fixed up with our behavior allowed, having discovered that no websites rely on unpaired surrogates. However, we don't know if that future will come about.
  • simonsapin: Let's ship with replacement characters and see if we get bug reports on real sites.
  • larsberg: As long jdm & ms2ger are behind it, I'll stop pushing back.
  • jdm: The question is - can we get data back on this change from the wild so that we can push back on the spec? If we remove the panic, we'll have trouble providing that data for the spec.
  • pcwalton: Then I won't push against having the -Z flag to turn off the panic. I do want to unblock jesse.
  • jdm: Pref'ing it makes sense.
  • simonsapin: Could do the lossy conversion and still have telemetry.
  • jack: Problem is that we want to have this telemetry. Otherwise, we've already lost the data.
  • Ms2ger: Once we have telemetry, I'd certainly be willing to rediscuss.
  • jack: A pref that turns the panic off is acceptable, unblocks jesse, and shouldn't affect real users. If people start commenting on HN with unpaired surrogates, we'll have to do rediscuss.
  • simonsapin: Different codepath if it's ACID3?
  • jack: Wildly reported in the press as "benchmark cheating" I believe :-)
  • jack: Simonsapin - can you add a -Z flag to do that?
  • simonsapin: OK, will look into it.

Q3 goals

  • jack: Just a reminder - get the goals/deadlines done soon. I don't know if there's a deadline?
  • azita: There was a mail with a deadline.

document.write

  • simonsapin: Same surrogate issue as in every other API here in document.write. Other than that, I think that h5e has support for adding things to the beginning of the queue. Just wiring it up in the DOM is all! It doesn't look too hard...
  • jdm: Maybe I'll make an E-LessEasy issue around it.
  • jack: I'll pick somebody to do it...
  • larsberg: I volunteered, if we run out of time.

Social media changes

  • larsberg: We have the old servo twitter account now. I propose that we start making more use of it, and we'll put together a little blurb at the start of the meeting notes rather than the weekly/monthly TWiS.
  • jack: How much work is TWiS? How automated can it be?
  • manish: If you're in the loop, not too much. If you're not, you're digging through several pages of PRs every weeks and adding human descriptions...
  • larsberg: I worry that anytime whoever is responsible goes travelling/has an internship we lose the newsletter.
  • jack: Automation that generates the report automatically can make a big impact.

Training

  • larsberg: Might turn training materials into a series of blog posts. HAve offers to translate them into Japanese too. Will have experts go over them in detail, this time.
  • jack: We have a standing slot in the weekly meeting to show stuff off for the CTO's office. Might want to show off 3d transforms.
  • azita: Short slot!
  • jack: Yep, just to show off things we're working on, like Patrick's demos from last quarter. Also got a suggestion about perf problems demoing over vidyo - record in slow motion. Maybe we could add something that causes Servo animations to run in slow-mo?

New label for panics

  • jdm: I made I-panic. From now on, panic means I-panic. Only REAL crashes should be I-crash.
  • larsberg: We should be careful to be specific from now on. We should be watching the crash label very closely - we can use this for statistics, and fix them at the architectural level when possible. Panics are important, but they're not unsafe like actual crashes.

IPC and trait objects

  • pcwalton: Ported a bunch of stuff to use it. Does it seem OK so far? The basic thing is that everything works over IPC except for sending boxed trait objects. Can send senders and receivers over IPC without problems. Just no boxed trait objects. Also, can't select over both IPC and standard Rust channels at the same time. Those two issues seem pretty fundamental. Also, can't select over multiple IPC channels, but is potentially fixable. Not sure about that on OSX, but it's definitely fixable on Linux. The boxed traits, though, are not fixable anywhere because you just can't send them over IPC without introducing security issues. Do people feel OK with this?
  • jack: How many boxed traits were there?
  • pcwalton: 3-4.
  • manishearth: Traits we own or std ones? If they're ours, we can enum-ify them.
  • pcwalton: There's a Box<Any> (to try to split crates up), but otherwise they're traits we own. Usually, the trait object is wrapping a sender or wrapping an event loop. So, it would allow you to inject messages into SM's event loop from other threads. That won't work cross-process anyway, though, so it's just fundamentally not possible even if you enum-ify them. Generally, I've been wrapping them in newtype structs with the same API wrapping the sender and keeping the API as close as possible to what it was before. The internals often had to change and usually had to spawn a new thread.
  • jdm: A single IPC thread, as gecko does?
  • pcwalton: In most cases, the threads are ad-hoc right now.
  • jdm: I mean: can we make it a single thread in the future?
  • pcwalton: Yes, it would be nice. Not sure we want to do it all the way as Gecko does. Gecko proxies everything to an IPC thread, which makes it a bit of a bottleneck today and can cause a bunch of extra context switches. However, where you're spawning a bunch of little threads (e.g., AsyncResponseTarget stuff), it would be great to avoid those since they're small and frequent. A good rule of thumb might be to avoid ad-hoc thread spawns. If they're not one-off, though (e.g., communication between graphics & layout) then just have a dedicated thread for that.
  • larsberg: I'm really glad that did this work! How reusable is this outside of Servo?
  • pcwalton: The IPC code is a standalone library and has tests, but not sure how many people will use it other than a browser.
  • jack: Server-side software stuff, though, like qmail might do it?
  • pcwalton: Not as useful for people writing pure Rust, though. We have dangerous C libraries (SM and graphics drivers).
  • jdm: The Gecko people have been wanting to replace the IPC library for years...
  • pcwalton: I've talked with jeddavis about this. We have a lot of advantages over the current Chromium library - mainly because of the compiler integration. IPDL is a lot of work generating that marshalling code, has to be fit into the project, etc. With Servo's ipcchannel, you just have compiler integration generating the boilerplate and full CSP semantics (since you can send senders and receivers over senders). Would be great. jdm, can you look at this stuff this week?
  • jdm: Yup.
  • pcwalton: Future work - we don't support shared memory (shmem) yet. But we want both untrusted & trusted, especially for display lists. I think we have a path forward.

Modular peerage

  • larsberg: The highest level governance stuff is not moving forward right now. We brought it up with brendan and dherman and they are not in favor of using the traditional module ownership system and are coming up with something else. But is there something more specific than the high-level owner/peer stuff we can talk about?
  • simonsapin: Really care about: when is it OK to land something? I was thinking all reviewers can land everything...
  • jdm: I'd restrict it when it comes to things that impact web compatibility. If it's exposed to web content, it is preferred for somebody who spends time on DOM stuff to give final approval there.
  • larsberg: Do you need lists of area owners?
  • simonsapin: Yes.
  • larsberg: How about jack and I can make a list and circulate it around?
  • jack: I'd just like to note that reverting PRs is a very blunt instrument. Lars and I will work on this; it's been a low-pri discussion over the last 4--5 months.
Clone this wiki locally