issue #362 - firefox: block image broken on youtube thumbnails (recommended videos)

This review is related to current ticket:
https://issues.adblockplus.org/ticket/362

[saroyanm, 2014-08-10 12:24:49]

Wladimir can you please have a look on current review, when you have time. 

I was thinking about several solutions:
1. Save objects in array to have ordered list of objects.
2. Implement last property in "RequestEntry" class and update all enumerated
properties on attachToNode call.
But finally decided only save information about last RequestEntry, because
couldn't find case when we will need all loaded node datas.

Let me know if other solution is preferable here, if I'm missing something.
Hope we can be okey with current solution.

[Wladimir Palant, 2014-08-14 20:44:57]

http://codereview.adblockplus.org/5681038109966336/diff/5629499534213120/lib/...
File lib/requestNotifier.js (right):

http://codereview.adblockplus.org/5681038109966336/diff/5629499534213120/lib/...
lib/requestNotifier.js:361: setEntry(nodeData, node, this);
The the list of blockable items is broken now because startScan() method isn't
working correctly.

We do need to save all the requests that we receive for a node. This is
especially true for the document node which will be associated with numerous
requests (e.g. XMLHttpRequests or background images).

I suggest adding a RequestEntry.id property which will simply increase with each
new object created:

  function RequestEntry(...)
  {
    this.id = ++maxId;

RequestNotifier.getDataForNode() should then return whichever matching entry has
the highest ID.

[saroyanm, 2014-08-17 14:25:20]

New Patch uploaded.

http://codereview.adblockplus.org/5681038109966336/diff/5629499534213120/lib/...
File lib/requestNotifier.js (right):

http://codereview.adblockplus.org/5681038109966336/diff/5629499534213120/lib/...
lib/requestNotifier.js:361: setEntry(nodeData, node, this);
On 2014/08/14 20:44:57, Wladimir Palant wrote:
> The the list of blockable items is broken now because startScan() method isn't
> working correctly.
> 
> We do need to save all the requests that we receive for a node. This is
> especially true for the document node which will be associated with numerous
> requests (e.g. XMLHttpRequests or background images).
> 
> I suggest adding a RequestEntry.id property which will simply increase with
each
> new object created:
> 
>   function RequestEntry(...)
>   {
>     this.id = ++maxId;
> 
> RequestNotifier.getDataForNode() should then return whichever matching entry
has
> the highest ID.

Oops, Thanks for that.

[Wladimir Palant, 2014-08-18 11:39:19]

Nice, LGTM