mozilla :: #pdfjs

19 Apr 2017
17:51mukulyury: Hi
18:23yurymukul: hi
18:27mukulyury: I am working to block pull and cancel method using capability
18:27mukuland following the same pattern as of start method
18:29mukulsomething like this
18:37yurysendWithStream part is correct
18:39yurymukul: rename 'start' to 'start_complete' and add 'pull_complete', 'cancel_complete'
18:40yuryif pull signalled that pull complete it does not mean it wants to call enqueue
18:40yuryalso it can call enqueue more that once
18:42mukuli thought when we have data in onPull we are calling enqueue immediately
18:43mukul`it can call enqueue more than once`, for a single pull?
18:49yurywhy not?
18:51yuryonPull,onCancel may return a promise
18:51yurymukul: Promise.resolve(pullResult).then() shall wait and post back pull_complete
18:55mukul^^^^^^ in onPull?
19:03yuryafter the onPull called
19:06mukulOkay, you mean when we have chunk in onPull it returns a promise and after that we are going to post pull_complete
19:07yurynope, I mean that it shall be handled similar to how start_complete is handled
19:08yuryhow do you handle start_complete?
19:08yuryyou said "and following the same pattern as of start method" and I agree with that
19:09yurybut that's not what I see in the code
19:09mukulOkay, i think i am getting your point.
19:10mukulI have to follow the pattern on both side
19:10mukulnot just for sendWithStream
19:11mukulSo, i get a promise returned in the `pull` case, and then i have to post pull_complete
19:11mukulusing streamSinks[data.streamId].onPull(data.desiredSize).then()
19:12yurysure, but if you do `var pullResult = streamSinks[data.streamId].onPull(data.desiredSize);` and then `Promise.resolve(pullResult).then()` you don't have to worry about returning a promise from onPull
19:13yurybut if you will return a promise from onPull it will block then until it will be resolved
19:16mukul`it will block onPull` means it is not going to call sink.enqueue
19:17yurynot exactly what I said, but pull_complete will not be called until result from the onPull will be resolve
19:18yuryit's not related to enqueue since it will be called before that
19:19yuryinside pull enqueue's or close can be called, but preferably not after
19:21mukulyury: more than one enqueue's are called inside pull because we defined highWaterMark?
19:21yuryall you have to do is provide desiredSize of chunks
19:22yurymukul: in our case we will equeue one tine
19:23yury(but before pull reports its promise is resolved)
19:25yurymukul: looking at, you don't complete pull if enqueue is not called (e.g. when only cancel is called)
19:25yuryeh, (e.g. when only close is called)
19:27yurywhen 'cancel' called you don't have access to the controller, all you need to do it call onCancel and send cancel_compelete similar to the onPull
19:28mukulHmm...., that mean completion of pull depends calling of enqueue
19:29mukulIn my code
19:29yurypull -> onPull -> (enqueue* close?) -> pull_compete
19:29yurycancel -> onCancel -> ... -> cancel_complete
19:31yurygetReader() -> (action_handling) -> start_complete
19:33yuryif you think we need, onStart for symmetry we can do that :)
19:34mukulHmm..., sounds good :)
19:34mukulwe can do this for symmetry
19:51mukulyury: what we are going to do in onCancel?
19:52mukulNow code would look like this
19:55yuryonCancel if something needs to be destroyed closed -- we will do that (atm in getTextContent) we don't need to
19:57yurywrap onCancel's result with Promise.resolve()
19:58yurymukul: but this looks fine, so it shall work
19:58yuryuse streamSinks too
20:00yuryfew touches: after onCancel (and also close) called streamSinks record shall be removed
20:01yurythe similar of other side: 'cancel_complete' and 'close'
20:01yuryto remove controller
20:01mukulmeans i have to remove particular sink from streamSinks
20:02yuryyeah, see how removing of callbacks is done
20:03yurydelete callbacksCapabilities[callbackId];
20:03mukulwe can't simply delete the particular object
20:03mukulwith a streamId?
20:04* yury needs to see code
20:07mukulhere is full code
20:08yuryline 54 need to be fixed to look like 21-22
20:09yuryat lines 45 and 52 `delete streamControllers[data.streamId];` can be called
20:10yuryand at lines 102 and 71 sink can be removed similar way
20:11yuryother that that you shall have fully running getTextContent
20:31mukulyury: i am not getting what onCancel method returns
20:32* mukul still have some confusion with onCancel working
20:34mukulfew doubts to be clear to finish this up
21:13yurymukul: I don't understand the confusion. For getTextCnt it's empty, but it might return a promise some day
21:13yurySimilar to onPull
21:16mukulThat is what i am confused about, i don't have something to return for now
21:18github_pdfjs[pdf.js] timvandermeij pushed 2 new commits to master:
21:18github_pdfjspdf.js/master a77130a Jonas Jenwald: Remove the `signchromium` target from `make.js`...
21:18github_pdfjspdf.js/master 93f3454 Tim van der Meij: Merge pull request #8315 from Snuffleupagus/rm-signchromium...
21:18soakbotPull 8315: Remove the `signchromium` target from `make.js`.
21:20github_pdfjs[pdf.js] timvandermeij pushed 2 new commits to master:
21:20github_pdfjspdf.js/master 3b59169 Jonas Jenwald: [Firefox addon] Upstream changes from: Bug 1346616 - Migrate callsites that are retrieving requested locale from pref, to use LocaleService::GetRequestedLocales...
21:20github_pdfjspdf.js/master 2928578 Tim van der Meij: Merge pull request #8313 from Snuffleupagus/bug-1346616...
21:20soakbotPull 8313: [Firefox addon] Upstream changes from: Bug 1346616 - Migrate callsites that are retrieving requested locale from pref, to use LocaleService::GetRequestedLocales.
21:20yury Promise.resolve() will wrap that and will give you a promise anyway
21:21yurymukul: you don't need to return anything in onPull too
21:23yurymukul: you will need that to not perform check if result is promise or not
21:25yuryLine 107 need this wrap too
21:26* yury taking about pastebin lines
21:27mukulokay, i will change that too
21:27yuryNew version of paste bin?
21:33yuryDid you try to run the code yet?
21:33yuryIt shall work imho
21:34mukulno, i am just modifying the sendWithStream branch for now
21:34mukuland it does not contain getTextContent function and handler in worker
21:34yuryPublish all of that once you done anyway
21:38github_pdfjs[pdf.js] pdfjsbot force-pushed gh-pages from dd54a3b to 49c118e:
21:38github_pdfjspdf.js/gh-pages 49c118e pdfjsbot: gh-pages site created via gulpfile.js script...
22:34github_pdfjs[pdf.js] pdfjsbot force-pushed gh-pages from 49c118e to f1ab499:
22:34github_pdfjspdf.js/gh-pages f1ab499 pdfjsbot: gh-pages site created via gulpfile.js script...
20 Apr 2017
No messages
Last message: 156 days and 16 hours ago