Bug 78282 - Refactor Mac platform implementation of the Pasteboard class to use Platform Strategies
: Refactor Mac platform implementation of the Pasteboard class to use Platform ...
Status: RESOLVED FIXED
: WebKit
HTML Editing
: 528+ (Nightly build)
: Macintosh Unspecified
: P2 Normal
Assigned To:
:
:
: 78410
: 77259
  Show dependency treegraph
 
Reported: 2012-02-09 14:19 PST by
Modified: 2012-02-13 14:45 PST (History)


Attachments
patch (70.95 KB, patch)
2012-02-09 15:43 PST, Enrica Casucci
andersca: review+
Review Patch | Details | Formatted Diff | Diff
Patch to verify the Qt build fix (76.82 KB, patch)
2012-02-13 13:39 PST, Enrica Casucci
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-02-09 14:19:45 PST
This bug track another step in the direction of removing access to NSPasteboard from the WebProcess.
The Mac implementation of the Pasteboard class needs to be redesigned to use a platform strategy that can provide different implementations for WebKit and WebKit2.
------- Comment #1 From 2012-02-09 15:43:06 PST -------
Created an attachment (id=126392) [details]
patch
------- Comment #2 From 2012-02-11 00:48:02 PST -------
I rolled it out, because it broke the Qt build - http://trac.webkit.org/changeset/107475

../../../Source/WebKit/qt/WebCoreSupport/PlatformStrategiesQt.cpp: In static member function 'static void PlatformStrategiesQt::initialize()':
../../../Source/WebKit/qt/WebCoreSupport/PlatformStrategiesQt.cpp:51: error: cannot allocate an object of abstract type 'PlatformStrategiesQt'
../../../Source/WebKit/qt/WebCoreSupport/PlatformStrategiesQt.h:41: note:   because the following virtual functions are pure within 'PlatformStrategiesQt':
../../../Source/WebCore/platform/PlatformStrategies.h:95: note:     virtual WebCore::PasteboardStrategy* WebCore::PlatformStrategies::createPasteboardStrategy()

Next time please use EWS bots or watch the bots after landing to ensure that your patch builds on all platform.

http://www.webkit.org/coding/contributing.html
"Your change must at least compile on all platforms."

Qt build was broken during ~70 revision because of this patch. :( It means that the test coverage was 0 in this time. And unfortunately somebody broke all JSC tests on Qt-debug during build breakage. It won't be a good game to find it manually ...
------- Comment #3 From 2012-02-11 06:49:42 PST -------
(In reply to comment #2)
> I rolled it out, because it broke the Qt build - http://trac.webkit.org/changeset/107475
> 
> ../../../Source/WebKit/qt/WebCoreSupport/PlatformStrategiesQt.cpp: In static member function 'static void PlatformStrategiesQt::initialize()':
> ../../../Source/WebKit/qt/WebCoreSupport/PlatformStrategiesQt.cpp:51: error: cannot allocate an object of abstract type 'PlatformStrategiesQt'
> ../../../Source/WebKit/qt/WebCoreSupport/PlatformStrategiesQt.h:41: note:   because the following virtual functions are pure within 'PlatformStrategiesQt':
> ../../../Source/WebCore/platform/PlatformStrategies.h:95: note:     virtual WebCore::PasteboardStrategy* WebCore::PlatformStrategies::createPasteboardStrategy()
> 
> Next time please use EWS bots or watch the bots after landing to ensure that your patch builds on all platform.
> 
> http://www.webkit.org/coding/contributing.html
> "Your change must at least compile on all platforms."
> 
> Qt build was broken during ~70 revision because of this patch. :( It means that the test coverage was 0 in this time. And unfortunately somebody broke all JSC tests on Qt-debug during build breakage. It won't be a good game to find it manually ...

My apologies, I did not notice that the Qt port was using platform strategies. I will resubmit my patch with the build fix for Qt.
------- Comment #4 From 2012-02-13 13:39:24 PST -------
Created an attachment (id=126819) [details]
Patch to verify the Qt build fix
------- Comment #5 From 2012-02-13 13:41:09 PST -------
Attachment 126819 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/platform/PlatformPasteboard.h:53:  The parameter name "string" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/platform/PasteboardStrategy.h:50:  The parameter name "string" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit/mac/WebCoreSupport/WebPlatformStrategies.h:67:  The parameter name "buffer" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit/mac/WebCoreSupport/WebPlatformStrategies.h:69:  The parameter name "string" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit2/WebProcess/WebCoreSupport/WebPlatformStrategies.h:75:  The parameter name "string" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/platform/Pasteboard.h:33:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 6 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.
------- Comment #6 From 2012-02-13 14:45:51 PST -------
http://trac.webkit.org/changeset/107606