Conditional import of adblocker.

Practical fixes for ad filtering.

Only import adblocker if we actually define any rules. If we define
rules, and fail to import adblockerparser, give detailed installation
instructions.

Fix typo that prevented the filtering from working in the model, and have the filtering actually use the new adblocker matcher rather than the old heuristic.

Committed: https://crrev.com/8497c4725e9a01b5c05e8f10af5e523fa77d1580
Cr-Commit-Position: refs/heads/master@{#372079}

[mattcary, 2016-01-28 10:21:51]

Description was changed from

==========
Conditional import of adblocker.

Only import adblocker if we actually define any rules. If we define
rules, and fail to import adblockerparser, give detailed installation
instructions.

BUG=
==========

to

==========
Conditional import of adblocker.

Only import adblocker if we actually define any rules. If we define
rules, and fail to import adblockerparser, give detailed installation
instructions.
==========

[mattcary, 2016-01-28 10:21:51]

mattcary@chromium.org changed reviewers:
+ blundell@chromium.org, lizeb@chromium.org

[mattcary, 2016-01-28 11:44:15]

Oops, I don't think I successfully mailed this out.

Colin & Benoit---

Here is the change Colin & I discussed to make installing adblocker optional
when it's not used.

[Benoit L, 2016-01-28 11:53:37]

lgtm, thank you.

[blundell, 2016-01-28 11:55:36]

lgtm

https://codereview.chromium.org/1645953002/diff/1/tools/android/loading/conte...
File tools/android/loading/content_classification_lens.py (right):

https://codereview.chromium.org/1645953002/diff/1/tools/android/loading/conte...
tools/android/loading/content_classification_lens.py:96: if self._rules:
Maybe a comment above this like "Only try to include adblockparser if the user
has actually supplied rules to avoid requiring that all users install
adblockparser."

[mattcary, 2016-01-28 12:00:10]

Expanded scope to fix misc ad bug and replace my old ad-hoc filtering.

[blundell, 2016-01-28 12:04:26]

On 2016/01/28 12:00:10, mattcary wrote:
> Expanded scope to fix misc ad bug and replace my old ad-hoc filtering.

I would split the loading_model* changes into a separate CL unless there's a
relationship I'm missing.

[chromium-reviews, 2016-01-28 12:06:13]

Both the node->node_info and FilterAds change is necessary to get analyze
to actually use the new rules, so it seemed appropriate. Let me change the
CL title.

On Thu, Jan 28, 2016 at 1:04 PM, <blundell@chromium.org> wrote:

> On 2016/01/28 12:00:10, mattcary wrote:
> > Expanded scope to fix misc ad bug and replace my old ad-hoc filtering.
>
> I would split the loading_model* changes into a separate CL unless there's a
> relationship I'm missing.
> https://codereview.chromium.org/1645953002/
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[mattcary, 2016-01-28 12:06:58]

Description was changed from

==========
Conditional import of adblocker.

Only import adblocker if we actually define any rules. If we define
rules, and fail to import adblockerparser, give detailed installation
instructions.
==========

to

==========
Practical fixes for ad filtering.

Only import adblocker if we actually define any rules. If we define
rules, and fail to import adblockerparser, give detailed installation
instructions.

Fix typo that prevented the filtering from working in the model, and have the
filtering actually use the new adblocker matcher rather than the old heuristic.
==========

[Benoit L, 2016-01-28 12:22:20]

On 2016/01/28 12:06:13, chromium-reviews wrote:
> Both the node->node_info and FilterAds change is necessary to get analyze
> to actually use the new rules, so it seemed appropriate. Let me change the
> CL title.
> 
> On Thu, Jan 28, 2016 at 1:04 PM, <mailto:blundell@chromium.org> wrote:
> 
> > On 2016/01/28 12:00:10, mattcary wrote:
> > > Expanded scope to fix misc ad bug and replace my old ad-hoc filtering.
> >
> > I would split the loading_model* changes into a separate CL unless there's a
> > relationship I'm missing.
> > https://codereview.chromium.org/1645953002/
> >
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

still lgtm

[mattcary, 2016-01-28 12:23:51]

The CQ bit was checked by mattcary@chromium.org

[mattcary, 2016-01-28 12:23:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
blundell@chromium.org
Link to the patchset: https://codereview.chromium.org/1645953002/#ps40001
(title: "Use new filters for ad filtering")

[commit-bot: I haz the power, 2016-01-28 12:24:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1645953002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1645953002/40001

[commit-bot: I haz the power, 2016-01-28 12:42:31]

Description was changed from

==========
Practical fixes for ad filtering.

Only import adblocker if we actually define any rules. If we define
rules, and fail to import adblockerparser, give detailed installation
instructions.

Fix typo that prevented the filtering from working in the model, and have the
filtering actually use the new adblocker matcher rather than the old heuristic.
==========

to

==========
Practical fixes for ad filtering.

Only import adblocker if we actually define any rules. If we define
rules, and fail to import adblockerparser, give detailed installation
instructions.

Fix typo that prevented the filtering from working in the model, and have the
filtering actually use the new adblocker matcher rather than the old heuristic.
==========

[commit-bot: I haz the power, 2016-01-28 12:42:32]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-01-28 12:43:28]

Description was changed from

==========
Practical fixes for ad filtering.

Only import adblocker if we actually define any rules. If we define
rules, and fail to import adblockerparser, give detailed installation
instructions.

Fix typo that prevented the filtering from working in the model, and have the
filtering actually use the new adblocker matcher rather than the old heuristic.
==========

to

==========
Practical fixes for ad filtering.

Only import adblocker if we actually define any rules. If we define
rules, and fail to import adblockerparser, give detailed installation
instructions.

Fix typo that prevented the filtering from working in the model, and have the
filtering actually use the new adblocker matcher rather than the old heuristic.

Committed: https://crrev.com/8497c4725e9a01b5c05e8f10af5e523fa77d1580
Cr-Commit-Position: refs/heads/master@{#372079}
==========

[commit-bot: I haz the power, 2016-01-28 12:43:29]

Patchset 3 (id:??) landed as
https://crrev.com/8497c4725e9a01b5c05e8f10af5e523fa77d1580
Cr-Commit-Position: refs/heads/master@{#372079}