Bug 28776

Summary:

[v8] Add support for newly added lowMemoryNotification() API in V8

Product:

WebKit

Reporter:

Andrei Popescu <andreip>

Component:

WebCore Misc.

Assignee:

Ben Murdoch <benm>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, benm, eric, levin, mrowe

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

Other

Attachments:

Description	Flags
Add "LowMemoryNotification()" method on the ScriptController class.	levin: review-
Add "LowMemoryNotification()" method on the ScriptController class (take 2)	levin: review+, eric: commit-queue-

Andrei Popescu

Reported 2009-08-27 10:53:36 PDT

V8 added a new API for Android's benefit. http://code.google.com/p/v8/source/detail?spec=svn2777&r=2725 We access V8 via the ScriptController class so the new V8 methods need to be proxied through this class, as well.

Attachments
Add "LowMemoryNotification()" method on the ScriptController class. (2.19 KB, patch) 2009-08-27 10:56 PDT, Andrei Popescu	levin: review-	Details Formatted Diff Diff
Add "LowMemoryNotification()" method on the ScriptController class (take 2) (1.99 KB, patch) 2009-09-01 04:58 PDT, Andrei Popescu	levin: review+ eric: commit-queue-	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Andrei Popescu

Comment 1 2009-08-27 10:56:57 PDT

Created attachment 38674 [details] Add "LowMemoryNotification()" method on the ScriptController class.

Mark Rowe (bdash)

Comment 2 2009-08-30 18:10:10 PDT

What is the point of adding ScriptController::lowMemoryNotification given that nothing calls it?

Eric Seidel (no email)

Comment 3 2009-08-31 03:38:24 PDT

I assume this is called by some other Andriod code? Why wouldn't that code just call the v8 API directly?

David Levin

Comment 4 2009-08-31 09:19:35 PDT

Comment on attachment 38674 [details] Add "LowMemoryNotification()" method on the ScriptController class. Marking r- based on previous comments in bug that need an answer. Also, note that chromium has moved past that revision, so there is no need for that if PLATFORM. http://src.chromium.org/viewvc/chrome/trunk/src/DEPS?view=markup

Andrei Popescu

Comment 5 2009-09-01 04:18:10 PDT

(In reply to comment #2) > What is the point of adding ScriptController::lowMemoryNotification given that > nothing calls it? Android code calls it. We haven't yet sent the patches that call that code, but we plan to do so very soon.

Andrei Popescu

Comment 6 2009-09-01 04:20:46 PDT

(In reply to comment #3) > I assume this is called by some other Andriod code? Why wouldn't that code > just call the v8 API directly? Android doesn't really know anything about V8, we just call methods on the script controller. I am not sure I understand why would we make an exception in this case?

Andrei Popescu

Comment 7 2009-09-01 04:58:27 PDT

Created attachment 38859 [details] Add "LowMemoryNotification()" method on the ScriptController class (take 2)

Eric Seidel (no email)

Comment 8 2009-09-01 09:18:22 PDT

Comment on attachment 38859 [details] Add "LowMemoryNotification()" method on the ScriptController class (take 2) Rejecting patch 38859 from commit-queue. This patch will require manual commit. ['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1'] failed with exit code 1

Ben Murdoch

Comment 9 2009-09-01 10:05:40 PDT

I will land.

Andrei Popescu

Comment 10 2009-09-01 10:14:35 PDT

(In reply to comment #8) > (From update of attachment 38859 [details]) > Rejecting patch 38859 from commit-queue. This patch will require manual > commit. > > ['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', > '--exit-after-n-failures=1'] failed with exit code 1 Does this mean my patch causes a layout test failure on OSX? That surely can't be since it only touches two files in bindings/v8... Anyway, Ben's looking at it.

Adam Barth

Comment 11 2009-09-01 10:19:21 PDT

> Does this mean my patch causes a layout test failure on OSX? That surely can't > be since it only touches two files in bindings/v8... The commit queue might be horked, or it could be flakiness. I doubt it's a problem with your patch.

Ben Murdoch

Comment 12 2009-09-01 11:05:07 PDT

Landed as r47942. Sending WebCore/ChangeLog Sending WebCore/bindings/v8/ScriptController.cpp Sending WebCore/bindings/v8/ScriptController.h Transmitting file data ... Committed revision 47942.

Eric Seidel (no email)

Comment 13 2009-09-01 16:35:43 PDT

media/video-source-add-src.html -> timed out Your patch was yet another victim of bug 28845. :(

Note You need to log in before you can comment on or make changes to this bug.