Closed Bug 1057105 Opened 10 years ago Closed 9 years ago

[e10s] "Stylish" add-on does not work with e10s - "Install with Stylish" buttons don't appear on userstyles.org

Categories

(Firefox :: Extension Compatibility, defect)

Product:
Firefox
Component:
Extension Compatibility
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
e10s + ---

People

(Reporter: dholbert, Unassigned)

References

(
URL
)

Details

(Keywords: addon-compat)

Attachments

(1 file)

screenshot (e10s on the left, non-e10s on the right)
157.11 KB, image/png
Details 
STR:
 1. Install stylish from https://addons.mozilla.org/en-US/firefox/addon/stylish/
 2. Toggle about:config pref "browser.tabs.remote.autostart" to true
 3. Restart Firefox
 4. Visit a page for a particular user style at userstyles.org, e.g. this one that I just made:
    https://userstyles.org/styles/104575/squarespace-flexbox-workaround

EXPECTED RESULTS: A green button should appear, saying "+ Install with stylish", to install the user style.

ACTUAL RESULTS: A blue button appears, saying "To use this style, install Stylish". In other words, userstyles.org can't tell that I've got stylish installed. (or stylish can't communicate properly with userstyles.org, or something.)
Version info:
  Ubuntu 14.04
  Firefox Nightly 34.0a1 (2014-08-21)
  (Mozilla/5.0 (X11; Linux x86_64; rv:34.0) Gecko/20100101 Firefox/34.0)
  Stylish version 1.4.3
Here's a screenshot showing e10s on the left, non-e10s on the right.

(These are completely different Firefox instances, each running with their own profile -- not an e10s-window/non-e10s-window, FWIW.)
hi Jason, if you have any questions about add-on support for multiprocess Firefox (e10s), just drop by the #e10s IRC channel on irc.mozilla.org. MDN also has a good introduction:

https://developer.mozilla.org/en-US/Add-ons/Working_with_multiprocess_Firefox
Two questions jump out at me after reading that and https://developer.mozilla.org/en-US/docs/The_message_manager:

1. "Frame scripts run in the content process and get direct access to content. Frame scripts communicate with the rest of the extension using a message-passing API." Do frame scripts have access to XPCOM services, like looking up prefs or string bundles? This seems imply they don't, but doesn't explicitly state it.
2. "Note that none of this requires multi-process Firefox: everything described here will work with single-process Firefox, so the same code will work in both variants." How far back is this stuff expected to work?
Ally, can you answer Jason's questions about backwards compatibility and which XPCOM services are CPOW'd or shimmed? 

(In reply to Jason Barnabe (np) from comment #4)
> Two questions jump out at me after reading that and
> https://developer.mozilla.org/en-US/docs/The_message_manager:
> 
> 1. "Frame scripts run in the content process and get direct access to
> content. Frame scripts communicate with the rest of the extension using a
> message-passing API." Do frame scripts have access to XPCOM services, like
> looking up prefs or string bundles? This seems imply they don't, but doesn't
> explicitly state it.

I think you can read prefs from frame scripts in the content process, but most I/O must happen in the parent process. Your frame scripts may need to send messages to the parent process to perform that I/O and return the result. For convenience, the frame scripts can send synchronous messages, blocking the content process but not the parent process (browser UI).

> 2. "Note that none of this requires multi-process Firefox: everything
> described here will work with single-process Firefox, so the same code will
> work in both variants." How far back is this stuff expected to work?

The message manager has been part of Firefox since version 4.0, I think. :) If you find any differences between the multi-process and single process modes in any of current release channels (currently Firefox 32 – Nightly 35), please file a bug. We will try to maintain compatibility with older versions of Firefox, but we only fix bugs in the current release channels (plus ESR).
Flags: needinfo?(ally)
0) I think it would be really handy to have a list of shimmed functions and cpowed objects for reference. Chris, can you talk to Will about that possibility?

1) I don't believe we've written any special code to allow content to see string bundles. It's not explicitly stated because we allow it for pref reading, but please consider it true elsewhere.

2) What Chris said.

Please feel free to hop in the #e10s channel and ask more questions. The entire e10s team hangs out there.
Flags: needinfo?(ally)
mrbkap believes that string bundles might be available via the magic of cpows, but we're not sure because I can't find the code to cross process that. So give it a shot I guess?
#1 was really more of a general question about chrome privileges; I just used prefs and string bundles as simple examples. I'm going to assume that generally, the frame script can't do much more than what content can, other than pass messages.
I've added e10s support in https://github.com/JasonBarnabe/stylish/issues/189. I'll need to do some extensive testing before this gets released.

(In reply to Chris Peterson (:cpeterson) from comment #5)
> The message manager has been part of Firefox since version 4.0, I think. :)
> If you find any differences between the multi-process and single process
> modes in any of current release channels (currently Firefox 32 – Nightly
> 35), please file a bug. We will try to maintain compatibility with older
> versions of Firefox, but we only fix bugs in the current release channels
> (plus ESR).

Stylish previously supported back to Firefox 4. With these changes, Stylish only goes back to Firefox 19.

Firefox 4-16 have a different API (changed in bug 776825):

var globalMM = Components.classes["@mozilla.org/globalmessagemanager;1"].getService(Components.interfaces.nsIMessageListenerManager);

Error: [Exception... "Component returned failure code: 0x80570018 (NS_ERROR_XPC_BAD_IID) [nsIJSCID.getService]"  nsresult: "0x80570018 (NS_ERROR_XPC_BAD_IID)"  location: "JS frame :: file:///home/jason/Extensions/stylish-firefox/components/stylishStartup.js :: wireUpMessaging :: line 367"  data: no]
Source File: file:///home/jason/Extensions/stylish-firefox/components/stylishStartup.js
Line: 428


Firefox 17-18 fails on code like new content.document.defaultView.XMLHttpRequest() with NS_ERROR_FAILURE: Failure.

I've documented these imcompatibilities at https://developer.mozilla.org/en-US/Add-ons/Working_with_multiprocess_Firefox#Backwards_compatibility_of_the_new_APIs
Jason: thanks for tracking down and documenting these problem.

Bill or Ally: can we polyfill the old Message Manager interface (Firefox 16 bug 776825) to improve backwards compatibility?
Flags: needinfo?(wmccloskey)
We could polyfill, but the purpose of bug 776825 was to remove the calls that we would be adding back. See https://bugzilla.mozilla.org/show_bug.cgi?id=776825#c7. I actually think Chris was maybe being a little alarmist in that comment, but I'd still rather wait to see if other add-ons run into problems here. Firefox 16 was a long time ago.
Flags: needinfo?(wmccloskey)
I'm fine with not supporting Firefox 4-18. Even if you polyfilled, I probably wouldn't use it.
Would you mind helping test the addon code mentioned in comment 9?
Flags: needinfo?(jbecerra)
I'm making a number of other changes for the next version, and they're not complete yet. When they are, I'll post on forum.userstyles.org where I expect people will test (though perhaps not with e10s enabled). So what I'm saying is, don't bother testing, or at least wait until it reaches beta.
Stylish 2.0.0b1 is available at https://forum.userstyles.org/discussion/44032/stylish-for-firefox-2-0-0b1, with e10s support.
Awesome. Thanks, Jason!

We should keep this bug open until Stylish 2.0.0 is on AMO.
Flags: needinfo?(jbecerra)
Jason,
Telemetry is reporting that stylish is regularly throwing exceptions at overlay.js 131 in version 2.0.0 that you appear to have up on AMO. Is this expected?
Flags: needinfo?(jason.barnabe)
I know it's not working on Android right now (though all I see when debugging is "uncaught exception: 2147746132"), though on Linux things seem fine. Do you have info on the platforms seeing the error?
Flags: needinfo?(jason.barnabe) → needinfo?(ally)
It's desktop only, so osx, linux, windows. 

Exact platforms are not reported, nor is the error itself due to privacy restrictions on finger printing.

Given the population makeup and that it works on linux, I'd guess that windows might be the offending os.
Flags: needinfo?(ally)
I can confirm locally (on both Linux and Windows) the following:
 (1) the extension does seem to be working, in general (the green "install with stylish" button shows up)
...but *also*:
 (2) the extension is throwing exceptions at overlay.js lines 116 and 131, on pageload, even for simply loading about:blank.

Jason, I'd expect you should be able to see these exceptions in the error console (ctrl+shift+j) -- I'm getting them in a fresh profile (with e10s enabled and Stylish installed), on every pageload.

The exception is "unsafe CPOW usage", and the lines of code are:
> 116    if (uri && uri.spec == content.document.location.href) {
and
> 131    if (stylishOverlay.lastUrl == content.document.location.href)
(The "unsafe CPOW usage" thing is classified as a "JS Warning" in the error console; I'm actually not sure if it's an exception or not, but I'm assuming it's related to what ally mentioned in comment 18, given the matching line-number.)
If I load about:home, I get the same warning ("unsafe CPOW usage") for more lines as well. The full list of flagged lines is:
 line 116
 line 131
 line 133
 line 134
 line 136
 line 174
...all for Stylish's file overlay.js.
(These warnings might merit their own followup bug, since the original issue here seems to be fixed. I'm just mentioning them here in response to comment 18 / comment 19.)
(In reply to Jason Barnabe (np) from comment #19)
> I know it's not working on Android right now (though all I see when
> debugging is "uncaught exception: 2147746132"), though on Linux things seem
> fine. Do you have info on the platforms seeing the error?

I see this on Android with a lightly modified c5cd6994ee41.
OK, I see the exceptions and warnings. I had assumed that I could see them on the latest release if I flipped the e10s pref.

The exceptions are actually unrelated to the original issue reported. The original issue has been fixed but not yet released.

Most of the "unsafe CPOW usage" warnings is Stylish trying to look at various properties of the current page, like content.location.href. What is the "safe" way to do this?
This blog post about the warnings should probably be linked here: http://blog.lassey.us/2015/01/10/unsafe-cpow-usage/

I was still a bit confused after reading it what the *safe* thing to do is, but the bug that added the warning (bug 1097998) gives a good description (quoting billm):

1. Content process gets an event and sends a sync message to chrome.
2. Chrome process sends down a bunch of CPOW messages querying the DOM.
3. Chrome process finishes handling the sync message and content process is allowed to continue.

IIUC it would be better to avoid CPOWs altogether though. I think you can do this by passing messages instead, but I'm not clear on the details.
I was hoping the whole messages thing could be bypassed for something as simple as "what's the current URL in this tab", but if not, I guess I'll revise...
Jason: do you know of any remaining e10s problems with Stylish 2.0.1? Or can we close this bug as fixed?
Flags: needinfo?(jason.barnabe)
I don't know of any problems. Then again, I didn't know of any problems when I released 2.0.0 either.
Flags: needinfo?(jason.barnabe)
Thanks, Jason!
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: