Use a port for communication #15
Conversation
…This makes sure that no other extension communication can interfere with the screen capture.
…EADME to tell people they need to change that variable if they rename page.js
|
I just sullied this pull request a bit with that last push. I should have made the change in #14. Sorry! |
…nect connections.
|
Hi Peter. Sorry for so many ongoing commits on this pull request. I am now integrating this extension's code into one of my extensions, and so am ironing out a few kinks. It's working! It was very easy, just a few lines of code added and my other extension now has screenshots. Thanks :-) |
…ust had it take >1 second). Add 100ms delay before disconnecting the port to avoid occasional exception being thrown due to last message ('done') being in flight.
|
How about you get it to a steady state and then create a pull request with that? |
|
I've been using it for a while now, without further problems. Did you try it? Sorry for all this activity, I hope I'm not being a pain! I don't have any more large changes in mind, BTW :-) |
…0ms was definitely too short. Test the value of 'port' before using it after a setTimeout.
|
OK, I'll pull this down & do some more work on it tomorrow. |
|
Hi Peter I've done some more playing with this and it it's working well for me. I increased two timeouts to 3 seconds just to be conservative. Those are the timeout for the injection to be loaded and the timeout for the extension to ask the page to send another arrangement. This code is still based on #14 so the diff against master is bigger than the diff against #14 |
|
Hi again Peter. I did some more work on this to fix a problem with duplicate listening for messages in the code I'd written (elsewhere) that uses this code. I also added a I don't mind at all if you don't feel like this change should land, just let me know if you can. I'm doing all this to help others, and I think these changes are good for that (isolating the extensions messaging from that of any other extension into which this code is incorporated). |
…not 1 (the value on a MacBook Pro with Retina display is 2).
This is a branch from #14, so that should be merged first and then I can merge master into this branch to clean up the diff.
All communications are done via a port that exists for the lifetime of the screen capture. When the capture is done the port is closed (by the content script). The incoming port connection in
api.jsis checked to make sure it's definitely for us. This makes it pretty much impossible to (accidentally) get crossed wires due to simpler messaging in any extension that uses this code.The flow of control logic changes a little, in a way that I think is cleaner and more explicit. There are messages passed now to request a new arrangement, to request a capture, to indicate the capture is done. The possible failure modes are clean / easy to understand.