Closed
Bug 1468099
Opened 6 years ago
Closed 6 years ago
Add a way to check if all the tabs in a process are in background, and whether all those can be throttled
Categories
(Core :: DOM: Core & HTML, enhancement)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla62
Tracking | Status | |
---|---|---|
firefox62 | --- | fixed |
People
(Reporter: smaug, Assigned: smaug)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
farre
:
review+
|
Details | Diff | Splinter Review |
Throttling check could use TimeoutManager::BudgetThrottlingEnabled.
We need to make TabChild a linked list or something.
(Somewhat similar TabChild::HasVisibleTabs() is very broken. It was added for Quantum DOM)
Assignee | ||
Comment 1•6 years ago
|
||
I guess I want to be able to iterate TabGroups, not TabChilds since TabChilds may share the same TabGroup
Assignee | ||
Comment 2•6 years ago
|
||
I tested this locally.
TabGroupList() isn't used by the patch, but I could imagine code like
https://searchfox.org/mozilla-central/rev/bf81d741ff5dd11bb364ef21306da599032fd479/toolkit/components/perfmonitoring/PerformanceUtils.cpp#25 should use it.
(that code is currently wrong since it may iterate same tabgroup several times.)
Attachment #8985045 -
Flags: review?(afarre)
Updated•6 years ago
|
Attachment #8985045 -
Flags: review?(afarre) → review+
Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7fc2698f9e19
Add a way to check if all the tabs in a process can be throttled, r=farre
Comment 4•6 years ago
|
||
Backed out changeset 7fc2698f9e19 (bug 1468099) for gtest failures on a CLOSED TREE
Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/354df588746a
Failure push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=7fc2698f9e19d42e59c3ce640dba0667801fd8f2&selectedJob=183272557
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=183272557&repo=mozilla-inbound&lineNumber=721
INFO - Moving /Users/cltbld/tasks/task_1529022816/build/tests/bin/plugins/gmp-clearkey to /Users/cltbld/tasks/task_1529022816/build/application/Firefox NightlyDebug.app/Contents/Resources
17:36:12 ERROR - shutil error: Destination path '/Users/cltbld/tasks/task_1529022816/build/application/Firefox NightlyDebug.app/Contents/Resources/gmp-clearkey' already exists
17:36:12 INFO - Moving /Users/cltbld/tasks/task_1529022816/build/tests/bin/plugins/gmp-fake to /Users/cltbld/tasks/task_1529022816/build/application/Firefox NightlyDebug.app/Contents/Resources
17:36:12 INFO - Moving /Users/cltbld/tasks/task_1529022816/build/tests/bin/plugins/gmp-fakeopenh264 to /Users/cltbld/tasks/task_1529022816/build/application/Firefox NightlyDebug.app/Contents/Resources
17:36:12 INFO - Moving /Users/cltbld/tasks/task_1529022816/build/tests/gtest/dependentlibs.list.gtest to /Users/cltbld/tasks/task_1529022816/build/application/Firefox NightlyDebug.app/Contents/Resources
17:36:12 INFO - copying tree: /Users/cltbld/tasks/task_1529022816/build/tests/gtest/gtest_bin to /Users/cltbld/tasks/task_1529022816/build/application/Firefox NightlyDebug.app/Contents/MacOS
17:36:12 INFO - mkdir: /Users/cltbld/tasks/task_1529022816/build/application/Firefox NightlyDebug.app/Contents/MacOS/gtest
17:36:12 INFO - copying tree: /Users/cltbld/tasks/task_1529022816/build/tests/gtest/gtest_bin/gtest to /Users/cltbld/tasks/task_1529022816/build/application/Firefox NightlyDebug.app/Contents/MacOS/gtest
17:36:12 INFO - rmtree: /Users/cltbld/tasks/task_1529022816/build/application/Firefox NightlyDebug.app/Contents/MacOS/gtest
17:36:12 INFO - retry: Calling rmtree with args: ('/Users/cltbld/tasks/task_1529022816/build/application/Firefox NightlyDebug.app/Contents/MacOS/gtest',), kwargs: {}, attempt #1
17:36:12 INFO - Running post-action listener: _resource_record_post_action
17:36:12 INFO - [mozharness: 2018-06-15 00:36:12.570973Z] Finished stage-files step (success)
17:36:12 INFO - [mozharness: 2018-06-15 00:36:12.571112Z] Running run-tests step.
17:36:12 INFO - Running pre-action listener: _resource_record_pre_action
17:36:12 INFO - Running pre-action listener: _set_gcov_prefix
17:36:12 INFO - Running main action method: run_tests
17:36:12 INFO - mkdir: /Users/cltbld/tasks/task_1529022816/build/blobber_upload_dir
17:36:12 INFO - #### Running gtest suites
Flags: needinfo?(bugs)
Assignee | ||
Comment 5•6 years ago
|
||
hmm, that means there is some unrelated leak somewhere.
Flags: needinfo?(bugs)
Assignee | ||
Comment 6•6 years ago
|
||
Yeah, our gtest setup is broken. It doesn't detect leaks, and LinkedList implementation happens to assert hard about leaks.
I'll change the patch a bit to hack around this issue.
Assignee | ||
Comment 7•6 years ago
|
||
same but not using statically allocated LinkedList to avoid the shutdown assertion in gtest.
remote: View your changes here:
remote: https://hg.mozilla.org/try/rev/20ab1c2259176b005113156759fc455aa4501258
remote: https://hg.mozilla.org/try/rev/6bfed2badaa331c483c731fe4ae2d4c999341f3e
remote: https://hg.mozilla.org/try/rev/8e1ccac26253886ca224fb95c9a612d3ee267c72
remote: https://hg.mozilla.org/try/rev/d7268334b33f28e05eba00aae019839247579fb0
remote:
remote: Follow the progress of your build on Treeherder:
remote: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d7268334b33f28e05eba00aae019839247579fb0
remote: recorded changegroup in replication log in 0.113s
Assignee: nobody → bugs
Attachment #8985045 -
Attachment is obsolete: true
Attachment #8985577 -
Flags: review?(afarre)
Comment 8•6 years ago
|
||
Comment on attachment 8985577 [details] [diff] [review]
throttable_tabs_2.diff
Review of attachment 8985577 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/base/TabGroup.cpp
@@ +23,5 @@
> namespace dom {
>
> static StaticRefPtr<TabGroup> sChromeTabGroup;
>
> +LinkedList<TabGroup>* TabGroup::sTabGroups = nullptr;
So, how about StaticAutoPtr<LinkedList<TabGroup>> here instead?
@@ +69,5 @@
> +
> + if (sTabGroups->isEmpty()) {
> + delete sTabGroups;
> + sTabGroups = nullptr;
> + }
With StaticAutoPtr the need for deleting sTabGroups goes away.
::: dom/base/TabGroup.h
@@ +178,5 @@
> DocGroupMap mDocGroups;
> nsTArray<nsPIDOMWindowOuter*> mWindows;
> uint32_t mForegroundCount;
> +
> + static LinkedList<TabGroup>* sTabGroups;
See comment about StaticAutoPtr
Updated•6 years ago
|
Flags: needinfo?(bugs)
Assignee | ||
Comment 9•6 years ago
|
||
StaticAutoPtr explicitly deletes LinkedList, and apparently gtests don't care about leaks, so we'd see the same assertion about list not being empty.
Need to explicitly delete the list only when the last TabGroup goes away
(or fix all the gtest stuff to not leak ever)
Flags: needinfo?(bugs)
Updated•6 years ago
|
Attachment #8985577 -
Flags: review?(afarre) → review+
Comment 10•6 years ago
|
||
Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1950fa531169
Add a way to check if all the tabs in a process can be throttled, r=farre
Comment 11•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•