Use method registration approach for creating events on core and modules.

Use method registration approach for creating events on core and modules.

BUG=376506
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175179

[eseidel, 2014-05-28 22:04:22]

lgtm

https://codereview.chromium.org/305723002/diff/40001/Source/modules/EventModu...
File Source/modules/EventModulesFactory.h (right):

https://codereview.chromium.org/305723002/diff/40001/Source/modules/EventModu...
Source/modules/EventModulesFactory.h:19: virtual PassRefPtrWillBeRawPtr<Event>
create(const String& eventType) OVERRIDE;
Surprised this isn't an AtomicString.  I guess we create events on non-main
threads?

https://codereview.chromium.org/305723002/diff/40001/Source/modules/InitModul...
File Source/modules/InitModules.cpp (right):

https://codereview.chromium.org/305723002/diff/40001/Source/modules/InitModul...
Source/modules/InitModules.cpp:31: Document::registerEventFactory(new
EventModulesFactory());
create() functions are generally nicer than new, certainly in the case of
RefCounted.  I guess this is going away with oilpan though, so maybe that
doens't matter anymore.  This is OK.

[eseidel, 2014-05-28 22:04:41]

It would be nice to ensure/ASSERT that two event factories don't have
overlapping events they support.

[c.shu, 2014-05-28 22:25:48]

On 2014/05/28 22:04:41, eseidel wrote:
> It would be nice to ensure/ASSERT that two event factories don't have
> overlapping events they support.

The create() function is generated from EventFactory.in and
EventModulesFactory.in. As long as they don't have overlapping events, it should
be fine. I am not sure how to add this ASSERT in the code though...(Any
eventtype goes through the 1st create and possibly the 2nd one.)

[c.shu, 2014-05-29 15:06:49]

https://codereview.chromium.org/305723002/diff/40001/Source/modules/EventModu...
File Source/modules/EventModulesFactory.h (right):

https://codereview.chromium.org/305723002/diff/40001/Source/modules/EventModu...
Source/modules/EventModulesFactory.h:19: virtual PassRefPtrWillBeRawPtr<Event>
create(const String& eventType) OVERRIDE;
On 2014/05/28 22:04:23, eseidel wrote:
> Surprised this isn't an AtomicString.  I guess we create events on non-main
> threads?

Not sure. Maybe worth investigation.

https://codereview.chromium.org/305723002/diff/40001/Source/modules/InitModul...
File Source/modules/InitModules.cpp (right):

https://codereview.chromium.org/305723002/diff/40001/Source/modules/InitModul...
Source/modules/InitModules.cpp:31: Document::registerEventFactory(new
EventModulesFactory());
On 2014/05/28 22:04:23, eseidel wrote:
> create() functions are generally nicer than new, certainly in the case of
> RefCounted.  I guess this is going away with oilpan though, so maybe that
> doens't matter anymore.  This is OK.

Ok. thanks.

[c.shu, 2014-05-29 15:19:30]

The CQ bit was checked by c.shu@samsung.com

[commit-bot: I haz the power, 2014-05-29 15:21:33]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/c.shu@samsung.com/305723002/40001

[c.shu, 2014-05-30 14:39:31]

The CQ bit was unchecked by c.shu@samsung.com

[c.shu, 2014-05-30 14:39:42]

The CQ bit was checked by c.shu@samsung.com

[commit-bot: I haz the power, 2014-05-30 14:39:58]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/c.shu@samsung.com/305723002/40001

[commit-bot: I haz the power, 2014-05-30 14:40:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-30 14:40:13]

Failed to apply patch for Source/bindings/v8/custom/V8DocumentCustom.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/bindings/v8/custom/V8DocumentCustom.cpp
  Hunk #3 FAILED at 83.
  1 out of 3 hunks FAILED -- saving rejects to file
Source/bindings/v8/custom/V8DocumentCustom.cpp.rej

Patch:       Source/bindings/v8/custom/V8DocumentCustom.cpp
Index: Source/bindings/v8/custom/V8DocumentCustom.cpp
diff --git a/Source/bindings/v8/custom/V8DocumentCustom.cpp
b/Source/bindings/v8/custom/V8DocumentCustom.cpp
index
fcf60021ebac6970a3d35d7b4faaad660684f808..f88483279132c0223f851d1cfb4071f7071b8dcd
100644
--- a/Source/bindings/v8/custom/V8DocumentCustom.cpp
+++ b/Source/bindings/v8/custom/V8DocumentCustom.cpp
@@ -33,7 +33,6 @@
 
 #include "bindings/core/v8/V8CanvasRenderingContext2D.h"
 #include "bindings/core/v8/V8DOMImplementation.h"
-#include "bindings/core/v8/V8Event.h"
 #include "bindings/core/v8/V8Node.h"
 #include "bindings/core/v8/V8Touch.h"
 #include "bindings/core/v8/V8TouchList.h"
@@ -55,7 +54,6 @@
 #include "core/xml/DocumentXPathEvaluator.h"
 #include "core/xml/XPathNSResolver.h"
 #include "core/xml/XPathResult.h"
-#include "modules/InitModules.h"
 #include "wtf/RefPtr.h"
 
 namespace WebCore {
@@ -85,26 +83,4 @@ void V8Document::evaluateMethodCustom(const
v8::FunctionCallbackInfo<v8::Value>&
     v8SetReturnValueFast(info, result.release(), document.get());
 }
 
-// Customize createEvent so it can call createEventModules in modules.
-// FIXME: Use method registration approach instead. http://crbug.com/358074
-void V8Document::createEventMethodCustom(const
v8::FunctionCallbackInfo<v8::Value>& info)
-{
-    ExceptionState exceptionState(ExceptionState::ExecutionContext,
"createEvent", "Document", info.Holder(), info.GetIsolate());
-    if (UNLIKELY(info.Length() < 1)) {
-        throwMinimumArityTypeError(exceptionState, 1, info.Length());
-        return;
-    }
-    Document* impl = V8Document::toNative(info.Holder());
-    V8StringResource<> eventType;
-    {
-        TOSTRING_VOID_INTERNAL_NOTRYCATCH(eventType, info[0]);
-    }
-    RefPtrWillBeRawPtr<Event> result = createEventModules(eventType,
exceptionState);
-    if (exceptionState.hadException()) {
-        exceptionState.throwIfNeeded();
-        return;
-    }
-    v8SetReturnValueFast(info, WTF::getPtr(result.release()), impl);
-}
-
 } // namespace WebCore

[c.shu, 2014-05-30 15:52:58]

The CQ bit was unchecked by c.shu@samsung.com

[c.shu, 2014-05-30 15:56:04]

The CQ bit was checked by c.shu@samsung.com

[commit-bot: I haz the power, 2014-05-30 15:56:35]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/c.shu@samsung.com/305723002/60001

[commit-bot: I haz the power, 2014-05-30 17:20:00]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/6531)

[commit-bot: I haz the power, 2014-05-30 18:30:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-30 18:30:45]

Try jobs failed on following builders:
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/6535)

[c.shu, 2014-05-30 23:58:01]

The CQ bit was unchecked by c.shu@samsung.com

[c.shu, 2014-05-30 23:58:14]

The CQ bit was checked by c.shu@samsung.com

[commit-bot: I haz the power, 2014-05-31 00:00:17]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/c.shu@samsung.com/305723002/60001

[commit-bot: I haz the power, 2014-05-31 01:05:00]

Change committed as 175179