1326138 - add a new profile item <binding> and make rich-result-popup append item accordingly

Status: RESOLVED FIXED

(Toolkit :: Form Manager, defect)

Description

[Ray Lin[:ralin]]

Our impl now relies on autocomplete mechanism to popup results. To support two column layout, we should add a new <binding> prototype for profile item, and make codepath diverged from appending results[0]. 

Here we need to ensure that popup append correct <binding> respect the format of results. 


[0] https://dxr.mozilla.org/mozilla-central/rev/a6d29e9432f5f88a941c0ea5284cb082f34bd097/toolkit/content/widgets/autocomplete.xml#

Comment 1

[Ray Lin[:ralin]]

Attachment: Bug 1326138 - Add a new profile item binding and append it to result popup accordingly. f=MattN

Review commit: https://reviewboard.mozilla.org/r/101496/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/101496/

Comment 2

[Ray Lin[:ralin]]

Hi Matt,

I'm not sure if it is fine to assume that we could rely on `originalType` to determine the type of result. For now, I'd like to move forwards to next followup bug 1326139 based on this patch, but won't land this path until we come up with a more proper approach. We should replace the nasty hacks in `nsFormAutoCompleteResult.jsm` with a better way afterwards.


Could you give me feedback about this patch? thanks.

Comment 3

[Matthew N. [:MattN]]

Comment on attachment 8822624 [details]
Bug 1326138 - Add a new profile item binding and append it to result popup accordingly. f=MattN

https://reviewboard.mozilla.org/r/101496/#review102804

See bug 1328778 for a different direction. I think it would be great to understand if our binding (and perhaps the insecure field one) needs the complexity of chrome://global/content/bindings/autocomplete.xml#autocomplete-richlistitem or whether we should instead extend the parent of it which is chrome://global/content/bindings/richlistbox.xml#richlistitem. I think the main reason autocomplete-richlistitem was re-used is because of underflow/overflow calcuations related to ellipsis for long text but I wonder if that can be done in a better way nowadays with flexbox or some other CSS layout.

Comment 4

[Ray Lin[:ralin]]

(In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from comment #3)
> Comment on attachment 8822624 [details]
> Bug 1326138 - Add a new profile item binding and append it to result popup
> accordingly.
> 
> https://reviewboard.mozilla.org/r/101496/#review102804
> 
> See bug 1328778 for a different direction. I think it would be great to
> understand if our binding (and perhaps the insecure field one) needs the
> complexity of
> chrome://global/content/bindings/autocomplete.xml#autocomplete-richlistitem
> or whether we should instead extend the parent of it which is
> chrome://global/content/bindings/richlistbox.xml#richlistitem. I think the
> main reason autocomplete-richlistitem was re-used is because of
> underflow/overflow calcuations related to ellipsis for long text but I
> wonder if that can be done in a better way nowadays with flexbox or some
> other CSS layout.

Yeah, I was figuring out how much portion of logic should be re-used from #autocomplete-richlistitem. Extending #autocomplete-richlistitem seems too much for profile item, and also its variable name and attribute are more likely specified for `urlSearch` instead of a general purpose. It make more sense to me to extend our binding from #richlistitem, however, still have no idea how much complication would be brought out if we (need?) rewrite underflow/overflow calculation? I'd like to test flexbox on new binding based on #richlistitem first, and see whether works fine with it. I'll keep discuss with Sean about Bug 1328778 offline and try to come up with a appropriate direction to go. 

Thanks for the feedback!

Comment 5

[Ray Lin[:ralin]]

Comment on attachment 8822624 [details]
Bug 1326138 - Add a new profile item binding and append it to result popup accordingly. f=MattN

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/101496/diff/1-2/

Comment 6

[Ray Lin[:ralin]]

Comment on attachment 8822624 [details]
Bug 1326138 - Add a new profile item binding and append it to result popup accordingly. f=MattN

Hi Matt

New patch is based on #richlistitem now, and its layout should works fine on Mac (but result display incorrectly for now...). Also I got some thought about two different implementations of new binding:

#1. based on #autocomplete-richlistitem
good - might easily leverage original mechanism. e.g. `_appendCurrentResult` [0]
bad  - complicated layout and a bunch of unused data/functions
bad  - hard to style new layout. e.g. ellipsis for both long label & comment, or two-lines layout when width is smaller than 150px
risk - might mess up old codes and breaks autocomplete. e.g. is the current under/overflow calculation can work with new profile item styles(content wrapping, padding, margin, border-bottom, etc)?


#2. based on #richlistitem
good - flexbox works quite good, also is fine with `adjustHeight`[1]
good - easy to style as content are now written in HTML and no more unused structures. No need to handle overflow/underflow now
good - a whole new land for profile, which won't breaks original item binding
bad  - not reusable by popup [0]. We would need to re-write `_appendCurrentResult` to have a separate code-path for profile
risk  - potentially lack of functions might be used by unexpected call-sites that are not easily observable now.
 

The problem for both ways:
- modifying `_appendCurrentResult` in popup binding seems inevitable. 
-- concerning to new result format/API, we might need to separate it into two different constructors. (Perhaps we can refactor it and pass the result to their own constructor?)
-- even we retain old interface or format, still we have to refrain from calling undesired function (or undefined function if we chose #2)
- is it extendable/flexible for new features or further design changes?


Do you have any other concerns about these two approaches? If you don't mind, could you also tell us which way make more sense to you? Thanks :-)

(if it might takes you too much time to answer on bugzilla, we could discuss this in regular meeting)


[0] https://dxr.mozilla.org/mozilla-central/rev/845cc4dea57f6cc93f46810d24b1058b640c3b74/toolkit/content/widgets/autocomplete.xml#1288
[1] https://dxr.mozilla.org/mozilla-central/rev/845cc4dea57f6cc93f46810d24b1058b640c3b74/toolkit/content/widgets/autocomplete.xml#1221

Comment 7

[Matthew N. [:MattN]]

Comment on attachment 8822624 [details]
Bug 1326138 - Add a new profile item binding and append it to result popup accordingly. f=MattN

As discussed in our meeting I think option #2 is much cleaner and less fragile as long as we can achieve the desired layout with two text-overflow:ellipsis columns.

Comment 8

[Ray Lin[:ralin]]

Attachment: Bug 1326138 - Add a new profile item binding.

Review commit: https://reviewboard.mozilla.org/r/104740/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/104740/

Comment 9

[Ray Lin[:ralin]]

Comment on attachment 8826938 [details]
Bug 1326138 - Add a new profile item binding.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/104740/diff/1-2/

Comment 10

[Ray Lin[:ralin]]

Hi Matt

Could you help to review this patch?

The patch might be a less hacky approach for reusing binding I can think of now. The main point in this patch would be in `_appendCurrentResult`, where I tried to move `urlbar` specified code away from. I guess that we could not get rid of modifying popup to unless we create a popup binding for profile. Though, creating a new popup binding or not ought to be decided when we start on implementing the fixed footer of popup, that shall be a better timing for discussion then. BTW, We could keep seeking for a more proper way to achieve if any concern, it's fine to work on bug 1326139 in parallel.

Thank :-)

Comment 11

[Ray Lin[:ralin]]

type: s/popup to unless we/popup unless/

Comment 12

[Matthew N. [:MattN]]

Comment on attachment 8826938 [details]
Bug 1326138 - Add a new profile item binding.

Hey Drew,

Could you review only the changes to the existing binding (to make the popup binding more generic)?

Thanks

Comment 13

[Matthew N. [:MattN]]

Comment on attachment 8826938 [details]
Bug 1326138 - Add a new profile item binding.

https://reviewboard.mozilla.org/r/104740/#review106188

::: browser/themes/shared/autocomplete.inc.css:15
(Diff revision 2)
> +#PopupAutoComplete > richlistbox > richlistitem[originaltype="autofill-profile"] {
> +  display: block;
> +  height: auto;

I had originally said:
Move this to a shared theme file like flyweb did but I don't we need the three platforms yet or the @import. You will need to add the `skin` registration to the jar.mn file.

https://dxr.mozilla.org/mozilla-central/rev/6a23526fe5168087d7e4132c0705aefcaed5f571/browser/extensions/flyweb/jar.mn#9-10

But now I realize these are all properties that are fine in a content stylesheet so you can simple make a file autocomplete.css in the extensions /content/ directory. When we deal with theming we can create the skin directories.

::: toolkit/content/widgets/autocomplete.xml:1527
(Diff revision 2)
>          this.classList.add("forceHandleUnderflow");
>        ]]></constructor>
>      </implementation>
>    </binding>
>  
> +  <binding id="autocomplete-profile-listitem" extends="chrome://global/content/bindings/richlistbox.xml#richlistitem">

Please move this to a new XML file in the /content/ directory of our extension. Don't forget to include the XML header, license and <bindings>.

::: toolkit/content/xul.css:848
(Diff revision 2)
>    -moz-binding: url("chrome://global/content/bindings/autocomplete.xml#autocomplete-richlistitem");
>    -moz-box-orient: vertical;
>    overflow: -moz-hidden-unscrollable;
>  }
>  
> +.autocomplete-richlistitem[originaltype="autofill-profile"] {

This can move to the content autocomplete.css file too.

Comment 14

[Ray Lin[:ralin]]

Attachment: Bug 1326138 - Refactor _appendCurrentResult to be more generic.

Review commit: https://reviewboard.mozilla.org/r/106842/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/106842/

Comment 15

[Ray Lin[:ralin]]

Comment on attachment 8826938 [details]
Bug 1326138 - Add a new profile item binding.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/104740/diff/2-3/

Comment 16

[Ray Lin[:ralin]]

Hi MattN, 

I put the css rules and new profile binding in formautofill content folder now, could you review this again? Thanks

---

Hi Drew,

I attempted to move `urlbar` specific code from _appendCurrentResult to its own binding, and make _appendCurrentResult more generic. Could you review this patch? Thanks

Comment 17

[Matthew N. [:MattN]]

Comment on attachment 8826938 [details]
Bug 1326138 - Add a new profile item binding.

https://reviewboard.mozilla.org/r/104740/#review108132

::: browser/extensions/formautofill/bootstrap.js:27
(Diff revision 3)
>      return;
>    }
>  
> +  let styleSheetService = Cc["@mozilla.org/content/style-sheet-service;1"]
> +    .getService(Ci.nsIStyleSheetService);
> +  let styleSheetsURI = Services.io.newURI(styleSheet, null, null);

Nit: The last two arguments are recently optional.

::: browser/extensions/formautofill/bootstrap.js:29
(Diff revision 3)
> +  styleSheetService.loadAndRegisterSheet(styleSheetsURI, styleSheetService.AUTHOR_SHEET);
> +

I'm pretty sure you don't want to use this API as it will load the stylesheet in every document, I think even website ones (or at least all documents in the parent process). It's also synchronous which we don't need and may slow startup.

I think you should use something like: https://dxr.mozilla.org/mozilla-central/rev/8ff550409e1d1f8b54f6f7f115545dbef857be0b/browser/base/content/browser-compacttheme.js#34-41 but you probably don't need the load handler. You will need to run that code in every "navigator:browser" that is already open and add a listener to for new browser windows that open and load the stylesheet there too.

See https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIWindowMediator#Enumerating_windows (use type="navigator:browser") and https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIWindowMediator#addListener() (also check for "navigator:browser")

Comment 18

[Ray Lin[:ralin]]

Comment on attachment 8829884 [details]
Bug 1326138 - Refactor _appendCurrentResult to be more generic.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/106842/diff/1-2/

Comment 19

[Ray Lin[:ralin]]

Comment on attachment 8826938 [details]
Bug 1326138 - Add a new profile item binding.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/104740/diff/3-4/

Comment 20

[Ray Lin[:ralin]]

Comment on attachment 8826938 [details]
Bug 1326138 - Add a new profile item binding.

https://reviewboard.mozilla.org/r/104740/#review108132

> I'm pretty sure you don't want to use this API as it will load the stylesheet in every document, I think even website ones (or at least all documents in the parent process). It's also synchronous which we don't need and may slow startup.
> 
> I think you should use something like: https://dxr.mozilla.org/mozilla-central/rev/8ff550409e1d1f8b54f6f7f115545dbef857be0b/browser/base/content/browser-compacttheme.js#34-41 but you probably don't need the load handler. You will need to run that code in every "navigator:browser" that is already open and add a listener to for new browser windows that open and load the stylesheet there too.
> 
> See https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIWindowMediator#Enumerating_windows (use type="navigator:browser") and https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIWindowMediator#addListener() (also check for "navigator:browser")

I've modified the patch according as your helpful suggestion, could you help to review it? Thanks.

Comment 21

[Matthew N. [:MattN]]

Comment on attachment 8826938 [details]
Bug 1326138 - Add a new profile item binding.

https://reviewboard.mozilla.org/r/104740/#review112270

::: browser/extensions/formautofill/bootstrap.js:10
(Diff revision 4)
>  "use strict";
>  
>  /* exported startup, shutdown, install, uninstall */
>  
>  const {classes: Cc, interfaces: Ci, results: Cr, utils: Cu} = Components;
> +const {console} = Cu.import("resource://gre/modules/Console.jsm", {});

Nit: I think this is leftover from development so should be removed but I plan to add a logging helper in the next week.

::: browser/extensions/formautofill/bootstrap.js:11
(Diff revision 4)
> +const styleSheetURI = "chrome://formautofill/content/formautofill.css";
> +const cachedStyleSheets = [];

Nit: I believe the style guide has constants as uppercase with underscores.

::: browser/extensions/formautofill/bootstrap.js:28
(Diff revision 4)
> +    let onWindowLoaded = () => {
> +      domWindow.removeEventListener("load", onWindowLoaded);
> +      insertStyleSheet(domWindow, styleSheetURI);
> +    };
> +
> +    domWindow.addEventListener("load", onWindowLoaded);

Please use the new `once` option.

Comment 22

[Ray Lin[:ralin]]

Comment on attachment 8829884 [details]
Bug 1326138 - Refactor _appendCurrentResult to be more generic.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/106842/diff/2-3/

Comment 23

[Ray Lin[:ralin]]

Comment on attachment 8826938 [details]
Bug 1326138 - Add a new profile item binding.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/104740/diff/4-5/

Comment 24

[Ray Lin[:ralin]]

Comment on attachment 8826938 [details]
Bug 1326138 - Add a new profile item binding.

https://reviewboard.mozilla.org/r/104740/#review112270

> Please use the new `once` option.

also moved `onWindowLoaded` to a named function as second parameter of event listner.

Comment 25

[Drew Willcoxon :adw]

Comment on attachment 8829884 [details]
Bug 1326138 - Refactor _appendCurrentResult to be more generic.

https://reviewboard.mozilla.org/r/106842/#review113586

Sorry for the delay.  I think this looks OK, but there is one potential problem regarding item.collapsed.

::: toolkit/content/widgets/autocomplete.xml:1326
(Diff revision 3)
> -            let url = controller.getValueAt(this._currentIndex);
>  
> -            if (this._currentIndex < existingItemsCount) {
> +            if (itemExists) {
> -              // re-use the existing item
>                item = this.richlistbox.childNodes[this._currentIndex];
> +              item.collapsed = false;

Currently, collapsed is set to false in the itemExists && !reused case *after* _adjustAcItem is called.  I don't think we should change that unless there's a good reason.  Is that an intentional change?

::: toolkit/content/widgets/autocomplete.xml:2061
(Diff revision 3)
> +              this.adjustSiteIconStart(popup._siteIconStart);
> +              // The popup may have changed size between now and the last
> +              // time the item was shown, so always handle over/underflow.
> +              this.handleOverUnderflow();
> +
> +              reused = true;

Nit: Usually we just return early instead of setting a variable and then returning only once at the end.  It's not a big deal.

Comment 26

[Ray Lin[:ralin]]

Comment on attachment 8829884 [details]
Bug 1326138 - Refactor _appendCurrentResult to be more generic.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/106842/diff/3-4/

Comment 27

[Ray Lin[:ralin]]

Comment on attachment 8826938 [details]
Bug 1326138 - Add a new profile item binding.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/104740/diff/5-6/

Comment 28

[Ray Lin[:ralin]]

Comment on attachment 8829884 [details]
Bug 1326138 - Refactor _appendCurrentResult to be more generic.

https://reviewboard.mozilla.org/r/106842/#review113586

Thanks for your review, the issues are fixed now.

> Currently, collapsed is set to false in the itemExists && !reused case *after* _adjustAcItem is called.  I don't think we should change that unless there's a good reason.  Is that an intentional change?

yeah, we should put this line of code after _adjustAcItem() where fulfills itemExists && !reused. I've changed back the way how we used to do.

> Nit: Usually we just return early instead of setting a variable and then returning only once at the end.  It's not a big deal.

Ah! We don't need that variable indeed.

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/40d22c458595
Refactor _appendCurrentResult to be more generic. r=adw
https://hg.mozilla.org/integration/autoland/rev/1582e02a447d
Add a new profile item binding. r=MattN

Comment 30

[Iris Hsiao [:ihsiao]]

sorry had to back this out for eslint failure, https://treeherder.mozilla.org/logviewer.html#?job_id=77133572&repo=autoland&lineNumber=8470

Comment 31

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/623383299258
Backed out changeset 1582e02a447d for eslint failure
https://hg.mozilla.org/integration/autoland/rev/2e824f54ab6e
Backed out changeset 40d22c458595

Comment 32

[Ray Lin[:ralin]]

Comment on attachment 8829884 [details]
Bug 1326138 - Refactor _appendCurrentResult to be more generic.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/106842/diff/4-5/

Comment 33

[Ray Lin[:ralin]]

Comment on attachment 8826938 [details]
Bug 1326138 - Add a new profile item binding.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/104740/diff/6-7/

Comment 34

[Ray Lin[:ralin]]

Comment on attachment 8829884 [details]
Bug 1326138 - Refactor _appendCurrentResult to be more generic.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/106842/diff/5-6/

Comment 35

[Ray Lin[:ralin]]

Comment on attachment 8826938 [details]
Bug 1326138 - Add a new profile item binding.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/104740/diff/7-8/

Comment 36

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/059c01d1a40f
Refactor _appendCurrentResult to be more generic. r=adw
https://hg.mozilla.org/integration/autoland/rev/4aef71d8a987
Add a new profile item binding. r=MattN

Comment 37

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for leaks, e.g. in devtools/client/aboutdebugging/test/browser_service_workers_not_compatible.js:

https://hg.mozilla.org/integration/autoland/rev/931772473e3657d6aa5939e1ad6d49020b4938b2
https://hg.mozilla.org/integration/autoland/rev/7d9b751f833bca6466dc49eb7159b2110a7209cc

This might be the same issue like the JP failures in the Try push.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=4aef71d8a98724202391e48a1be5bbe65dc1434f&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log example: https://treeherder.mozilla.org/logviewer.html#?job_id=77500443&repo=autoland

[task 2017-02-15T08:27:37.433986Z] 08:27:37     INFO - TEST-START | Shutdown
[task 2017-02-15T08:27:37.434033Z] 08:27:37     INFO - Browser Chrome Test Summary
[task 2017-02-15T08:27:37.435673Z] 08:27:37     INFO - Passed:  139
[task 2017-02-15T08:27:37.437028Z] 08:27:37     INFO - Failed:  0
[task 2017-02-15T08:27:37.438047Z] 08:27:37     INFO - Todo:    0
[task 2017-02-15T08:27:37.439131Z] 08:27:37     INFO - Mode:    e10s
[task 2017-02-15T08:27:37.441554Z] 08:27:37     INFO - *** End BrowserChrome Test Results ***
[task 2017-02-15T08:27:37.821164Z] 08:27:37     INFO - --DOCSHELL 0x7f225488d000 == 4 [pid = 1454] [id = {d3e80d59-5f72-4972-8be7-e4d84d12ad27}]
[task 2017-02-15T08:27:37.844742Z] 08:27:37     INFO - --DOCSHELL 0x7f224980d000 == 3 [pid = 1454] [id = {b186eb20-dadd-4c10-b49b-236d689c95f9}]
[task 2017-02-15T08:27:37.913385Z] 08:27:37     INFO - --DOCSHELL 0x7f2261c42800 == 2 [pid = 1454] [id = {58c45956-8beb-4679-b1df-7071f7e2cff4}]
[task 2017-02-15T08:27:38.867395Z] 08:27:38     INFO - --DOCSHELL 0x7f2254895000 == 1 [pid = 1454] [id = {aeacb16c-3f96-41b0-95c5-ef2a1a623f8b}]
[task 2017-02-15T08:27:38.867555Z] 08:27:38     INFO - --DOCSHELL 0x7f225e9df800 == 0 [pid = 1454] [id = {f741ddb8-644c-4e2c-9f28-4cf2e28a3d0c}]
[task 2017-02-15T08:27:38.991771Z] 08:27:38     INFO - [Child 1548] WARNING: NS_ENSURE_TRUE(maybeContext) failed: file /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp, line 1009
[task 2017-02-15T08:27:39.009178Z] 08:27:39     INFO - [Child 1548] WARNING: Finishing incremental GC in progress during CC: file /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp, line 3537
[task 2017-02-15T08:27:39.037253Z] 08:27:39     INFO - [Child 1512] WARNING: NS_ENSURE_TRUE(maybeContext) failed: file /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp, line 1009
[task 2017-02-15T08:27:39.367605Z] 08:27:39     INFO - [Child 1548] WARNING: 'NS_FAILED(DebuggerOnGCRunnable::Enqueue(aContext, aDesc)) && reason != JS::gcreason::SHUTDOWN_CC && reason != JS::gcreason::DESTROY_RUNTIME && reason != JS::gcreason::XPCONNECT_SHUTDOWN', file /home/worker/workspace/build/src/xpcom/base/CycleCollectedJSContext.cpp, line 827
[task 2017-02-15T08:27:39.370909Z] 08:27:39     INFO - --DOCSHELL 0x7f49fce39000 == 0 [pid = 1548] [id = {358a5c7c-2dde-4d61-975a-d00cb0f04018}]
[task 2017-02-15T08:27:40.034364Z] 08:27:40     INFO - --DOMWINDOW == 1 (0x7f49fe055800) [pid = 1548] [serial = 33] [outer = (nil)] [url = about:blank]
[task 2017-02-15T08:27:40.034636Z] 08:27:40     INFO - --DOMWINDOW == 0 (0x7f49fddcb000) [pid = 1548] [serial = 32] [outer = (nil)] [url = about:blank]
[task 2017-02-15T08:27:40.034778Z] 08:27:40     INFO - [Child 1548] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:40.034911Z] 08:27:40     INFO - [Child 1548] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:40.035041Z] 08:27:40     INFO - [Child 1548] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:40.035177Z] 08:27:40     INFO - [Child 1548] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:40.035316Z] 08:27:40     INFO - [Child 1548] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:40.035531Z] 08:27:40     INFO - [Child 1548] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:40.035778Z] 08:27:40     INFO - [Child 1548] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:40.036322Z] 08:27:40     INFO - [Child 1548] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:40.092916Z] 08:27:40     INFO - [Child 1548] WARNING: '!compMgr', file /home/worker/workspace/build/src/xpcom/components/nsComponentManagerUtils.cpp, line 63
[task 2017-02-15T08:27:40.093081Z] 08:27:40     INFO - [Child 1548] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:40.093222Z] 08:27:40     INFO - [Child 1548] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:40.093356Z] 08:27:40     INFO - [Child 1548] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:40.093537Z] 08:27:40     INFO - [Child 1548] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:40.093644Z] 08:27:40     INFO - [Child 1548] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:40.093798Z] 08:27:40     INFO - [Child 1548] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:40.093930Z] 08:27:40     INFO - [Child 1548] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:40.094070Z] 08:27:40     INFO - [Child 1548] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:40.094153Z] 08:27:40     INFO - nsStringStats
[task 2017-02-15T08:27:40.095339Z] 08:27:40     INFO -  => mAllocCount:          62604
[task 2017-02-15T08:27:40.095887Z] 08:27:40     INFO -  => mReallocCount:          659
[task 2017-02-15T08:27:40.096451Z] 08:27:40     INFO -  => mFreeCount:           62604
[task 2017-02-15T08:27:40.097929Z] 08:27:40     INFO -  => mShareCount:          47166
[task 2017-02-15T08:27:40.099528Z] 08:27:40     INFO -  => mAdoptCount:          11161
[task 2017-02-15T08:27:40.100894Z] 08:27:40     INFO -  => mAdoptFreeCount:      11161
[task 2017-02-15T08:27:40.102014Z] 08:27:40     INFO -  => Process ID: 1548, Thread ID: 139956438640448
[task 2017-02-15T08:27:40.140655Z] 08:27:40     INFO - [Child 1512] WARNING: '!compMgr', file /home/worker/workspace/build/src/xpcom/components/nsComponentManagerUtils.cpp, line 63
[task 2017-02-15T08:27:40.140770Z] 08:27:40     INFO - [Child 1512] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:40.141702Z] 08:27:40     INFO - [Child 1512] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:40.142247Z] 08:27:40     INFO - [Child 1512] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:40.142395Z] 08:27:40     INFO - [Child 1512] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:40.142478Z] 08:27:40     INFO - [Child 1512] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:40.143093Z] 08:27:40     INFO - [Child 1512] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:40.144780Z] 08:27:40     INFO - [Child 1512] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:40.145716Z] 08:27:40     INFO - [Child 1512] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:40.160768Z] 08:27:40     INFO - nsStringStats
[task 2017-02-15T08:27:40.162044Z] 08:27:40     INFO -  => mAllocCount:          64955
[task 2017-02-15T08:27:40.163353Z] 08:27:40     INFO -  => mReallocCount:          818
[task 2017-02-15T08:27:40.164793Z] 08:27:40     INFO -  => mFreeCount:           64955
[task 2017-02-15T08:27:40.166240Z] 08:27:40     INFO -  => mShareCount:          50795
[task 2017-02-15T08:27:40.167890Z] 08:27:40     INFO -  => mAdoptCount:          10964
[task 2017-02-15T08:27:40.169216Z] 08:27:40     INFO -  => mAdoptFreeCount:      10964
[task 2017-02-15T08:27:40.170484Z] 08:27:40     INFO -  => Process ID: 1512, Thread ID: 140172248438592
[task 2017-02-15T08:27:40.814416Z] 08:27:40     INFO - [Parent 1454] WARNING: NS_ENSURE_TRUE(mDB) failed: file /home/worker/workspace/build/src/netwerk/cache/nsDiskCacheDeviceSQL.cpp, line 1376
[task 2017-02-15T08:27:41.018397Z] 08:27:41     INFO - [Parent 1454] WARNING: NS_ENSURE_TRUE(maybeContext) failed: file /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp, line 1009
[task 2017-02-15T08:27:41.019846Z] 08:27:41     INFO - [Parent 1454] WARNING: NS_ENSURE_TRUE(maybeContext) failed: file /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp, line 1009
[task 2017-02-15T08:27:41.021331Z] 08:27:41     INFO - [Parent 1454] WARNING: 'NS_FAILED(RemovePermissionChangeObserver())', file /home/worker/workspace/build/src/dom/notification/Notification.cpp, line 669
[task 2017-02-15T08:27:43.724328Z] 08:27:43     INFO - --DOMWINDOW == 11 (0x7f224758a800) [pid = 1454] [serial = 124] [outer = (nil)] [url = about:blank]
[task 2017-02-15T08:27:43.726289Z] 08:27:43     INFO - --DOMWINDOW == 10 (0x7f225e9e1000) [pid = 1454] [serial = 4] [outer = (nil)] [url = about:blank]
[task 2017-02-15T08:27:43.735223Z] 08:27:43     INFO - --DOMWINDOW == 9 (0x7f225e9e0000) [pid = 1454] [serial = 3] [outer = (nil)] [url = chrome://browser/content/browser.xul]
[task 2017-02-15T08:27:43.738446Z] 08:27:43     INFO - --DOMWINDOW == 8 (0x7f224980f800) [pid = 1454] [serial = 12] [outer = (nil)] [url = chrome://mochikit/content/browser-harness.xul]
[task 2017-02-15T08:27:43.738929Z] 08:27:43     INFO - --DOMWINDOW == 7 (0x7f2249810800) [pid = 1454] [serial = 13] [outer = (nil)] [url = about:blank]
[task 2017-02-15T08:27:43.739105Z] 08:27:43     INFO - --DOMWINDOW == 6 (0x7f225488e800) [pid = 1454] [serial = 6] [outer = (nil)] [url = about:blank]
[task 2017-02-15T08:27:43.739633Z] 08:27:43     INFO - --DOMWINDOW == 5 (0x7f225177b800) [pid = 1454] [serial = 10] [outer = (nil)] [url = about:blank]
[task 2017-02-15T08:27:43.740665Z] 08:27:43     INFO - --DOMWINDOW == 4 (0x7f2254895800) [pid = 1454] [serial = 7] [outer = (nil)] [url = about:blank]
[task 2017-02-15T08:27:43.742431Z] 08:27:43     INFO - --DOMWINDOW == 3 (0x7f223c08e000) [pid = 1454] [serial = 156] [outer = (nil)] [url = about:blank]
[task 2017-02-15T08:27:43.778857Z] 08:27:43     INFO - --DOMWINDOW == 2 (0x7f2247589800) [pid = 1454] [serial = 123] [outer = (nil)] [url = chrome://browser/content/browser.xul]
[task 2017-02-15T08:27:43.781133Z] 08:27:43     INFO - --DOMWINDOW == 1 (0x7f2261c43000) [pid = 1454] [serial = 1] [outer = (nil)] [url = resource://gre-resources/hiddenWindow.html]
[task 2017-02-15T08:27:43.789778Z] 08:27:43     INFO - --DOMWINDOW == 0 (0x7f225cf3d800) [pid = 1454] [serial = 5] [outer = (nil)] [url = resource://gre-resources/hiddenWindow.html]
[task 2017-02-15T08:27:43.855715Z] 08:27:43     INFO - [Parent 1454] WARNING: '!compMgr', file /home/worker/workspace/build/src/xpcom/components/nsComponentManagerUtils.cpp, line 63
[task 2017-02-15T08:27:43.858011Z] 08:27:43     INFO - [Parent 1454] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:43.859644Z] 08:27:43     INFO - [Parent 1454] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:43.861651Z] 08:27:43     INFO - [Parent 1454] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:43.862937Z] 08:27:43     INFO - [Parent 1454] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:43.864454Z] 08:27:43     INFO - [Parent 1454] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:43.865666Z] 08:27:43     INFO - [Parent 1454] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:43.866983Z] 08:27:43     INFO - [Parent 1454] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:43.868533Z] 08:27:43     INFO - [Parent 1454] WARNING: '!mMainThread', file /home/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 314
[task 2017-02-15T08:27:43.869717Z] 08:27:43     INFO - nsStringStats
[task 2017-02-15T08:27:43.870876Z] 08:27:43     INFO -  => mAllocCount:         473752
[task 2017-02-15T08:27:43.871980Z] 08:27:43     INFO -  => mReallocCount:        42095
[task 2017-02-15T08:27:43.873150Z] 08:27:43     INFO -  => mFreeCount:          473752
[task 2017-02-15T08:27:43.874339Z] 08:27:43     INFO -  => mShareCount:         387139
[task 2017-02-15T08:27:43.876010Z] 08:27:43     INFO -  => mAdoptCount:          33559
[task 2017-02-15T08:27:43.876709Z] 08:27:43     INFO -  => mAdoptFreeCount:      33559
[task 2017-02-15T08:27:43.877438Z] 08:27:43     INFO -  => Process ID: 1454, Thread ID: 139786344990528
[task 2017-02-15T08:27:43.902395Z] 08:27:43     INFO - TEST-INFO | Main app process: exit 0
[task 2017-02-15T08:27:43.902577Z] 08:27:43    ERROR - TEST-UNEXPECTED-FAIL | devtools/client/aboutdebugging/test/browser_service_workers_not_compatible.js | leaked 1 window(s) until shutdown [url = about:blank]
[task 2017-02-15T08:27:43.903079Z] 08:27:43    ERROR - TEST-UNEXPECTED-FAIL | devtools/client/aboutdebugging/test/browser_service_workers_not_compatible.js | leaked 1 window(s) until shutdown [url = chrome://browser/content/browser.xul]

Comment 38

[Ray Lin[:ralin]]

Comment on attachment 8829884 [details]
Bug 1326138 - Refactor _appendCurrentResult to be more generic.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/106842/diff/6-7/

Comment 39

[Ray Lin[:ralin]]

Comment on attachment 8826938 [details]
Bug 1326138 - Add a new profile item binding.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/104740/diff/8-9/

Comment 40

[Ray Lin[:ralin]]

Comment on attachment 8829884 [details]
Bug 1326138 - Refactor _appendCurrentResult to be more generic.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/106842/diff/7-8/

Comment 41

[Ray Lin[:ralin]]

Comment on attachment 8826938 [details]
Bug 1326138 - Add a new profile item binding.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/104740/diff/9-10/

Comment 42

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b25738421d6e
Refactor _appendCurrentResult to be more generic. r=adw
https://hg.mozilla.org/integration/autoland/rev/bd3f65389f27
Add a new profile item binding. r=MattN

Comment 43

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

https://hg.mozilla.org/mozilla-central/rev/b25738421d6e
https://hg.mozilla.org/mozilla-central/rev/bd3f65389f27

Comment 44

[Ray Lin[:ralin]]

(In reply to Iris Hsiao [:ihsiao] from comment #30)
> sorry had to back this out for eslint failure,
> https://treeherder.mozilla.org/logviewer.
> html#?job_id=77133572&repo=autoland&lineNumber=8470

thank you. remove NI as patch already landed

Comment 45

[Adrian Florinescu [:aflorinescu]]

As per San-Francisco meeting with :vchen, marking this bug as qe-.