693555 - Update inline spell checker underlining for multiprocess

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect, P1)

Comment 1

[Asaf Romano (gone)]

Attachment: wip

This is almost done. A couple of things left:
0. Actually implement takeOwnership.
1. selectDictionary support for e10s case
2. figure out why my clear methods are buggy
3. documentation for InlineSpellCheckerMenu constructor arguments.

I'd like to land this apart from the context menu patch, so I'm including here minimal browser/ changes just to keep the current setup working (In other words, without the context menu patch, this treats the content-chrome case as chrome-chrome).

Comment 2

[Asaf Romano (gone)]

Comment on attachment 566175 [details] [diff] [review]
wip

not yet!

Comment 3

[Asaf Romano (gone)]

Attachment: a patch

With browser changes applied (which are not part of this patch), this works nicely (except "Add to dictionary", which I'll get to soon).  However the spellchecker api and usage are somewhat a mess - I've *avoided* fixing that so far and tried to keep the api changes minimal. I think I was wrong on that.

Here's the way it was used so far.

There were two couples of menu methods:
addSuggestionsToMenu(menu, insertbefoe) addDictionaryListToMenu(menu, insertbefore)
clearSuggestionsFromMenu() clearDictionaryListFromMenu()

The first couple is called, together, on popupshowing. The second couple was _supposed_ to be called on popuphidden. For chrome textboxes, they're indeed called this way. For content textboxes, there's a hack that calls them onpoupshowing because popuphidden wasn't always dispatched.  Popups were written at least once since then, so i think we should clean that up.

As you can see, the add* methods always take the menu, and the clear* methods do not. This means that any spell checker object could switch between one menu to another. When an add* method was called, it cached the menu it needs to cleanup for the clean method.

I suggest the following changes (they're not part of this patch, but they're easy to do):
1. The lifetime of the spell checker "front end" objects is from popupshowing to popuphidden - once the menu is closed, the objects are gone.
2. No add* and clear* methods. The constructor will populate the menu, and on-popuphidden we will call uninit.

Gavin, I'm leaving the decision for you. The current setup works for me (as in, with my fixes for context menu and this patch, spell checker works in all modes). It just isn't nice.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 566487 [details] [diff] [review]
a patch

I'm having a really hard time following this logic. Can you lay out exactly how the e10s mode works, in terms of how data gets passed between which objects, and when (i.e. triggered by which events)?

It seems like this could benefit from being simplified further, with the content-chrome interaction abstracted in a cleaner way. Making the chrome-only code work on top of a simplified chrome<->content system seems like it should be simple and easier to follow. Am I crazy talking?

Comment 5

[Asaf Romano (gone)]

Sadly

Comment 6

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

I think the general architecture for this bug will have to be as follows. The actual checking of words probably needs to happen in the parent. That's just because we don't want to have a copy of all the dictionary data structures in every child process. On my system, the US dictionary is a 610K text file. Expanded into whatever data structure we use, it's probably even bigger.

However, there's a lot of code that needs to run in the child to actually decide what ranges of text to check as the user types. When it actually needs to check a word, this code could send a message to the parent. A lot of this is already somewhat asynchronous, so I'm hoping we should be able to get away with async messaging here without too much trouble.

The final piece is the UI code that adds suggestions to the context menu and such. That obviously needs to run in the parent. It looks like the patch that's already in this bug addresses that aspect, although I haven't looked at it too closely.

I think it would make sense to split this into two pieces. One piece would cover the C++ side, where we need to break the actual checking of words apart from deciding what text ranges to check. The other piece would cover the UI aspect.

Comment 7

[Allison Naaktgeboren :ally]

Gavin, since you gave the initial feedback, does Bill's proposal in comment 6 feel like a cleaner abstraction to you? What do you think of the idea and should we execute on that?

Comment 8

[:Gavin Sharp [email: [e-mail]]]

That all sounds quite reasonable. I don't recall enough of the details here to provide any more detailed feedback without digging into it further.

Comment 9

[Allison Naaktgeboren :ally]

despite how broken this appears to be, browser_InlineSpellChecker.js passes all tests.

Comment 10

[Allison Naaktgeboren :ally]

Tweaking
 - hardcoding dictionary path did not produce wavy lines or a non null
    range in Inlinespellchecker.jsm
      - though the call to getDictionaryList does return en-us,
        getCurrentDictionary returns an empty string

Current Plan of Attack
Part 1: Underlying engine interface
  A) Create a new class that implements mozISpellCheckingEngine
    - In its methods use proper message passing where needed
      - much of it will be forwarding to the parent
    - Will probably have to block child in several cases, 
      - api is designed sync
  B) When remote, point the consumers at new "engine" class
  C) Possibly Modify mozISpellCheckingEngine
    - might need a getCurrentlySelectedDictionary() function
  D) trying to install a new dictionary from the context menu fails with aDocShell is null
    - browser.js ln 175
    - might end up a followup bug
  Notes:
    - 99% sure that squiggly lines will work after this
    - should pick up personal dictionary without further work
    - concerns about slow down here from ipc burden
Part 2: UI bits, context menu & InlineSpellchecker.jsm 
  A) Change nsIInlineSpellchecker.getMisspelledWord() to return a string
literal instead of a dom range
  B) Modify InlineSpellchecker.jsm to access the list of dictionaries
directly
    - see notes
  C) Modifiy InlineSpellchecker.jsm to directly invoke mozISpellcheckerEngine 
    - replace check calls
    - replace suggest calls
    - test other calls; should function correctly with cpows as is (~5)
  Notes:
  - extends current spellcheck UI patch (gets rangeparent & offset correctly)
  - (B) will be a slight functional change
      - seems only one finnish spellchecker addon will be affected
      - ehsan is ok breaking it, but I'd like to see about trying to 
          preserve the theoretical goal of multiple spelling backends

Comment 11

[(no longer active)]

This looks good to me.  Please note that supporting the existing setup of multiple backends is not particularly difficult, it's just a bit more work.  But if you're willing to dig into that, that's awesome! :-)

Comment 13

[Allison Naaktgeboren :ally]

Attachment: part 1: wip remote spellchecking engine ipdl

wip, the skeleton of the ipdl.

Comment 14

[Allison Naaktgeboren :ally]

Attachment: part 2 wip spellchecker_UI (will depend on part1. range/list of words is 0)

Comment 15

[Allison Naaktgeboren :ally]

Attachment: part1 : messy but functional ipdl (child asks for language, parent sends Klingnon)

- verified successful ipdl message exchange
-- load objdir/msvc/mozilla.sln in VS
-- start firefox.exe (with patch applied)
-- In VS set breakpoint on mozHunspell::GetLanguage()
-- attach VS debugger to PluginContainer.exe (Debug -> Attach to Process..)
-- step through. parent is hardcoded to return 'Klingon' as the language

- lots of debug/stray code
-- Using winutils caused a rash of strange compiler errors, http://pastebin.mozilla.org/5324959

Comment 19

[Allison Naaktgeboren :ally]

Attachment: wip, works mostly (see comment)

Spellchecker can tell the difference between correct & incorrect words! \o/

Seems like a good time to checkpoint on the general behavior of the code. Bill, Blake, Brad, Felipe, feel free to weigh in as well.

Remaining work
  1 RemoteSpellcheckEngineParent: getting the dictionary in the parent
      - does this work when other dictionaries are the default?
      - triggers two asserts
        - http://mxr.mozilla.org/mozilla-central/source/extensions/spellcheck/src/mozInlineSpellChecker.cpp#820
        - http://mxr.mozilla.org/mozilla-central/source/extensions/spellcheck/src/mozInlineSpellChecker.cpp#2044
      - hairy bit
        - requires SetDictionary to ipc to make the parent get the dictionary, but also is called from jsm generating the (sync, cpow, sync) crash. 
        - compromise: shortcircuit js calls that crash until part2 (contextmenu rewrite)
        - compromise discussed & agreed upon in irc (:blassey, :ehsan, :ally)

  2 unified build causes compilation errors. 
    - must fix before shipping
    - blassey & gps stumped. Ehsan, what might be colliding?
  
  3 serious polish
    - remove debugging notes and dead code
    - sort out vim vs vs spacing issues
    - undo any accidentially deleted \n
    - seperate remotespellcheckengine Parent/child cpp/h into 4 files
    - check for nasty vs added \r
    - double check spacings around if (file's convention trumps moz
      standard. old code likely to run into this)
    - rename ipdl misspelled argument to something clearer
    - remove the receivelanguage call from the ipdl

Comment 20

[Allison Naaktgeboren :ally]

Attachment: Screenshot (21).png spellchecker telling words apart \o/

Comment 21

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 8443139 [details] [diff] [review]
wip, works mostly (see comment)

Review of attachment 8443139 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/ipc/ContentChild.cpp
@@ +1036,5 @@
> +
> +bool
> +ContentChild::DeallocPRemoteSpellcheckEngineChild(PRemoteSpellcheckEngineChild *child)
> +{
> +    return true;

shouldn't you delete the child here?

::: dom/ipc/ContentParent.cpp
@@ +2647,5 @@
> +
> +bool
> +ContentParent::DeallocPRemoteSpellcheckEngineParent(PRemoteSpellcheckEngineParent *parent)
> +{
> +    return true;

same here with the deleting

Comment 22

[(no longer active)]

Comment on attachment 8443139 [details] [diff] [review]
wip, works mostly (see comment)

Review of attachment 8443139 [details] [diff] [review]:
-----------------------------------------------------------------

I think this is on the right track.  Note that I mostly ignored the IPDL code as I don't know it very well, so please ask for someone else to check those bits.  Have you tested this with different dictionaries for different languages installed?  You can use the lang HTML attribute to ask us to spell check fields with other languages.

About the problem with unified builds, can you please post a build log?  That might give me an idea on what's going wrong.

::: editor/composer/src/nsEditorSpellCheck.cpp
@@ +160,5 @@
> +  */
> +  if(XRE_GetProcessType() == GeckoProcessType_Content) {
> +	  // result code is discarded in HandleCompletion
> +	  HandleCompletion(0);
> +	  return NS_OK;

Hmm, you need to find another way to do this work, by falling back to the code path that we currently take if there is no language content pref for the web page.  I suspect this might explain some of the assertions you're hitting.

::: extensions/spellcheck/hunspell/src/PRemoteSpellcheckEngine.ipdl
@@ +18,5 @@
> +  sync CheckWord(nsString aword) returns (bool misspelled);
> +
> +  sync SuggestWords(nsString aword) returns (nsString[] suggestions, int count);
> +
> +  sync SetDictionary(nsString aDictionary) returns (bool success);

We should investigate to see which one of the other methods on mozISpellcheckingEngine need to be reflected here.

::: extensions/spellcheck/hunspell/src/RemoteSpellCheckEngine.cpp
@@ +12,5 @@
> +// TODO split into two files
> +//---------------------------------------------------------------------------
> +// Concrete remote spellcheck engine parent
> +//---------------------------------------------------------------------------
> +  RemoteSpellcheckEngineParent::RemoteSpellcheckEngineParent() {

Nit: no need to indent the contents of namespaces.  Also, your patch contains some trailing whitespaces and some tabs, both of which kill ponies.  ;-)

@@ +22,5 @@
> +  bool 
> +  RemoteSpellcheckEngineParent::RecvGetLanguageOfCurrentDictionary(nsString* lang)
> +  {
> +    // TODO remove from ipdl
> +    lang->AssignLiteral("Klingon");

:-)

@@ +31,5 @@
> +		const nsString& aDictionary,
> +		bool* success)
> +  {
> +	  nsresult rv;
> +	  nsCOMPtr<mozISpellCheckingEngine> engine = do_GetService(DEFAULT_SPELL_CHECKER, &rv);

Wouldn't it make sense to keep a reference to this as a member of this class?

::: extensions/spellcheck/hunspell/src/mozHunspell.cpp
@@ +516,5 @@
>  NS_IMETHODIMP mozHunspell::Check(const char16_t *aWord, bool *aResult)
>  {
>    NS_ENSURE_ARG_POINTER(aWord);
>    NS_ENSURE_ARG_POINTER(aResult);
> +  NS_ENSURE_TRUE(mHunspell, NS_ERROR_FAILURE); //parent dropping ball here

What does this comment mean?

::: extensions/spellcheck/src/mozInlineSpellChecker.cpp
@@ +2024,5 @@
>    nsRefPtr<UpdateCurrentDictionaryCallback> cb =
>      new UpdateCurrentDictionaryCallback(this, mDisabledAsyncToken);
>    NS_ENSURE_STATE(cb);
>    nsresult rv = spellCheck->UpdateCurrentDictionary(cb);
> +  if (NS_FAILED(rv)) { //e10s also fails here

This might be caused by skipping the contentprefs stuff as I mentioned earlier.

::: extensions/spellcheck/src/mozSpellChecker.cpp
@@ +132,5 @@
> +	  // calling code expects true for misspellings
> +	  *aIsMisspelled = !isCorrect;
> +	  return NS_OK;
> +  }
> +  else {

Nit: no else after return.

Comment 23

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8443139 [details] [diff] [review]
wip, works mostly (see comment)

Review of attachment 8443139 [details] [diff] [review]:
-----------------------------------------------------------------

This looks like a good start to me.

Comment 24

[Allison Naaktgeboren :ally]

Attachment: review candidate v0

- Bug 1030449 addresses making other dictionaries (personal & installed) work
- Bug 1030451 is the refactor of the context menu code
- Bug 1029678 is the ?! windows.h, build system bug

I'd like to rename DictionaryFetched() to LookForADictionary() or something because the name is misleading. That's the function that goes looking for what dictionary to use if contentprefs don't turn one up.

No ponies were harmed in the making of this patch

Comment 25

[(no longer active)]

Comment on attachment 8446260 [details] [diff] [review]
review candidate v0

Review of attachment 8446260 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!  Blake, can you please do the honors for the IPDL stuff?  Thanks!

::: extensions/spellcheck/hunspell/src/RemoteSpellCheckEngineParent.cpp
@@ +11,5 @@
> +namespace mozilla {
> +
> +RemoteSpellcheckEngineParent::RemoteSpellcheckEngineParent() {
> +  nsresult rv;
> +  mEngine = do_GetService(DEFAULT_SPELL_CHECKER, &rv);

Nit: you might as well drop the second argument and remove rv completely.

::: extensions/spellcheck/src/moz.build
@@ +5,5 @@
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +include('/ipc/chromium/chromium-config.mozbuild')
> +#IPDL_SOURCES = [
> +#    'PRemoteSpellcheckEngine.ipdl',
> +#]

Nit: please remove the above.

::: extensions/spellcheck/src/mozSpellChecker.cpp
@@ +48,5 @@
>  }
>  nsresult
>  mozSpellChecker::Init()
>  {
> +	if (XRE_GetProcessType() == GeckoProcessType_Content) {

Nit: tabs!  :-)

Comment 27

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8446260 [details] [diff] [review]
review candidate v0

Review of attachment 8446260 [details] [diff] [review]:
-----------------------------------------------------------------

I have a bunch of nits (and one sort of substantive question). r=me with them addressed and the question answered.

::: dom/ipc/ContentChild.cpp
@@ +1059,5 @@
> +
> +bool
> +ContentChild::DeallocPRemoteSpellcheckEngineChild(PRemoteSpellcheckEngineChild *child)
> +{
> +    return true;

This needs to actually free the memory here, right?

::: dom/ipc/ContentParent.cpp
@@ +2663,5 @@
> +
> +bool
> +ContentParent::DeallocPRemoteSpellcheckEngineParent(PRemoteSpellcheckEngineParent *parent)
> +{
> +    return true;

This should actually deallocate the memory, right?

::: editor/composer/src/nsEditorSpellCheck.cpp
@@ +154,5 @@
>  {
> +  if (XRE_GetProcessType() == GeckoProcessType_Content) {
> +    /* ContentPrefs are currently broken under multiprocess firefox. Bug 1027898
> +     *  Skip this custom pref option for now in e10s
> +     */

Uber-nits: I'd use C++ //-style comments here (and remove the extra spaced before the S in "Skip").

::: extensions/spellcheck/hunspell/src/RemoteSpellCheckEngineChild.cpp
@@ +4,5 @@
> +
> +#include "RemoteSpellcheckEngineChild.h"
> +
> +namespace mozilla {
> +RemoteSpellcheckEngineChild::RemoteSpellcheckEngineChild() {

Uber-nit: Please add a blank line after the { and before the } for the namespace.

::: extensions/spellcheck/hunspell/src/RemoteSpellCheckEngineParent.cpp
@@ +9,5 @@
> +#define DEFAULT_SPELL_CHECKER "@mozilla.org/spellchecker/engine;1"
> +
> +namespace mozilla {
> +
> +RemoteSpellcheckEngineParent::RemoteSpellcheckEngineParent() {

Uber-nit (here and below): For methods in C++, the { goes on its own line under the function definition.

@@ +23,5 @@
> +		const nsString& aDictionary,
> +		bool* success) {
> +  nsresult rv;
> +  rv = mEngine->SetDictionary(aDictionary.get());
> +  *success=  (rv == NS_OK)? true : false;

Nits: Please declare rv on the same line as its first use: |nsresult rv = mEngine....;|

I'd prefer to see the second line as: *success = NS_SUCCEEDED(rv);

@@ +29,5 @@
> +}
> +
> +bool
> +RemoteSpellcheckEngineParent::RecvCheckWord(
> +      const nsString& aword,

Nit: aWord

@@ +33,5 @@
> +      const nsString& aword,
> +      bool* isCorrect)
> +{
> +  nsresult rv;
> +	*isCorrect = false;

Boo tabs!

Also, you're not using rv, I'd just nuke it.

::: extensions/spellcheck/src/mozSpellChecker.cpp
@@ +124,5 @@
> +  if (XRE_GetProcessType() == GeckoProcessType_Content) {
> +    nsString wordwrapped = nsString(aWord);
> +    bool isCorrect;
> +    mEngine->SendCheckWord(wordwrapped, &isCorrect);
> +    *aIsMisspelled = !isCorrect;

Given that the only use for this is here, why not invert the sense of the message? Then this could be SenIsWordMisspelled and you could also avoid the isCorrect.

::: extensions/spellcheck/src/mozSpellChecker.h
@@ +17,5 @@
>  #include "nsTArray.h"
>  #include "mozISpellI18NUtil.h"
>  #include "nsCycleCollectionParticipant.h"
> +namespace mozilla {
> +  class PRemoteSpellcheckEngineChild;

Nit: No indentation for namespace declarations.

Comment 28

[Allison Naaktgeboren :ally]

(In reply to Blake Kaplan (:mrbkap) from comment #27)
> Comment on attachment 8446260 [details] [diff] [review]
> review candidate v0
> 
> Review of attachment 8446260 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I have a bunch of nits (and one sort of substantive question). r=me with
> them addressed and the question answered.

> ::: extensions/spellcheck/src/mozSpellChecker.cpp
> @@ +124,5 @@
> > +  if (XRE_GetProcessType() == GeckoProcessType_Content) {
> > +    nsString wordwrapped = nsString(aWord);
> > +    bool isCorrect;
> > +    mEngine->SendCheckWord(wordwrapped, &isCorrect);
> > +    *aIsMisspelled = !isCorrect;
> 
> Given that the only use for this is here, why not invert the sense of the
> message? Then this could be SenIsWordMisspelled and you could also avoid the
> isCorrect.
> 
Ehsan requested that the ipdl calls mirror the behavior of mozISpellcheckingEngine interface that they wrap on the parent side. https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/mozISpellCheckingEngine#check%28%29 . There may exist spellcheck engines that actually use this. Ehsan thought he found a finnish one.

Comment 29

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8446260 [details] [diff] [review]
review candidate v0

Review of attachment 8446260 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/modules/InlineSpellChecker.jsm
@@ +270,5 @@
> +    let remoteTabs = gPrefService.getBoolPref("browser.tabs.remote");
> +    if (remoteTabs) {
> +      // Avoid a crash in e10s until Bug 1030451 lands
> +      return;
> +    }

Is this InlineSpellChecker.selectDictionary function being called from the parent or the child process?

browser.tabs.remote is now true for all profiles (well, for Nightly at least), so it's no longer the right way to check for e10s, as it's a per-window state now.

I'm guessing this is happening in the child, all you need to determine is if this is indeed a remote process. Here's the code necessary: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/satchel/nsFormAutoComplete.js#497

If it's a call in the parent, then it's gonna be a bit more complicated because then we need to find out which window is calling this.

@@ +296,5 @@
> +    let remoteTabs = gPrefService.getBoolPref("browser.tabs.remote");
> +    if (remoteTabs) {
> +      // Avoid a crash in e10s until Bug 1030451 lands
> +      return;
> +    }

likewise

Comment 30

[(no longer active)]

(In reply to :Allison Naaktgeboren from comment #28)
> (In reply to Blake Kaplan (:mrbkap) from comment #27)
> > Comment on attachment 8446260 [details] [diff] [review]
> > review candidate v0
> > 
> > Review of attachment 8446260 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > I have a bunch of nits (and one sort of substantive question). r=me with
> > them addressed and the question answered.
> 
> > ::: extensions/spellcheck/src/mozSpellChecker.cpp
> > @@ +124,5 @@
> > > +  if (XRE_GetProcessType() == GeckoProcessType_Content) {
> > > +    nsString wordwrapped = nsString(aWord);
> > > +    bool isCorrect;
> > > +    mEngine->SendCheckWord(wordwrapped, &isCorrect);
> > > +    *aIsMisspelled = !isCorrect;
> > 
> > Given that the only use for this is here, why not invert the sense of the
> > message? Then this could be SenIsWordMisspelled and you could also avoid the
> > isCorrect.
> > 
> Ehsan requested that the ipdl calls mirror the behavior of
> mozISpellcheckingEngine interface that they wrap on the parent side.
> https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/
> Interface/mozISpellCheckingEngine#check%28%29 . There may exist spellcheck
> engines that actually use this. Ehsan thought he found a finnish one.

FWIW since the IPDL messages will not be exposed to the add-ons, changing the semantics of the IPDL message either way is fine by me, as long as we don't change mozISpellcheckingEngine itself in the process.

Comment 31

[Allison Naaktgeboren :ally]

remote:   https://hg.mozilla.org/integration/fx-team/rev/d87da7f7a3a9

Comment 32

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

This failed to build on at least Android and B2G platforms: https://tbpl.mozilla.org/php/getParsedLog.php?id=42888177&tree=Fx-Team
Backed out in https://hg.mozilla.org/integration/fx-team/rev/a0b340c5704d

Comment 33

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to Wes Kocher (:KWierso) from comment #32)
> This failed to build on at least Android and B2G platforms:
> https://tbpl.mozilla.org/php/getParsedLog.php?id=42888177&tree=Fx-Team
> Backed out in https://hg.mozilla.org/integration/fx-team/rev/a0b340c5704d

Is this just a case sensitivity thing? The file name is RemoteSpellCheckEngineChild.h but the failure mentions RemoteSpellcheckEngineChild.h (with a lowercase 'c' in "Spellcheck").

Comment 34

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Everything failed to build except OSX and Windows.

Comment 35

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

It also failed Windows reftest and reftest-no-accel: https://tbpl.mozilla.org/php/getParsedLog.php?id=42890548&tree=Fx-Team

Comment 36

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

And Windows mochitest-2: https://tbpl.mozilla.org/php/getParsedLog.php?id=42891580&tree=Fx-Team

Comment 37

[Allison Naaktgeboren :ally]

I borrowed a box and fixed the linux & android build issues. Gonna push to try and see what shakes out.

Comment 40

[Allison Naaktgeboren :ally]

  https://hg.mozilla.org/integration/fx-team/rev/d5d67c4fe497

Once more, with feeling! My try build is green, so let's hope it sticks!

mochitest: weak reference for lifetime tracking
reftest: moving the dictionaryholder out of the if block
crashtests-ipc - significant rejiggering around the code to skip the content pref service in e10s and still run

big thank you to mrbkap for all his help.

Comment 41

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d5d67c4fe497