Bug 40279 - Change OwnPtrCommon to include platform-specific headers
Summary: Change OwnPtrCommon to include platform-specific headers
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 40305 41208
Blocks:
  Show dependency treegraph
 
Reported: 2010-06-07 20:50 PDT by Kwang Yul Seo
Modified: 2015-05-07 16:54 PDT (History)
7 users (show)

See Also:


Attachments
Patch (6.59 KB, patch)
2010-06-07 20:53 PDT, Kwang Yul Seo
abarth: review-
Details | Formatted Diff | Diff
Revised patch (7.14 KB, patch)
2010-06-07 22:29 PDT, Kwang Yul Seo
no flags Details | Formatted Diff | Diff
Revised patch (build fix) (14.45 KB, patch)
2010-06-09 04:46 PDT, Kwang Yul Seo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kwang Yul Seo 2010-06-07 20:50:39 PDT
Adding new type to OwnPtrCommon needlessly causes all ports to do full rebuilds. Change OwnPtrCommon to include platform-specific headers to avoid all ports rebuilds.
Comment 1 Kwang Yul Seo 2010-06-07 20:53:03 PDT
Created attachment 58106 [details]
Patch
Comment 2 Adam Barth 2010-06-07 21:53:15 PDT
Comment on attachment 58106 [details]
Patch

Don't you need to make some changes to a build system?
Comment 3 Kwang Yul Seo 2010-06-07 22:08:01 PDT
(In reply to comment #2)
> (From update of attachment 58106 [details])
> Don't you need to make some changes to a build system?

I wanted to check it with Windows build bot.
Comment 4 Adam Barth 2010-06-07 22:14:11 PDT
It will probably build on Windows without being added to the build system, but you probably want to add them anyway.  You can just look at the vcproj files and infer the structure.
Comment 5 Kwang Yul Seo 2010-06-07 22:29:22 PDT
Created attachment 58110 [details]
Revised patch

Add OwnPtrWin.h to WTF.vcproj.
Comment 6 Adam Barth 2010-06-07 22:32:28 PDT
Comment on attachment 58110 [details]
Revised patch

Great!  Thanks.
Comment 7 WebKit Commit Bot 2010-06-08 01:35:22 PDT
Comment on attachment 58110 [details]
Revised patch

Clearing flags on attachment: 58110

Committed r60830: <http://trac.webkit.org/changeset/60830>
Comment 8 WebKit Commit Bot 2010-06-08 01:35:28 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Eric Seidel (no email) 2010-06-08 02:30:00 PDT
Thanks for the patch!

The guards possibly should have been WTF_OwnPtrBrew_h instead of just OwnPtrBrew_h.  I'm not entirely sure of the convention.
Comment 10 Kwang Yul Seo 2010-06-08 02:42:25 PDT
(In reply to comment #9)
> Thanks for the patch!
> 
> The guards possibly should have been WTF_OwnPtrBrew_h instead of just OwnPtrBrew_h.  I'm not entirely sure of the convention.

I originally used WTF_OwnPtrBrew_h, but check-webkit-style complained about it.
Comment 11 Adam Barth 2010-06-08 10:16:38 PDT
This seems to have broken the Windows build:

http://build.webkit.org/builders/Windows%20Debug%20(Build)/builds/16866/steps/compile-webkit/logs/stdio


QTMovieGWorld.cpp

c:\cygwin\home\buildbot\slave\win-debug\build\webkitbuild\include\private\javascriptcore\OwnPtrCommon.h(45) : fatal error C1083: Cannot open include file: 'wtf/win/OwnPtrWin.h': No such file or directory
Comment 12 Kwang Yul Seo 2010-06-09 04:46:39 PDT
Created attachment 58234 [details]
Revised patch (build fix)

New patch with build fix. It took me several hours to setup Windows build environment :(
Comment 13 Adam Barth 2010-06-09 09:10:10 PDT
Comment on attachment 58234 [details]
Revised patch (build fix)

Thanks.  Sorry the Windows build is so painful.
Comment 14 Kwang Yul Seo 2010-06-23 23:49:03 PDT
It seems this patch is in the commit-queue forever. Eric, can you look at this?
Comment 15 Eric Seidel (no email) 2010-06-24 00:20:31 PDT
The commit-queue has been blocked for nearly 5 days due to various problems.  First was the Gtk builders being broken, then a hyphenate test, and then finally some cq issues on teh bot itself today.  it's running now,b ut the tree is currently red.  Once the tree turns green again I expect the queue will clear quickly.
Comment 16 Eric Seidel (no email) 2010-06-24 00:20:55 PDT
This bug is closed.  Is it suppposd to be open?  The commit-queue doesn't land patches from closed bugs.
Comment 17 Kwang Yul Seo 2010-06-24 00:23:07 PDT
Reopen the bug as we still have a pending patch.
Comment 18 WebKit Commit Bot 2010-06-25 02:48:51 PDT
Comment on attachment 58234 [details]
Revised patch (build fix)

Clearing flags on attachment: 58234

Committed r61842: <http://trac.webkit.org/changeset/61842>
Comment 19 WebKit Commit Bot 2010-06-25 02:48:56 PDT
All reviewed patches have been landed.  Closing bug.
Comment 20 Csaba Osztrogonác 2010-06-25 04:12:05 PDT
Reopen, because it was rolled out by http://trac.webkit.org/changeset/61847
Comment 21 Csaba Osztrogonác 2012-02-03 04:51:17 PST
Is there any update for this bug?
Comment 22 Darin Adler 2012-02-03 11:33:16 PST
To me this doesn’t seem important to do. Modifying OwnPtrCommon doesn’t seem like a frequent enough operation to be worth optimizing. Maybe I am missing something.
Comment 23 Martin Robinson 2015-05-07 16:54:20 PDT
OwnPtr isn't part of the code base any longer.