Opened 2 weeks ago

Last modified 12 days ago

#7260 new change

Internalize third-party request check in matcher

Reported by: mjethani Assignee: mjethani
Priority: P3 Milestone:
Module: Core Keywords:
Cc: sebastian Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29998564/

Description

Background

We moved the third-party request checking function into adblockpluscore recently (#7232). But there's no reason for external code to call this function directly: the matcher can check if a request is third-party based on the URL and the document's domain. Because most requests hit the cache anyway, of which we recently increased the capacity (#7254), the check wouldn't even have to be done in most cases.

What to change

[TBD]

Change History (5)

comment:1 Changed 2 weeks ago by sebastian

  • Cc sebastian added

comment:2 Changed 2 weeks ago by mjethani

  • Review URL(s) modified (diff)

comment:3 Changed 2 weeks ago by mjethani

  • Priority changed from P2 to P3

I am not seeing any performance benefit after the changes. In fact, maybe it even hurts the performance. In any case, this is no longer about performance. As such I am setting this to a lower priority for now. Let's look into this for a future release.

comment:4 follow-up: Changed 2 weeks ago by sebastian

Even if there is no performance benefit per-se, I think from an API point of view it doesn't make any sense to provide an isThirdParty() function just so that the calling code can pass the result into the matcher. The simplifications in adblockpluschrome seem well worth it. After all, this was the reason we moved the public suffix list and related logic into core in the first place.

Last edited 2 weeks ago by sebastian (previous) (diff)

comment:5 in reply to: ↑ 4 Changed 12 days ago by mjethani

Replying to sebastian:

Even if there is no performance benefit per-se, I think from an API point of view it doesn't make any sense to provide an isThirdParty() function just so that the calling code can pass the result into the matcher.

I agree.

Note: See TracTickets for help on using tickets.