This is a first step toward 1310770 - namely it handles deleting the other tabs participating in the same GroupedSHistory when closing the tab. It doesn't handle dragging between windows yet unfortunately, though I have plans for what that may look like.

MozReview-Commit-ID: KiMsKOljNpE

Comment on attachment 8811938 [details] [diff] [review] Remove inactive hidden tabs when removing the primary tab Review of attachment 8811938 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/base/content/test/general/browser_close_dependent_tabs.js @@ +28,5 @@ > + }); > + > + // At this point tab2 should be closed > + todo(!tab2.linkedBrowser, "The new tab should be closed"); > + yield BrowserTestUtils.removeTab(tab2); // XXX: Shouldn't be needed once the todo is fixed I'm assuming you'll be fixing this soon? ::: dom/base/GroupedSHistory.cpp @@ +173,5 @@ > +{ > + for (int32_t i = 0; i < mPartialHistories.Length(); ++i) { > + if (i != mIndexOfActivePartialHistory) { > + nsCOMPtr<nsIFrameLoader> loader; > + mPartialHistories[i]->GetOwnerFrameLoader(getter_AddRefs(loader)); Other code here null checks mPartialHistories[i].... Also is loader guaranteed to be non-null?

Comment on attachment 8811938 [details] [diff] [review] Remove inactive hidden tabs when removing the primary tab Looks good. I assume our plan for navigating to chrome process url such as about:config is using updateBrowserRemoteness, and closeInactiveFrameLoaderOwners can also be used in this case, which is nice. (In reply to :Ehsan Akhgari from comment #2) > Comment on attachment 8811938 [details] [diff] [review] > ::: dom/base/GroupedSHistory.cpp > @@ +173,5 @@ > > +{ > > + for (int32_t i = 0; i < mPartialHistories.Length(); ++i) { > > + if (i != mIndexOfActivePartialHistory) { > > + nsCOMPtr<nsIFrameLoader> loader; > > + mPartialHistories[i]->GetOwnerFrameLoader(getter_AddRefs(loader)); > > Other code here null checks mPartialHistories[i].... > > Also is loader guaranteed to be non-null? May all be unnecessary. It was the nervous first time cycle collector user of me adding null checks everywhere...

Pushed by michael@thelayzells.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/c4669804cc49 Remove inactive hidden tabs when removing the primary tab, r=ehsan

Backed out for failing browser_close_dependent_tabs.js: https://hg.mozilla.org/integration/mozilla-inbound/rev/b2e450275deefe6855206ae14493a89dbe68977d Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=c4669804cc496d815bc7438a19d3ef23459abc2f Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=39814337&repo=mozilla-inbound [task 2016-11-24T18:11:33.570831Z] 18:11:33 INFO - TEST-PASS | browser/base/content/test/general/browser_close_dependent_tabs.js | The browser changed process! - [task 2016-11-24T18:11:33.571020Z] 18:11:33 INFO - Buffered messages finished [task 2016-11-24T18:11:33.572984Z] 18:11:33 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_close_dependent_tabs.js | Uncaught exception - [Exception... "[JavaScript Error: "Unknown userContextId!" {file: "resource://gre/components/UnifiedComplete.js" line: 370}]'[JavaScript Error: "Unknown userContextId!" {file: "resource://gre/components/UnifiedComplete.js" line: 370}]' when calling method: [mozIPlacesAutoComplete::unregisterOpenPage]" nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)" location: "JS frame :: chrome://browser/content/tabbrowser.xml :: _beginRemoveTab :: line 2627" data: yes] [task 2016-11-24T18:11:33.577387Z] 18:11:33 INFO - Stack trace: [task 2016-11-24T18:11:33.578731Z] 18:11:33 INFO - _beginRemoveTab@chrome://browser/content/tabbrowser.xml:2627:15 [task 2016-11-24T18:11:33.580402Z] 18:11:33 INFO - removeTab@chrome://browser/content/tabbrowser.xml:2480:18 [task 2016-11-24T18:11:33.581863Z] 18:11:33 INFO - removeTab/<@resource://testing-common/BrowserTestUtils.jsm:823:9 [task 2016-11-24T18:11:33.583412Z] 18:11:33 INFO - removeTab@resource://testing-common/BrowserTestUtils.jsm:813:12 [task 2016-11-24T18:11:33.588354Z] 18:11:33 INFO - this.BrowserTestUtils.withNewTab<@resource://testing-common/BrowserTestUtils.jsm:82:13 [task 2016-11-24T18:11:33.590102Z] 18:11:33 INFO - @chrome://mochitests/content/browser/browser/base/content/test/general/browser_close_dependent_tabs.js:35:9 [task 2016-11-24T18:11:33.592436Z] 18:11:33 INFO - Tester_execTest@chrome://mochikit/content/browser-test.js:737:9 [task 2016-11-24T18:11:33.594453Z] 18:11:33 INFO - Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:657:7 [task 2016-11-24T18:11:33.599372Z] 18:11:33 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:744:59 [task 2016-11-24T18:11:33.601171Z] 18:11:33 INFO - Tester_execTest@chrome://mochikit/content/browser-test.js:737:9 [task 2016-11-24T18:11:33.602773Z] 18:11:33 INFO - Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:657:7 [task 2016-11-24T18:11:33.604449Z] 18:11:33 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:744:59 [task 2016-11-24T18:11:33.606131Z] 18:11:33 INFO - Leaving test bound

This needs to be landed at the same time as bug 1318767 and that failure should go away.

try server now builds with -Werror=sign-compare so it needs update.

rebase

Pushed by echen@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/5dbec5fef561 Remove inactive hidden tabs when removing the primary tab, r=ehsan