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

Issue 29384812: Issue 4127 - [emscripten] Convert subscription classes to C++ - Part 1 (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 10 months ago by Wladimir Palant
Modified:
2 years, 9 months ago
Reviewers:
hub, sergei
CC:
Felix Dahlke
Base URL:
https://hg.adblockplus.org/adblockpluscore
Visibility:
Public.

Description

This implements the subscriptions but doesn`t connect them to filters yet. Adding filters to subscriptions and everything related will be implemented in the next part.

Patch Set 1 #

Patch Set 2 : Fixed method names according to convention #

Patch Set 3 : Added missing license headers #

Patch Set 4 : Replace subscriptionClasses.js properly instead of creating a new file #

Total comments: 2

Patch Set 5 : Safe buffer manipulation #

Patch Set 6 : Switched to our own number conversion #

Patch Set 7 : Use uint64_t for properties #

Total comments: 3

Patch Set 8 : Removed unnecessary raw buffer #

Total comments: 29

Patch Set 9 : Addressed comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+764 lines, -635 lines) Patch
M compiled/String.h View 1 2 3 4 5 6 2 chunks +13 lines, -1 line 0 comments Download
M compiled/StringMap.h View 1 2 3 4 5 6 7 8 1 chunk +5 lines, -0 lines 0 comments Download
M compiled/bindings.cpp View 1 2 2 chunks +35 lines, -0 lines 0 comments Download
M compiled/bindings.ipp View 1 2 3 4 5 6 7 8 7 chunks +136 lines, -78 lines 0 comments Download
M compiled/filter/Filter.h View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
A compiled/subscription/DownloadableSubscription.h View 1 2 3 4 5 6 1 chunk +41 lines, -0 lines 0 comments Download
A compiled/subscription/DownloadableSubscription.cpp View 1 2 3 4 5 6 1 chunk +100 lines, -0 lines 0 comments Download
A compiled/subscription/Subscription.h View 1 2 3 4 5 6 7 8 1 chunk +116 lines, -0 lines 0 comments Download
A compiled/subscription/Subscription.cpp View 1 2 3 4 5 6 7 8 1 chunk +105 lines, -0 lines 0 comments Download
A compiled/subscription/UserDefinedSubscription.h View 1 2 3 4 5 6 7 8 1 chunk +33 lines, -0 lines 0 comments Download
A compiled/subscription/UserDefinedSubscription.cpp View 1 2 3 4 5 6 7 8 1 chunk +88 lines, -0 lines 0 comments Download
M lib/subscriptionClasses.js View 1 2 3 1 chunk +4 lines, -533 lines 0 comments Download
M test/subscriptionClasses.js View 1 2 3 4 5 6 7 8 1 chunk +87 lines, -23 lines 0 comments Download

Messages

Total messages: 16
Wladimir Palant
2 years, 10 months ago (2017-03-15 15:05:20 UTC) #1
Wladimir Palant
Added Hubert to CC.
2 years, 9 months ago (2017-03-30 17:40:55 UTC) #2
hub
https://codereview.adblockplus.org/29384812/diff/29395601/compiled/subscription/DownloadableSubscription.cpp File compiled/subscription/DownloadableSubscription.cpp (right): https://codereview.adblockplus.org/29384812/diff/29395601/compiled/subscription/DownloadableSubscription.cpp#newcode43 compiled/subscription/DownloadableSubscription.cpp:43: int len = sprintf(buffer, "%.0f", mLastCheck); I think we ...
2 years, 9 months ago (2017-03-31 03:17:21 UTC) #3
Wladimir Palant
https://codereview.adblockplus.org/29384812/diff/29395601/compiled/subscription/DownloadableSubscription.cpp File compiled/subscription/DownloadableSubscription.cpp (right): https://codereview.adblockplus.org/29384812/diff/29395601/compiled/subscription/DownloadableSubscription.cpp#newcode43 compiled/subscription/DownloadableSubscription.cpp:43: int len = sprintf(buffer, "%.0f", mLastCheck); On 2017/03/31 03:17:21, ...
2 years, 9 months ago (2017-04-04 14:45:37 UTC) #4
Wladimir Palant
Patch set 6 is rebased to apply on top of https://codereview.adblockplus.org/29404594/ so that we can ...
2 years, 9 months ago (2017-04-06 15:28:24 UTC) #5
Wladimir Palant
With Patch set 7 we have proper 64-bit properties now, the conversion to double only ...
2 years, 9 months ago (2017-04-07 07:14:58 UTC) #6
hub
https://codereview.adblockplus.org/29384812/diff/29405572/compiled/bindings.ipp File compiled/bindings.ipp (right): https://codereview.adblockplus.org/29384812/diff/29405572/compiled/bindings.ipp#newcode193 compiled/bindings.ipp:193: signature[pos] = 0; Nowhere we check that pos doesn't ...
2 years, 9 months ago (2017-04-10 15:30:09 UTC) #7
Wladimir Palant
Uploaded Patch set 8 https://codereview.adblockplus.org/29384812/diff/29405572/compiled/bindings.ipp File compiled/bindings.ipp (right): https://codereview.adblockplus.org/29384812/diff/29405572/compiled/bindings.ipp#newcode193 compiled/bindings.ipp:193: signature[pos] = 0; On 2017/04/10 ...
2 years, 9 months ago (2017-04-10 18:27:49 UTC) #8
hub
LGTM
2 years, 9 months ago (2017-04-11 05:40:18 UTC) #9
sergei
https://codereview.adblockplus.org/29384812/diff/29408763/compiled/String.h File compiled/String.h (right): https://codereview.adblockplus.org/29384812/diff/29408763/compiled/String.h#newcode357 compiled/String.h:357: assert(source, u"Null buffer passed to OwnedString.append()"_str); I think it ...
2 years, 9 months ago (2017-04-12 12:04:36 UTC) #10
hub
https://codereview.adblockplus.org/29384812/diff/29408763/compiled/String.h File compiled/String.h (right): https://codereview.adblockplus.org/29384812/diff/29408763/compiled/String.h#newcode357 compiled/String.h:357: assert(source, u"Null buffer passed to OwnedString.append()"_str); On 2017/04/12 12:04:31, ...
2 years, 9 months ago (2017-04-12 12:12:08 UTC) #11
sergei
Forgot to mention that I start to concern a bit more about global variables (of ...
2 years, 9 months ago (2017-04-12 12:13:17 UTC) #12
sergei
https://codereview.adblockplus.org/29384812/diff/29408763/compiled/String.h File compiled/String.h (right): https://codereview.adblockplus.org/29384812/diff/29408763/compiled/String.h#newcode357 compiled/String.h:357: assert(source, u"Null buffer passed to OwnedString.append()"_str); On 2017/04/12 12:12:07, ...
2 years, 9 months ago (2017-04-12 12:14:40 UTC) #13
Wladimir Palant
https://codereview.adblockplus.org/29384812/diff/29408763/compiled/StringMap.h File compiled/StringMap.h (right): https://codereview.adblockplus.org/29384812/diff/29408763/compiled/StringMap.h#newcode236 compiled/StringMap.h:236: size_type max_size() const On 2017/04/12 12:04:31, sergei wrote: > ...
2 years, 9 months ago (2017-04-13 13:04:44 UTC) #14
sergei
LGTM
2 years, 9 months ago (2017-04-13 13:34:46 UTC) #15
hub
2 years, 9 months ago (2017-04-13 15:48:28 UTC) #16
Message was sent while issue was closed.
LGTM
Sign in to reply to this message.

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