Closed Bug 452485 Opened 12 years ago Closed 12 years ago

check for OOM during dom callback.

Categories

(Core :: DOM: Core & HTML, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: dougt, Assigned: dougt)

References

Details

Attachments

(3 files)

during OOM situations, we need to stop scripts from executing or face having our process killed or worse.

This is a patch that basically calls IsLowMemory() in the DOMOperationCallback.  If it is low memory, we throw up an alert, and return JS_FALSE stopping the script.
Attached patch patch v.1Splinter Review
Attachment #335766 - Flags: review?(jst)
Comment on attachment 335766 [details] [diff] [review]
patch v.1

- In nsJSContext::DOMOperationCallback():

+  // Check to see if we are running OOM
+  nsCOMPtr<nsIMemory> mem;
+  NS_GetMemoryManager(getter_AddRefs(mem));
+  if (!mem)
+    return JS_FALSE;
+
+  PRBool lowMemory;
+  mem->IsLowMemory(&lowMemory);
+  if (lowMemory) {

Have you run SunSpider or other JS benchmarks to make sure this doesn't significantly ding JS performance? I don't know how fast the above code is, but this could be called quite a number of times in loopy JS...

+      nsCOMPtr<nsIStringBundleService>
+        stringService(do_GetService(NS_STRINGBUNDLE_CONTRACTID));
+      if (!stringService)
+        return JS_TRUE;
+
+      nsCOMPtr<nsIStringBundle> bundle;
+      stringService->CreateBundle(kDOMStringBundleURL, getter_AddRefs(bundle));
+      if (!bundle)
+        return JS_TRUE;
+      
+      nsXPIDLString title, msg;
+      
+      rv = bundle->GetStringFromName(NS_LITERAL_STRING("LowMemoryTitle").get(),
+                                     getter_Copies(title));
+      rv |= bundle->GetStringFromName(NS_LITERAL_STRING("LowMemoryMessage").get(),
+                                      getter_Copies(msg));

Make this use nsContentUtils::GetLocalizedString().

r+sr=jst with the above changed, and assuming this tests out performance wise.
Attachment #335766 - Flags: superreview+
Attachment #335766 - Flags: review?(jst)
Attachment #335766 - Flags: review+
Attached file sun spider results
The "from" build is a clean firefox build from the tip today
The "to" build is from the same build with this change
this is what I plan on checking it. nsContentUtils ftw.
dougt@mozilla.com
Thu Aug 28 09:41:15 2008 -0700	a165006bcd8e
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Depends on: 490163
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.