[webkit-reviews] review granted: [Bug 52447] [Chromium] Notify embedder and crash WebCore process immediately in case of OOM in v8 : [Attachment 78942] Patch

[bugzilla-daemon at webkit.org]

Dimitri Glazkov (Google) <dglazkov at chromium.org> has granted Yury Semikhatsky
<yurys at chromium.org>'s request for review:
Bug 52447: [Chromium] Notify embedder and crash WebCore process immediately in
case of OOM in v8
https://bugs.webkit.org/show_bug.cgi?id=52447

Attachment 78942: Patch
https://bugs.webkit.org/attachment.cgi?id=78942&action=review

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=78942&action=review

ok with comments:

> Source/WebCore/ChangeLog:8
> +	   No new tests. (OOPS!)

Can you explain why?

> Source/WebCore/ChangeLog:21
> +	   * bindings/v8/V8AbstractEventListener.cpp:
> +	   (WebCore::V8AbstractEventListener::invokeEventHandler):
> +	   * bindings/v8/V8DOMWindowShell.cpp:
> +	   (WebCore::handleFatalErrorInV8):
> +	   (WebCore::V8DOMWindowShell::initContextIfNeeded):
> +	   * bindings/v8/V8Proxy.cpp:
> +	   (WebCore::V8Proxy::runScript):
> +	   * bindings/v8/V8Proxy.h:
> +	   * bindings/v8/WorkerContextExecutionProxy.cpp:
> +	   (WebCore::WorkerContextExecutionProxy::initV8IfNeeded):
> +	   (WebCore::WorkerContextExecutionProxy::runScript):
> +	   * platform/chromium/ChromiumBridge.h:

I really prefer when the changes are documented (what/why) here to help
reviewers and archeologists.

> WebKit/chromium/ChangeLog:11
> +	   * public/WebKitClient.h:
> +	   (WebKit::WebKitClient::notifyJSOutOfMemory):
> +	   * src/ChromiumBridge.cpp:
> +	   (WebCore::ChromiumBridge::notifyJSOutOfMemory):

And here too.