Moving all the PlatformEvent subclasses from core/platform to platform

Moving all the PlatformEvent subclasses from core/platform to platform

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

[atreat, 2013-10-08 17:07:12]

Moving all the Platform*Event classes to Source/platform.

[abarth-chromium, 2013-10-08 17:11:54]

Please move core/platform/chromium/PlatformKeyboardEventChromium.cpp to
platform/PlatformKeyboardEvent.cpp at the same time because it implements a few
functions declared in these headers.

LGTM

https://codereview.chromium.org/26510003/diff/1/Source/platform/PlatformKeybo...
File Source/platform/PlatformKeyboardEvent.h (right):

https://codereview.chromium.org/26510003/diff/1/Source/platform/PlatformKeybo...
Source/platform/PlatformKeyboardEvent.h:63: void disambiguateKeyDownEvent(Type);
Given that this function is out-of-line, don't we need to export the class to
make it link in the component build?

class PLATFORM_EXPORT PlatformKeyboardEvent : public PlatformEvent {

https://codereview.chromium.org/26510003/diff/1/Source/platform/PlatformKeybo...
Source/platform/PlatformKeyboardEvent.h:93: protected:
We usually have a blank line above this line.

[atreat, 2013-10-08 17:39:43]

On 2013/10/08 17:11:54, abarth wrote:
> Please move core/platform/chromium/PlatformKeyboardEventChromium.cpp to
> platform/PlatformKeyboardEvent.cpp at the same time because it implements a
few
> functions declared in these headers.
> 
> LGTM
> 
>
https://codereview.chromium.org/26510003/diff/1/Source/platform/PlatformKeybo...
> File Source/platform/PlatformKeyboardEvent.h (right):
> 
>
https://codereview.chromium.org/26510003/diff/1/Source/platform/PlatformKeybo...
> Source/platform/PlatformKeyboardEvent.h:63: void
disambiguateKeyDownEvent(Type);
> Given that this function is out-of-line, don't we need to export the class to
> make it link in the component build?
> 
> class PLATFORM_EXPORT PlatformKeyboardEvent : public PlatformEvent {
> 
>
https://codereview.chromium.org/26510003/diff/1/Source/platform/PlatformKeybo...
> Source/platform/PlatformKeyboardEvent.h:93: protected:
> We usually have a blank line above this line.

Fixed.  I didn't add the export since I moved the implementation to platform as
well.  Ok to mark CQ now?

[abarth-chromium, 2013-10-08 21:13:16]

https://codereview.chromium.org/26510003/diff/6001/Source/platform/PlatformKe...
File Source/platform/PlatformKeyboardEvent.h (right):

https://codereview.chromium.org/26510003/diff/6001/Source/platform/PlatformKe...
Source/platform/PlatformKeyboardEvent.h:35: class PlatformKeyboardEvent : public
PlatformEvent {
class PLATFORM_EXPORT PlatformKeyboardEvent : public PlatformEvent {

[abarth-chromium, 2013-10-08 21:16:36]

On 2013/10/08 17:39:43, atreat wrote:
> On 2013/10/08 17:11:54, abarth wrote:
> > Please move core/platform/chromium/PlatformKeyboardEventChromium.cpp to
> > platform/PlatformKeyboardEvent.cpp at the same time because it implements a
> few
> > functions declared in these headers.
> > 
> > LGTM
> > 
> >
>
https://codereview.chromium.org/26510003/diff/1/Source/platform/PlatformKeybo...
> > File Source/platform/PlatformKeyboardEvent.h (right):
> > 
> >
>
https://codereview.chromium.org/26510003/diff/1/Source/platform/PlatformKeybo...
> > Source/platform/PlatformKeyboardEvent.h:63: void
> disambiguateKeyDownEvent(Type);
> > Given that this function is out-of-line, don't we need to export the class
to
> > make it link in the component build?
> > 
> > class PLATFORM_EXPORT PlatformKeyboardEvent : public PlatformEvent {
> > 
> >
>
https://codereview.chromium.org/26510003/diff/1/Source/platform/PlatformKeybo...
> > Source/platform/PlatformKeyboardEvent.h:93: protected:
> > We usually have a blank line above this line.
> 
> Fixed.  I didn't add the export since I moved the implementation to platform
as
> well.  Ok to mark CQ now?

The callers are outside of platform, which means you need to export the
functions so they can call them.

[abarth-chromium, 2013-10-08 21:17:35]

You'll also need to export PlatformEvent because it's a base class.  You might
need to create a PlatformEvent.cpp to hold PlatformEvent::~PlatformEvent to make
windows happy.

[atreat, 2013-10-08 22:00:30]

On 2013/10/08 21:17:35, abarth wrote:
> You'll also need to export PlatformEvent because it's a base class.  You might
> need to create a PlatformEvent.cpp to hold PlatformEvent::~PlatformEvent to
make
> windows happy.

All fixed up.  I added exports to PlatformKeyboardEvent and PlatformEvent and
made PlatformEvent.cpp as you suggested.  Local build is fine on linux desktop
and binary runs.

[jamesr, 2013-10-08 22:05:05]

https://codereview.chromium.org/26510003/diff/14001/Source/platform/PlatformE...
File Source/platform/PlatformEvent.h (right):

https://codereview.chromium.org/26510003/diff/14001/Source/platform/PlatformE...
Source/platform/PlatformEvent.h:133: ~PlatformEvent();
why do you need to make the destructor out-of-line? it'll be trivial

[abarth-chromium, 2013-10-08 22:06:06]

On 2013/10/08 22:05:05, jamesr wrote:
> why do you need to make the destructor out-of-line? it'll be trivial

Sorry, I asked him to do that when I thought we needed to export PlatformEvent.

[abarth-chromium, 2013-10-08 22:06:58]

On 2013/10/08 22:06:06, abarth wrote:
> On 2013/10/08 22:05:05, jamesr wrote:
> > why do you need to make the destructor out-of-line? it'll be trivial
> 
> Sorry, I asked him to do that when I thought we needed to export
PlatformEvent.

Looks like he did export PlatformEvent.  Don't we need a compilation unit to
hold the exported symbols?

[jamesr, 2013-10-08 22:15:31]

On 2013/10/08 22:06:58, abarth wrote:
> On 2013/10/08 22:06:06, abarth wrote:
> > On 2013/10/08 22:05:05, jamesr wrote:
> > > why do you need to make the destructor out-of-line? it'll be trivial
> > 
> > Sorry, I asked him to do that when I thought we needed to export
> PlatformEvent.
> 
> Looks like he did export PlatformEvent.  Don't we need a compilation unit to
> hold the exported symbols?

If the d'tor is inline, as it was before, then PlatformEvent doesn't have any
exported symbols (and also doesn't need to be marked as exported)

[atreat, 2013-10-09 14:58:53]

On 2013/10/08 22:15:31, jamesr wrote:
> If the d'tor is inline, as it was before, then PlatformEvent doesn't have any
> exported symbols (and also doesn't need to be marked as exported)

Ok, I have removed the export of PlatformEvent and only exported those methods
in PlatformKeyboardEvent that are not inline.

Thanks for helping me sort this out guys!

[abarth-chromium, 2013-10-09 15:00:16]

LGTM assuming it builds.  :)

[commit-bot: I haz the power, 2013-10-09 15:00:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/adam.treat@samsung.com/26510003/22001

[commit-bot: I haz the power, 2013-10-09 15:00:58]

Failed to apply patch for Source/core/events/KeyboardEvent.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/core/events/KeyboardEvent.cpp
  Hunk #1 FAILED at 24.
  1 out of 1 hunk FAILED -- saving rejects to file
Source/core/events/KeyboardEvent.cpp.rej

Patch:       Source/core/events/KeyboardEvent.cpp
Index: Source/core/events/KeyboardEvent.cpp
diff --git a/Source/core/events/KeyboardEvent.cpp
b/Source/core/events/KeyboardEvent.cpp
index
b71008798b04580e5277089c10e44d1a7c7ca068..3b9f91725bee92b84259be63b4097af001baee58
100644
--- a/Source/core/events/KeyboardEvent.cpp
+++ b/Source/core/events/KeyboardEvent.cpp
@@ -24,8 +24,8 @@
 #include "core/events/KeyboardEvent.h"
 
 #include "core/events/EventNames.h"
-#include "core/platform/PlatformKeyboardEvent.h"
 #include "core/platform/WindowsKeyboardCodes.h"
+#include "platform/PlatformKeyboardEvent.h"
 
 namespace WebCore {

[commit-bot: I haz the power, 2013-10-09 16:45:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/adam.treat@samsung.com/26510003/27001

[commit-bot: I haz the power, 2013-10-09 16:46:12]

Failed to apply patch for Source/core/platform/ScrollbarThemeAuraOrGtk.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/core/platform/ScrollbarThemeAuraOrGtk.cpp
  Hunk #1 FAILED at 32.
  1 out of 1 hunk FAILED -- saving rejects to file
Source/core/platform/ScrollbarThemeAuraOrGtk.cpp.rej

Patch:       Source/core/platform/ScrollbarThemeAuraOrGtk.cpp
Index: Source/core/platform/ScrollbarThemeAuraOrGtk.cpp
diff --git a/Source/core/platform/ScrollbarThemeAuraOrGtk.cpp
b/Source/core/platform/ScrollbarThemeAuraOrGtk.cpp
index
88791dfcbae956789b461a0b048f5f435fd1407e..242c26045f13ccc193164a44c0aa6f5d577fe71c
100644
--- a/Source/core/platform/ScrollbarThemeAuraOrGtk.cpp
+++ b/Source/core/platform/ScrollbarThemeAuraOrGtk.cpp
@@ -32,10 +32,10 @@
 #include "core/platform/ScrollbarThemeAuraOrGtk.h"
 
 #include "RuntimeEnabledFeatures.h"
-#include "core/platform/PlatformMouseEvent.h"
 #include "core/platform/Scrollbar.h"
 #include "core/platform/ScrollbarThemeOverlay.h"
 #include "core/platform/graphics/GraphicsContext.h"
+#include "platform/PlatformMouseEvent.h"
 #include "public/platform/Platform.h"
 #include "public/platform/WebRect.h"
 #include "public/platform/default/WebThemeEngine.h"

[commit-bot: I haz the power, 2013-10-09 17:04:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/adam.treat@samsung.com/26510003/31001

[commit-bot: I haz the power, 2013-10-09 19:53:03]

Retried try job too often on win_blink_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_blink_...

[commit-bot: I haz the power, 2013-10-09 19:59:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/adam.treat@samsung.com/26510003/31001

[commit-bot: I haz the power, 2013-10-09 22:15:48]

Change committed as 159263