Issue 5090 - Use user stylesheets for element hiding if possible

lib/elemHideHelper.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2017 eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.

[Sebastian Noack, 2017/05/24 08:31:41]

This file is new. So how does it come that there are lines in this patch that
are not added, but already seem to exist? It looks like this patch has been
created on top of a previous iteration of this review. Please always upload
combined patches with all changes squashed. Otherwise, it's quite difficult to
review all changes together.

[Manish Jethani, 2017/05/24 17:13:34]

This is probably upload.py messing up. I'm trying hg review now.

[Sebastian Noack, 2017/05/24 18:14:05]

Well, that is what usually happens if you commit your changes for a new patch on
top, but don't specify a revision range to upload.py that includes all changes
related to this review. Are you sure, that's not what happened?

[Manish Jethani, 2017/05/25 02:52:25]

That may have been the case, I get your point. hg review has been working well
for me so far.

  */
 
+/** @module elemHideHelper */
+
 "use strict";
 
 const {RegExpFilter} = require("filterClasses");
 const {ElemHide} = require("elemHide");
 const {checkWhitelisted} = require("whitelisting");
 const {extractHostFromFrame} = require("url");
 const {port} = require("messaging");
 const devtools = require("devtools");
 
+let userStylesheetsSupported = true;
+
+function hideElements(tabId, frameId, selectors, callback)

[Sebastian Noack, 2017/05/24 08:31:41]

Perhaps we should make this function return a promise rather than taking a
callback, in particular considering that the calling code wraps it into a
proomise anyway?

[Manish Jethani, 2017/05/24 17:13:34]

Done.

+{
+  let code = selectors.join(", ") + "{display: none !important;}";
+
+  try
+  {
+    chrome.tabs.insertCSS(tabId,
+      {
+        code,
+        cssOrigin: "user",
+        frameId,
+        matchAboutBlank: true
+      },
+      () =>
+      {
+        callback(chrome.runtime.lastError);
+      }
+    );
+  }
+  catch (error)
+  {
+    if (/\bError processing cssOrigin\b/.test(error.message) != -1)
+      userStylesheetsSupported = false;
+
+    callback(error);

[Manish Jethani, 2017/05/18 02:25:23]

Doing this for any errors instead of just the one for unsupported cssOrigin. We
could also throw here if it's not an error we're interested in.

[Sebastian Noack, 2017/05/24 08:31:41]

Yeah, we definitely should make sure that any unexpected error will be noticed
during development and testing.

[Manish Jethani, 2017/05/24 17:13:34]

Done.

+  }
+}
+
 port.on("get-selectors", (msg, sender) =>
 {
   let selectors;
   let trace = devtools && devtools.hasPanel(sender.page);
 
   if (!checkWhitelisted(sender.page, sender.frame,
                         RegExpFilter.typeMap.DOCUMENT |
                         RegExpFilter.typeMap.ELEMHIDE))
   {
     let specificOnly = checkWhitelisted(sender.page, sender.frame,
                                         RegExpFilter.typeMap.GENERICHIDE);
     selectors = ElemHide.getSelectorsForDomain(
       extractHostFromFrame(sender.frame),
       specificOnly ? ElemHide.SPECIFIC_ONLY : ElemHide.ALL_MATCHING
     );
   }
   else
   {
     selectors = [];
   }
 
-  return {selectors, trace};
+  if (!userStylesheetsSupported)
+    return {selectors, trace, inject: true};
+
+  if (selectors.length == 0)
+  {
+    if (trace)
+      return {selectors, trace, inject: false};
+
+    return {trace, inject: false};
+  }
+
+  return new Promise(resolve =>
+  {
+    hideElements(sender.page.id, sender.frame.id, selectors, error =>
+    {
+      let response = {trace, inject: !!error};
+
+      if (trace || error)
+        response.selectors = selectors;
+
+      resolve(response);
+    });
+  });
 });
 
-port.on("forward", (msg, sender) =>
+port.on("hide-elements", (msg, sender) =>
 {
-  let targetPage;
-  if (msg.targetPageId)
-    targetPage = ext.getPage(msg.targetPageId);
-  else
-    targetPage = sender.page;
+  if (!msg.selectors || msg.selectors.length == 0)
+    return {success: true};
 
-  if (targetPage)
+  return new Promise(resolve =>
   {
-    msg.payload.sender = sender.page.id;
-    if (msg.expectsResponse)
-      return new Promise(targetPage.sendMessage.bind(targetPage, msg.payload));
-    targetPage.sendMessage(msg.payload);
-  }
+    hideElements(sender.page.id, sender.frame.id, msg.selectors, error =>
+    {
+      resolve({success: !error});
+    });
+  });
 });