Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1306)

Issue 29338121: Issue 3775 - fix saving of requests (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 4 months ago by sergei
Modified:
3 years, 4 months ago
Reviewers:
Wladimir Palant
Visibility:
Public.

Description

# abpcrawler project Major points to be noted: - Each entry of "requests" array is not necessarily a real request. For each request issued from a whitelisted nested frame there are at least two entries, one for the real request with matching blocking filter and the second one is the frame location with the whitelisting filter matching that location. - Now there is no "blocked" property of the request, because we cannot actually get such information because we don't know that request has been blocked because there is no any relation between entries for the request and for the whitelisting frame. E.g. for the following page root-page - resource with block-me-URL - whitelisted frame with frame-URL by exception-filter (@@||frame-URL-domain^$document) --resource with block-me-URL the stored requests are: {block-me-URL, blocking-filter, contentType} {block-me-URL, blocking-filter, contentType} {frame-URL, filter:null, contentType:SUBDOCUMENT} {frame-URL, filter:exception-filter, contentType:DOCUMENT} {frame-URL, filter:exception-filter, contentType:DOCUMENT} {frame-URL, filter:exception-filter, contentType:DOCUMENT}

Patch Set 1 #

Total comments: 6

Patch Set 2 : address comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+13 lines, -98 lines) Patch
M lib/crawler.js View 1 7 chunks +13 lines, -98 lines 0 comments Download

Messages

Total messages: 4
sergei
3 years, 4 months ago (2016-03-11 10:32:27 UTC) #1
Wladimir Palant
https://codereview.adblockplus.org/29338121/diff/29338122/lib/crawler.js File lib/crawler.js (right): https://codereview.adblockplus.org/29338121/diff/29338122/lib/crawler.js#newcode267 lib/crawler.js:267: let requestNotifier = new RequestNotifier(tab.linkedBrowser.outerWindowID, function({type, location, filter}, scanComplete) ...
3 years, 4 months ago (2016-03-14 19:50:43 UTC) #2
sergei
https://codereview.adblockplus.org/29338121/diff/29338122/lib/crawler.js File lib/crawler.js (right): https://codereview.adblockplus.org/29338121/diff/29338122/lib/crawler.js#newcode267 lib/crawler.js:267: let requestNotifier = new RequestNotifier(tab.linkedBrowser.outerWindowID, function({type, location, filter}, scanComplete) ...
3 years, 4 months ago (2016-03-15 10:59:56 UTC) #3
Wladimir Palant
3 years, 4 months ago (2016-03-15 11:05:44 UTC) #4
LGTM

https://codereview.adblockplus.org/29338121/diff/29338122/lib/crawler.js
File lib/crawler.js (right):

https://codereview.adblockplus.org/29338121/diff/29338122/lib/crawler.js#newc...
lib/crawler.js:267: let requestNotifier = new
RequestNotifier(tab.linkedBrowser.outerWindowID, function({type, location,
filter}, scanComplete)
On 2016/03/15 10:59:56, sergei wrote:
> However, I guess, it should be documented in `RequestNotifier` constructor and
I
> actually cannot reproduce it, how to reproduce it in crawler?

Yes, it should be documented. As to reproducing - I don't know, but
RequestNotifier.onComplete() will trigger the listener with null parameter.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5