Re: IPCStream landed in mozilla-central

[Ben Kelly]

On Sat, May 21, 2016 at 2:05 PM, Ben Kelly  wrote:

>
> On May 21, 2016 9:44 AM, "Honza Bambas"  wrote:
> > If it's nsPipeInputStream then it's definitely alright.  OTOH, we do
> copy the memory, right?  I was somehow hoping that you just expose the
> IPC-allocated buffers via your own implementation of nsIInputStream,
> avoiding coping to an XPCOM pipe.
>
> That would be nice, but no.  IPC doesn't even support passing dependent
> strings as far as I know.
>
> This first iteration is just an actor like any other.  I didn't change the
> IPC internals at all.
>
Oh, I think understand better now.  I wrote a bug to implement this idea in
the future:

https://bugzilla.mozilla.org/show_bug.cgi?id=1274815

Ben
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: IPCStream landed in mozilla-central

[Ben Kelly]

On May 21, 2016 9:44 AM, "Honza Bambas"  wrote:
> If it's nsPipeInputStream then it's definitely alright.  OTOH, we do copy
the memory, right?  I was somehow hoping that you just expose the
IPC-allocated buffers via your own implementation of nsIInputStream,
avoiding coping to an XPCOM pipe.

That would be nice, but no.  IPC doesn't even support passing dependent
strings as far as I know.

This first iteration is just an actor like any other.  I didn't change the
IPC internals at all.

Ben
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: FF49a1: Page load of jumping points doesn't work like it should in Wikipedia

[Xidorn Quan]

I guess they are going to track the DOM position rather than the scrolling
offset. This does make sense, and we probably should do this as well.

There are certain details we would need to take care. Not sure whether they
are tracking the position based on the center or the top (or the bottom?).
Also, we may need to trigger scroll event when reflow changes the offset in
that case.

- Xidorn

On Sat, May 21, 2016 at 3:35 AM, Kartikaya Gupta  wrote:

> Note that this might get fixed in chrome with their new "scroll
> anchoring" feature -
> https://developers.google.com/web/updates/2016/04/scroll-anchoring?hl=en
>
> kats
>
> On Fri, May 20, 2016 at 12:15 PM, Adam Roach  wrote:
> > On 5/20/16 10:13, Gijs Kruitbosch wrote:
> >>
> >> On 20/05/2016 16:11, Tobias B. Besemer wrote:
> >>>
> >>> Plz open e.g. this URL:
> >>>
> >>>
> https://en.wikipedia.org/wiki/Microsoft_Windows#Alternative_implementations
> >>>
> >>> FF49a1 loads the page, jumps to "Alternative implementations", stays
> >>> there for 1-2 sec and then go ~1 screen-high (page) down.
> >>>
> >>> Can someone very this bug?
> >>
> >>
> >> The same thing happens in Chrome, so it seems like it's more likely to
> be
> >> an issue with Wikipedia.
> >
> >
> > The fact that turning JavaScript off prevents this behavior would
> certainly
> > seem to support that supposition.
> >
> > --
> > Adam Roach
> > Principal Platform Engineer
> > Office of the CTO
> >
> > ___
> > dev-platform mailing list
> > dev-platform@lists.mozilla.org
> > https://lists.mozilla.org/listinfo/dev-platform
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: IPCStream landed in mozilla-central

[Ben Kelly]

On May 21, 2016 7:45 AM, "Honza Bambas"  wrote:
> But that doesn't mean "a fixed length input stream" - actually I may not
even follow how you have translated this to you.

Sorry, I was thinking a single OnDataAvailable call for the one IPC call
just passing the stream.  Clearly that won't work, though.

> As I understand, your impl of Available() may return _different_ number
of bytes than the stream is then able to deliver when Read/ReadSegments on
it is called, right?  Can you explain why?

No.  Available() should work fine.  It's just an nsPipeInputStream.

So we should be able to loop and call ODA for each ReadSegments callback
after the IPC hop.

Sorry for my confusion.

Ben
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: IPCStream landed in mozilla-central

[Honza Bambas]

On 5/21/2016 2:36, Ben Kelly wrote:

On Fri, May 20, 2016 at 8:10 PM, Ben Kelly  wrote:


On Fri, May 20, 2016 at 11:09 AM, Ben Kelly  wrote:


On Fri, May 20, 2016 at 7:37 AM, Honza Bambas 
wrote:


And I do! :)  Actually any parent necko channel, mainly HTTP, which
sends data to the child process.  We also have bug 1110596 which complains
about too much memory copying in that code.
Could your IPCStream be used for that?


Yes, I think that could work in general.

I think the main issue would be compat with existing nsIStreamListeners.
These listeners might be written such that they expect the nsIInputStream
passed in OnDataAvailable() to return their entire length from a single
Available() call.  This will not be true for a streamed pipe.


Actually, the nsIStreamListener interface explicitly requires a fixed
length nsIInputStream:

"The onDataAvailable impl must read exactly |aCount| bytes of data before
returning."


But that doesn't mean "a fixed length input stream" - actually I may not 
even follow how you have translated this to you.


(more below)



From:


https://dxr.mozilla.org/mozilla-central/source/netwerk/base/nsIStreamListener.idl#18

I don't think we can use a pipe-oriented stream here without changing that
interface contract.


Is there a reason we can't just make necko code call OnDataAvailable()
multiple times with a different slice of the giant buffer?  It already has
the mechanism for chunked data.  It just needs to split the single buffer
into multiple callouts.

Ben


We probably can.  Normally it works this way:

- nsInputStreamPipe queries Available() of the input stream (count = 
Available())


- it calls (once) OnDataAvailable(inputStream, count)

- it assumes the implementation has read count from inputStream, as the 
contract suggest


- nsInputStreamPipe loops again, until Available() returns 
NS_BASE_STREAM_CLOSED - which means we have successfully read the stream 
and input stream pipe coverts it to NS_OK - or other error - which is an 
immediate abort/failure.


- when done, OnStopRequest(status)


As I understand, your impl of Available() may return _different_ number 
of bytes than the stream is then able to deliver when Read/ReadSegments 
on it is called, right?  Can you explain why?


That seems - honestly - pretty weird, but whatever.  Does it at least 
change when you actually read?  Like avail() before read - actually read 
= avail() after read? Assuming no data has been put to it in the 
meantime, of course.


The implementation of ODA expects that reading from the stream is not 
going to fail, when reading no more than |count|.  If it fails, ODA 
usually just returns immediately with an error (unrecoverable) and the 
whole stream-listener contract ends with OnStop(that-read-error) - as 
described above.


What does your pipe return when reading past EOF?  I assume 
WOULD_BLOCK?  The thing is that the stream passed to ODA is not expected 
to be non-blocking, but blocking.  WOULD_BLOCK is something that Read() 
in ODA should never return.  And the stream should actually never block, 
regardless whether is blocking or non-blocking.


If we wrap your input stream and convert read errors to something else 
we still may have a problem.  Like returning BASE_STREAM_CLOSED from 
Read() or just bytes-read = 0 + NS_OK, which is a graceful EOF - 
something ODA also doesn't expect to have to handle, faulty impls will 
just indefinitely loop - that's why we have the stream-listener contract 
all for!


Also, not all stream-listener impls are capable to handle when Read() 
fails but later it gets instead of OnStop(failure) another ODA call.


-hb-

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform