Issue 3792 - Fix to support multiprocess firefox

lib/child/frameScript.js

+/*
+ * This Source Code is subject to the terms of the Mozilla Public License
+ * version 2.0 (the "License"). You can obtain a copy of the License at
+ * http://mozilla.org/MPL/2.0/.
+ */
+
+'use strict';

[Wladimir Palant, 2016/09/29 11:44:58]

Double quotation marks please.

[sergei, 2016/09/29 15:36:22]

Done.

+
+const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
+
+/**
+ * @param e exception
+ */
+function reportException(e)
+{
+  let stack = "";
+  if (e && typeof e == "object" && "stack" in e)
+    stack = e.stack + "\n";
+
+  Cu.reportError(e);
+  dump(e + "\n" + stack + "\n");
+}
+
+const {XPCOMUtils} = Cu.import("resource://gre/modules/XPCOMUtils.jsm", {});
+
+/**
+ * Progress listener capturing the data of the current page and calling
+ * onPageLoaded(data) when loading is finished, where data contains
+ * HTTP status and headers.
+ *
+ * @type nsIWebProgressListener
+ */
+const webProgressListener =
+{
+  onStateChange: function(webProgress, request, flags, status)
+  {
+    if (webProgress.DOMWindow == content &&
+        (flags & Ci.nsIWebProgressListener.STATE_STOP) &&
+        (flags & Ci.nsIWebProgressListener.STATE_IS_WINDOW))

[Wladimir Palant, 2016/09/29 11:44:59]

You don't have to check STATE_IS_WINDOW any more.

[sergei, 2016/09/29 15:36:21]

Done. I left it just in case.

+    {
+      if (content.location.protocol == 'about:')
+        return;

[Wladimir Palant, 2016/09/29 11:44:58]

This needs an explanation. Is that check necessary because you load about:blank
into unused tabs and you don't want to report that load? At the very least, this
needs a comment saying so.

Still, what issue does this check solve? My guess is that without this check the
code will collect data from about:blank, send a message and this message will be
ignored. Wasting a few CPU cycles, so what? Do we really need this special case?
Or maybe we should rather work correctly even if we get something like
about:newtab on our URL list?

[sergei, 2016/09/29 15:36:22]

Answer is in
https://codereview.adblockplus.org/29338242/diff/29340782/lib/child/frameScri...

+      const pageInfo = {channelStatus: status};

[Wladimir Palant, 2016/09/29 11:44:58]

Please don't declare regular variables as constants. Just because you don't
assign to pageInfo itself but only its properties this doesn't make it a
constant.

[sergei, 2016/09/29 15:36:21]

const in javascript does not mean immutable, it's useful to find unexpected
assignments. Do we use it as indicator of immutability?

I have changed to "let" most of the declarations.

[Wladimir Palant, 2016/09/30 07:43:12]

Yes, that's how const is normally used - to indicate values that are inherently
immutable, not merely unchanged due to current implementation details.

+      if (request instanceof Ci.nsIHttpChannel)
+      {
+        try
+        {
+          pageInfo.headers = [];
+          pageInfo.headers.push("HTTP/x.x " + request.responseStatus + " " + request.responseStatusText);
+          request.visitResponseHeaders((header, value) => pageInfo.headers.push(header + ": " + value));
+        }
+        catch (e)
+        {
+          reportException(e);
+        }
+      }
+      onPageLoaded(pageInfo);
+    }
+  },
+
+  onLocationChange: function() {},
+  onProgressChange: function() {},
+  onStatusChange: function() {},
+  onSecurityChange: function() {},
+
+  QueryInterface: XPCOMUtils.generateQI([Ci.nsIWebProgressListener, Ci.nsISupportsWeakReference])
+};
+
+function onPageLoaded(pageInfo)
+{
+  Object.assign(pageInfo, gatherPageInfo(content));
+  sendAsyncMessage("abpcrawler:pageInfoGathered", pageInfo);
+};
+
+const webProgress = docShell.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIWebProgress);

[Wladimir Palant, 2016/09/29 11:44:58]

As above, please don't mark any variable that you happen to only assign once (in
this revision of the code at least, but it might change later) as a constant.
Please get rid of the pseudo-constants below as well.

[sergei, 2016/09/29 15:36:21]

Done.

+webProgress.addProgressListener(webProgressListener, Ci.nsIWebProgress.NOTIFY_STATE_WINDOW);
+
+/**
+ * Gathers information about a DOM window.
+ * Currently
+ *  - creates a screenshot of the page
+ *  - serializes the page source code
+ * @param {nsIDOMWindow} wnd window to process
+ * @return {Object} the object containing "screenshot" and "source" properties.
+ */
+function gatherPageInfo(wnd)
+{
+  const document = wnd.document;
+  const result = {errors:[]};
+  if (!document.documentElement)
+  {
+    result.errors.push('No document.documentElement');

[Wladimir Palant, 2016/09/29 11:44:58]

Double quotation marks please.

[sergei, 2016/09/29 15:36:21]

Done.

+    return result;
+  }
+
+  try
+  {
+    const canvas = document.createElementNS("http://www.w3.org/1999/xhtml", "canvas");
+    canvas.width = document.documentElement.scrollWidth;
+    canvas.height = document.documentElement.scrollHeight;
+    const context = canvas.getContext("2d");
+    context.drawWindow(wnd, 0, 0, canvas.width, canvas.height, "rgb(255, 255, 255)");
+    result.screenshot = canvas.toDataURL("image/jpeg", 0.8);
+  }
+  catch (e)
+  {
+    reportException(e);
+    result.errors.push("Cannot make page screenshot");
+  }
+
+  try
+  {
+    // TODO: Capture frames as well?
+    const serializer = new wnd.XMLSerializer();
+    result.source = serializer.serializeToString(document.documentElement);
+  }
+  catch(e)
+  {
+    reportException(e);
+    result.errors.push("Cannot obtain page source code");
+  }
+
+  return result;
+}