Code review for 18490b9bf8

- Use a time offset to ensure value fit in a int32
- Also, fix potential false positives re. blocked image placeholders

src/js/contentscript.js

@@ -166,21 +166,20 @@ var collapser = (function() {
             return;
         }
 
-        var placeholders = response.placeholders,
+        let placeholders = response.placeholders;
-            tag, prop, src, collapsed, docurl, replaced;
 
-        for ( var target of targets ) {
+        for ( let target of targets ) {
-            tag = target.localName;
+            let tag = target.localName;
-            prop = src1stProps[tag];
+            let prop = src1stProps[tag];
             if ( prop === undefined ) { continue; }
-            src = target[prop];
+            let src = target[prop];
             if ( typeof src !== 'string' || src.length === 0 ) {
                 prop = src2ndProps[tag];
                 if ( prop === undefined ) { continue; }
                 src = target[prop];
                 if ( typeof src !== 'string' || src.length === 0 ) { continue; }
             }
-            collapsed = cachedBlockedMap.get(tagToTypeMap[tag] + ' ' + src);
+            let collapsed = cachedBlockedMap.get(tagToTypeMap[tag] + ' ' + src);
             if ( collapsed === undefined ) { continue; }
             if ( collapsed ) {
                 target.style.setProperty('display', 'none', 'important');
@@ -190,7 +189,7 @@ var collapser = (function() {
             switch ( tag ) {
             case 'iframe':
                 if ( placeholders.frame !== true ) { break; }
-                docurl =
+                let docurl =
                     'data:text/html,' +
                     encodeURIComponent(
                         placeholders.frameDocument.replace(
@@ -198,7 +197,7 @@ var collapser = (function() {
                             src
                         )
                     );
-                replaced = false;
+                let replaced = false;
                 // Using contentWindow.location prevent tainting browser
                 // history -- i.e. breaking back button (seen on Chromium).
                 if ( target.contentWindow ) {
@@ -214,6 +213,16 @@ var collapser = (function() {
                 break;
             case 'img':
                 if ( placeholders.image !== true ) { break; }
+                // Do not insert placeholder if the image was actually loaded.
+                // This can happen if an allow rule was created while the
+                // document was loading.
+                if (
+                    target.complete &&
+                    target.naturalWidth !== 0 &&
+                    target.naturalHeight !== 0
+                ) {
+                    break;
+                }
                 target.style.setProperty('display', 'inline-block');
                 target.style.setProperty('min-width', '20px', 'important');
                 target.style.setProperty('min-height', '20px', 'important');

src/js/pagestats.js

@@ -36,24 +36,25 @@ var BlockedCollapsibles = function() {
     this.blocked = new Map();
     this.hash = 0;
     this.timer = null;
+    this.tOrigin = Date.now();
 };
 
 BlockedCollapsibles.prototype = {
 
-    shelfLife: 10,
+    shelfLife: 10 * 1000,
 
     add: function(type, url, isSpecific) {
         if ( this.blocked.size === 0 ) { this.pruneAsync(); }
-        let now = Date.now() / 1000 | 0;
+        let tStamp = Date.now() - this.tOrigin;
         // The following "trick" is to encode the specifity into the lsb of the
         // time stamp so as to avoid to have to allocate a memory structure to
         // store both time stamp and specificity.
         if ( isSpecific ) {
-            now |= 0x00000001;
+            tStamp |= 0x00000001;
         } else {
-            now &= 0xFFFFFFFE;
+            tStamp &= 0xFFFFFFFE;
         }
-        this.blocked.set(type + ' ' + url, now);
+        this.blocked.set(type + ' ' + url, tStamp);
         this.hash += 1;
     },
 
@@ -64,26 +65,31 @@ BlockedCollapsibles.prototype = {
             clearTimeout(this.timer);
             this.timer = null;
         }
+        this.tOrigin = Date.now();
     },
 
     pruneAsync: function() {
         if ( this.timer === null ) {
             this.timer = vAPI.setTimeout(
                 this.boundPruneAsyncCallback,
-                this.shelfLife * 2000
+                this.shelfLife * 2
             );
         }
     },
 
     pruneAsyncCallback: function() {
         this.timer = null;
-        let obsolete = Math.ceil(Date.now() / 1000) - this.shelfLife;
+        let tObsolete = Date.now() - this.tOrigin - this.shelfLife;
         for ( let entry of this.blocked ) {
-            if ( entry[1] <= obsolete ) {
+            if ( entry[1] <= tObsolete ) {
                 this.blocked.delete(entry[0]);
             }
         }
-        if ( this.blocked.size !== 0 ) { this.pruneAsync(); }
+        if ( this.blocked.size !== 0 ) {
+            this.pruneAsync();
+        } else {
+            this.tOrigin = Date.now();
+        }
     }
 };