Bug 75487 – [chromium] Signal WebView to release recoverable resources if it is unlikely to be visible soon.

• Status: RESOLVED
• Resolution: INVALID
• Component: New Bugs

[Michal Mocny]

[chromium][not for review] Adding hibernateCHROMIUM extension to support more aggressive resource dealocation of tabs.

[Michal Mocny]

Created attachment 120983 [details]
Patch

[WebKit Review Bot]

Attachment 120983 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
Index mismatch: 3a7674dadd24ecd6a1b023eba203ecf2ed62fc59 != edb861612940a3244431d239dacdbc36317b627c
rereading e1e2538cff7d320c2b7cc22c0b75c9369ce9cdb2
	A	LayoutTests/svg/custom/webkit-transform-crash.html
	A	LayoutTests/svg/custom/webkit-transform-crash-expected.txt
	M	LayoutTests/ChangeLog
	M	Source/WebCore/ChangeLog
	M	Source/WebCore/svg/SVGStyledTransformableElement.cpp
103950 = 8282a1d85ad097ce4064a5896912bdb211b115e6 already exists! Why are we refetching it?
 at /usr/lib/git-core/git-svn line 5210

Died at Tools/Scripts/update-webkit line 158.


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Comment on attachment 120983 [details]
Patch

Attachment 120983 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11070156

[Michal Mocny]

Created attachment 122492 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail] before submitting because this patch contains changes to the Chromium public API.

[Darin Fisher (:fishd, Google)]

(In reply to comment #5)
> Please wait for approval from [e-mail] before submitting because this patch contains changes to the Chromium public API.

I defer to KBR for this API change.

[WebKit Review Bot]

Comment on attachment 122492 [details]
Patch

Attachment 122492 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11152746

[Michal Mocny]

I put some more thought into this over the weekend, and no longer thinking page visibility is the correct direction to go here.

"hibernation" as I've named it here is not really hibernation, it is just a signal to enter a more thorough visible(false).  The state of the tab remains simply "hidden".

Additionally, there may one day be need for a real "hibernated" state, when background tabs actually go idle and drop resources which have user visible side effects.

I'll make the relevant changes.

[Michal Mocny]

Created attachment 122662 [details]
Patch

[WebKit Review Bot]

Attachment 122662 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebKit/chromium/src/WebViewImpl.cpp:3181:  One line control clauses should not use braces.  [whitespace/braces] [4]
Total errors found: 1 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Comment on attachment 122662 [details]
Patch

Attachment 122662 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11269035

[Michal Mocny]

Comment on attachment 122662 [details]
Patch

setting r-

[Michal Mocny]

Created attachment 122975 [details]
Patch

[WebKit Review Bot]

Comment on attachment 122975 [details]
Patch

Attachment 122975 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11211758

[Michal Mocny]

Comment on attachment 122975 [details]
Patch

review- again, sorry

[Michal Mocny]

Created attachment 123180 [details]
Patch

[Michal Mocny]

Refactored to:
- remove the WGC3D changes, which are currently landing separately (see https://bugs.webkit.org/show_bug.cgi?id=76634).
- rename "UnlikelyTo.." to "releaseResources"
- finish plumbing for threaded compositor

There is a FIXME listed in this CL for updating hooks to use WGC3D releaseResources extension once that patch lands.  This patch could land independently, but since it would do nothing useful, may as well wait+update and land in one go.

Still, that change will be trivial and the bulk of the review may as well start now.

[Nat Duca]

Comment on attachment 123180 [details]
Patch

I think we're headed the wrong direction here. If I understand right, releaseResources is called via IPC from the browser process? Where? The RenderWidgetHostView?

Before we make this so drastically explicit, I think we need to consider adding resource managment extensions to GL. The GL process knows when it is interacting with a renderwidget --- renderwidgethostviews can directly IPC to the GPU process. I think it may be more effective for the RWHVs to keep the GPU process up to date on surfaces' "recentness" and "visibility" and then use that information to push a memory **limit** to each renderer-side GL context.

[Nat Duca]

What the heck? I'm so confused now. Who is using these resources? This makes no sense at all.

[Nat Duca]

SDHFSHFSJFH I see the problem. Discard framebuffer is implemented wrong. Its done as an IPC. It needs to be a command buffer operation so it executes inline with other GL commands.

[James Robinson]

Not sure I follow - flushes are IPCs and have offsets and all IPCs should be processed in order, so that in itself shouldn't be a problem. Should we take this to a crbug?

[Nat Duca]

http://chromiumcodereview.appspot.com/9910004/

^^ in progress