Adding monotonic only API to Blink's animation delegate interface

public/platform/WebAnimationDelegate.h

 /*
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
  *     notice, this list of conditions and the following disclaimer in the
  *     documentation and/or other materials provided with the distribution.
  *
  * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' AND ANY
  * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
  * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
  * DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS BE LIABLE FOR ANY
  * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
  * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
  * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
  * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
  * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #ifndef WebAnimationDelegate_h
 #define WebAnimationDelegate_h
 
 #include "WebAnimation.h"
+#include "WebCommon.h"
 
 #define WEB_ANIMATION_DELEGATE_TAKES_MONOTONIC_TIME 1
 
 namespace blink {
 
-class WebAnimationDelegate {
+class BLINK_PLATFORM_EXPORT WebAnimationDelegate {

[jamesr, 2014/03/04 21:38:37]

why does this class need to be exported? i don't see any symbols on this class
that would require it - only pure virtual functions and inline functions.  If
this is supposed to be exported, where is the object file providing the
definition of the symbols you are exporting?

[mithro-old, 2014/03/04 23:54:24]

The two methods I add below with inline definitions cause it to need the
BLINK_PLATFORM_EXPORT declaration otherwise Chrome on Windows fails to link
with;
---------------------
FAILED: ninja -t msvc -e environment.x86 -- E:\b\build\goma\gomacc.exe
"E:\b\depot_tools\win_toolchain\vs2013_files\VC\bin\amd64_x86\cl.exe" /nologo
/showIncludes /FC
@obj\third_party\WebKit\Source\platform\blink_platform.OverscrollTheme.obj.rsp
/c ..\..\third_party\WebKit\Source\platform\OverscrollTheme.cpp
/Foobj\third_party\WebKit\Source\platform\blink_platform.OverscrollTheme.obj
/Fdobj\third_party\WebKit\Source\platform\blink_platform.cc.pdb 
e:\b\build\slave\win\build\src\third_party\webkit\source\platform\graphics\graphicslayer.h(81)
: error C2220: warning treated as error - no 'object' file generated
e:\b\build\slave\win\build\src\third_party\webkit\source\platform\graphics\graphicslayer.h(81)
: warning C4275: non dll-interface class 'blink::WebAnimationDelegate' used as
base for dll-interface class 'WebCore::GraphicsLayer'
       
e:\b\build\slave\win\build\src\third_party\webkit\public\platform\webanimationdelegate.h(34)
: see declaration of 'blink::WebAnimationDelegate'
       
e:\b\build\slave\win\build\src\third_party\webkit\source\platform\graphics\graphicslayer.h(81)
: see declaration of 'WebCore::GraphicsLayer'
ninja: build stopped: subcommand failed.
---------------------

The suggested fix on #blink was adding a BLINK_PLATFORM_EXPORT.

 public:
-    // FIXME: Remove wallClockTime once the legacy implementation of CSS
-    // animations and transitions is removed.
-    virtual void notifyAnimationStarted(double wallClockTime, double monotonicTime, WebAnimation::TargetProperty) = 0;
-    virtual void notifyAnimationFinished(double wallClockTime, double monotonicTime, WebAnimation::TargetProperty) = 0;
+    // FIXME: Remove wallClockTime API after the following file is updated;
+    // webkit/renderer/compositor_bindings/web_to_cc_animation_delegate_adapter.cc
+    inline void notifyAnimationStarted(double wallClockTime, double monotonicTime, WebAnimation::TargetProperty prop)

[jamesr, 2014/03/04 21:38:37]

i don't think the "inline" annotation provides any value

don't you need to mark these virtual so you can override them on the chromium
side?

the parameter name 'prop' provides no value here and should be omitted per Blink
style

[mithro-old, 2014/03/04 23:54:24]

Fixed.
This class is only inherited from in Blink, Chrome only calls the interface.

These non-virtual methods are only being added so I don't need a two-sided
patch. Making these methods non-virtual means that every inheritor inside Blink
is statically checked to be using the non-walltime API. These methods will be
removed in https://codereview.chromium.org/178103004/ which will be submitted
after https://codereview.chromium.org/185643002/ lands and deps are rolled.

FYI the only Chrome file which references this file/class is 
webkit/renderer/compositor_bindings/web_to_cc_animation_delegate_adapter.cc and
I've checked that chrome successfully links on Windows, Mac and Linux with this
CL patched.
It needs a name so I can pass the value around?

+    {
+        notifyAnimationStarted(monotonicTime, prop);
+    }
+    inline void notifyAnimationFinished(double wallClockTime, double monotonicTime, WebAnimation::TargetProperty prop)
+    {
+        notifyAnimationFinished(monotonicTime, prop);
+    }
+    virtual void notifyAnimationStarted(double monotonicTime, WebAnimation::TargetProperty) = 0;
+    virtual void notifyAnimationFinished(double monotonicTime, WebAnimation::TargetProperty) = 0;
 };
 
 } // namespace blink
 
 #endif // WebAnimationDelegate_h