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

Issue 29705719: Issue 6402 - Split filter hit / request logging out into own API

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 months ago by kzar
Modified:
1 day, 22 hours ago
Reviewers:
Manish Jethani
CC:
Thomas Greiner, saroyanm, Sebastian Noack
Visibility:
Public.

Description

Issue 6402 - Split filter hit / request logging out into own API

Patch Set 1 #

Patch Set 2 : Fixed typo and exposed pageId for logged requests #

Patch Set 3 : Ensure devtools module is included in the bundle #

Total comments: 6

Patch Set 4 : Addressed Sebastian's feedback #

Total comments: 2

Patch Set 5 : Nightmarish rebase #

Total comments: 9
Unified diffs Side-by-side diffs Delta from patch set Stats (+172 lines, -125 lines) Patch
M dependencies View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M include.preload.js View 1 2 3 4 2 chunks +2 lines, -2 lines 0 comments Download
M lib/csp.js View 1 2 3 4 2 chunks +3 lines, -3 lines 0 comments Download
M lib/cssInjection.js View 1 2 3 4 2 chunks +2 lines, -2 lines 0 comments Download
M lib/devtools.js View 1 2 3 4 5 chunks +7 lines, -113 lines 1 comment Download
A lib/hitLogger.js View 1 2 3 4 1 chunk +152 lines, -0 lines 7 comments Download
M lib/popupBlocker.js View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M lib/requestBlocker.js View 1 2 3 4 2 chunks +2 lines, -2 lines 0 comments Download
M lib/whitelisting.js View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M metadata.chrome View 1 2 3 1 chunk +1 line, -0 lines 1 comment Download

Messages

Total messages: 12
kzar
Patch Set 1
2 months ago (2018-02-22 15:25:51 UTC) #1
kzar
Patch Set 2 : Fixed typo and exposed pageId for logged requests
2 months ago (2018-02-22 15:57:29 UTC) #2
kzar
Patch Set 3 : Ensure devtools module is included in the bundle
2 months ago (2018-02-22 16:30:32 UTC) #3
saroyanm
1 month, 1 week ago (2018-03-15 15:30:01 UTC) #4
saroyanm
1 month, 1 week ago (2018-03-15 15:30:36 UTC) #5
Sebastian Noack
https://codereview.adblockplus.org/29705719/diff/29705741/lib/devLogger.js File lib/devLogger.js (left): https://codereview.adblockplus.org/29705719/diff/29705741/lib/devLogger.js#oldcode100 lib/devLogger.js:100: type: "add-record", This abstraction is specific to the devtools ...
1 month ago (2018-03-20 01:28:26 UTC) #6
kzar
Patch Set 4 : Addressed Sebastian's feedback Now depends on this adblockpluscore change https://codereview.adblockplus.org/29729594/ https://codereview.adblockplus.org/29705719/diff/29705741/lib/devLogger.js ...
1 month ago (2018-03-21 15:18:46 UTC) #7
kzar
Patch Set 5 : Nightmarish rebase https://codereview.adblockplus.org/29705719/diff/29751602/lib/hitLogger.js File lib/hitLogger.js (right): https://codereview.adblockplus.org/29705719/diff/29751602/lib/hitLogger.js#newcode81 lib/hitLogger.js:81: let request = ...
1 week, 4 days ago (2018-04-13 15:25:04 UTC) #8
kzar
Any chance you could take a look at this instead Manish? I think Sebastian's a ...
2 days, 5 hours ago (2018-04-23 09:01:59 UTC) #9
Manish Jethani
https://codereview.adblockplus.org/29705719/diff/29751602/lib/hitLogger.js File lib/hitLogger.js (right): https://codereview.adblockplus.org/29705719/diff/29751602/lib/hitLogger.js#newcode34 lib/hitLogger.js:34: let HitLogger = exports.HitLogger = Object.create(new EventEmitter(), desc({ We ...
2 days, 1 hour ago (2018-04-23 12:32:41 UTC) #10
Manish Jethani
https://codereview.adblockplus.org/29705719/diff/29751602/lib/hitLogger.js File lib/hitLogger.js (right): https://codereview.adblockplus.org/29705719/diff/29751602/lib/hitLogger.js#newcode138 lib/hitLogger.js:138: for (let type of nonRequestTypes) On 2018/04/23 12:32:40, Manish ...
2 days ago (2018-04-23 13:32:04 UTC) #11
Manish Jethani
1 day, 22 hours ago (2018-04-23 15:30:59 UTC) #12
https://codereview.adblockplus.org/29705719/diff/29751602/lib/devtools.js
File lib/devtools.js (right):

https://codereview.adblockplus.org/29705719/diff/29751602/lib/devtools.js#new...
lib/devtools.js:261: let hitListener = addRecord.bind(null, panel);
I'm wondering if panel should be an object of a Panel class.

  function Panel(port)
  {
    this.port = port;
    this.records = [];
  }

  Panel.prototype = {
    addRecord(request, filter)
    {
      if (!this.hasRecord(request, filter))
      {
        ...
      }
    },
    hasRecord(request, filter)
    {
      ...
    }
  };

https://codereview.adblockplus.org/29705719/diff/29751602/lib/hitLogger.js
File lib/hitLogger.js (right):

https://codereview.adblockplus.org/29705719/diff/29751602/lib/hitLogger.js#ne...
lib/hitLogger.js:22: const {desc} = require("../adblockpluscore/lib/coreUtils");
I'm assuming the paths here are temporary and will be updated.

https://codereview.adblockplus.org/29705719/diff/29751602/metadata.chrome
File metadata.chrome (right):

https://codereview.adblockplus.org/29705719/diff/29751602/metadata.chrome#new...
metadata.chrome:125: lib/devtools.js
Where is lib/hitLogger.js included?
Sign in to reply to this message.

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