Closed Bug 1245355 Opened 4 years ago Closed 4 years ago

tabs.getAllInWindow has no unit tests

Categories

(WebExtensions :: Untriaged, defect)

defect
Not set

Tracking

(firefox48 fixed)

RESOLVED FIXED
mozilla48
Iteration:
48.2 - Apr 4
Tracking Status
firefox48 --- fixed

People

(Reporter: mattw, Assigned: mattw)

References

Details

(Whiteboard: [tabs] triaged)

Attachments

(1 file)

No description provided.
Flags: blocking-webextensions-
Flags: blocking-webextensions- → blocking-webextensions+
Iteration: --- → 47.2 - Feb 22
Iteration: 47.2 - Feb 22 → 47.3 - Mar 7
:mattw assigning to you because you moved into the iteration
Assignee: nobody → mwein
Iteration: 47.3 - Mar 7 → 48.1 - Mar 21
Attachment #8730652 - Flags: review?(kmaglione+bmo)
Comment on attachment 8730652 [details]
MozReview Request: Bug 1245355: Add a unit test for tabs.getAllInWindow. r?kmag

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40073/diff/1-2/
Attachment #8730652 - Attachment description: MozReview Request: Bug 1245355 Add a unit test for tabs.getAllInWindow → MozReview Request: Bug 1245355 Add a unit test for tabs.getAllInWindow r?kmag
Comment on attachment 8730652 [details]
MozReview Request: Bug 1245355: Add a unit test for tabs.getAllInWindow. r?kmag

https://reviewboard.mozilla.org/r/40073/#review36633
Attachment #8730652 - Flags: review?(kmaglione+bmo) → review+
Thanks
Keywords: checkin-needed
Backed out for leaking 2 windows and a docshell in debug builds.

Backout: https://hg.mozilla.org/integration/fx-team/rev/03bf161a225e

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=06ae85d888e8
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=8049222&repo=fx-team

10:04:39  WARNING -  TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_tabs_getAllInWindow.js | leaked 2 window(s) until shutdown [url = moz-extension://8ccbc303-aef0-4dd1-abfe-c45502c96e7c/_blank.html]
10:04:39     INFO -  TEST-INFO | browser/components/extensions/test/browser/browser_ext_tabs_getAllInWindow.js | windows(s) leaked: [pid = 4080] [serial = 973], [pid = 4080] [serial = 975]
10:04:39  WARNING -  TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_tabs_getAllInWindow.js | leaked 1 docShell(s) until shutdown
Flags: needinfo?(mwein)
Iteration: 48.1 - Mar 21 → ---
Iteration: --- → 48.2 - Apr 4
https://reviewboard.mozilla.org/r/40073/#review36633

I decided to remove this method since it's deprecated and none of the other deprecated methods are implemented.
Comment on attachment 8730652 [details]
MozReview Request: Bug 1245355: Add a unit test for tabs.getAllInWindow. r?kmag

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40073/diff/2-3/
Attachment #8730652 - Attachment description: MozReview Request: Bug 1245355 Add a unit test for tabs.getAllInWindow r?kmag → MozReview Request: Bug 1245355: Add a unit test for tabs.getAllInWindow. try: -b do -p linux64,macosx64,win32,win64 -u mochitest,xpcshell -n --post-to-bugzilla
Comment on attachment 8730652 [details]
MozReview Request: Bug 1245355: Add a unit test for tabs.getAllInWindow. r?kmag

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40073/diff/3-4/
Attachment #8730652 - Attachment description: MozReview Request: Bug 1245355: Add a unit test for tabs.getAllInWindow. try: -b do -p linux64,macosx64,win32,win64 -u mochitest,xpcshell -n --post-to-bugzilla → MozReview Request: Bug 1245355: Add a unit test for tabs.getAllInWindow.
Flags: needinfo?(mwein)
https://reviewboard.mozilla.org/r/40073/#review36633

That's OK with me, but this kind of change requires re-review.
https://reviewboard.mozilla.org/r/40073/#review38999

::: browser/components/extensions/ext-tabs.js
(Diff revision 4)
>            tab = TabManager.convert(extension, TabManager.getTab(context.tabId));
>          }
>          return Promise.resolve(tab);
>        },
>  
> -      getAllInWindow: function(windowId) {

This also needs to be marked "unsupported" in the schema.
Comment on attachment 8730652 [details]
MozReview Request: Bug 1245355: Add a unit test for tabs.getAllInWindow. r?kmag

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40073/diff/4-5/
Attachment #8730652 - Attachment description: MozReview Request: Bug 1245355: Add a unit test for tabs.getAllInWindow. → MozReview Request: Bug 1245355: Add a unit test for tabs.getAllInWindow. r?kmag
Keywords: checkin-needed
Whiteboard: [tabs] triaged
https://hg.mozilla.org/mozilla-central/rev/abba7d7115a4
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.