Bug 59666 – Enable strict OwnPtrs for Chromium

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Adam Barth]

Enable strict OwnPtrs for Chromium

[Adam Barth]

Created attachment 91397 [details]
Patch

[Adam Barth]

Should build cleanly on Mac, but jamesr says it doesn't build cleanly on Linux.

[David Levin]

Comment on attachment 91397 [details]
Patch

Pending clean build of course.

[James Robinson]

https://bugs.webkit.org/show_bug.cgi?id=59664 is needed for chromium linux.  I haven't tried chromium win, but I'm sure there are issues there as well.

[WebKit Review Bot]

Attachment 91397 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8518180

[Adam Barth]

I don't have a chromium-win build.  I'll probably land this late at night and fire-fight the bot, unless someone would like to volunteer to weed out these issues on windows.

[David Levin]

Figuring out how to do a try job with both depots was harder than I thought it would be but here you go:

http://build.chromium.org/p/tryserver.chromium/builders/win/builds/28759
http://build.chromium.org/p/tryserver.chromium/builders/linux/builds/23951
http://build.chromium.org/p/tryserver.chromium/builders/mac/builds/24454

[James Robinson]

Tryjobs all failed to apply the patch.  Try specifying -r to  git/gcl try.

[David Levin]

(In reply to comment #8)
> Tryjobs all failed to apply the patch.  Try specifying -r to  git/gcl try.

I'm not sure if that would help because I think that specifies chromium's revision and the problem is webkit's revision (maybe if I ran it from the subdir and then I wouldn't have to update DEPS?)

Anyway, I modified the patch so that it won't conflict with the other patch that is on the webkit side (http://build.chromium.org/p/tryserver.chromium/builders/linux/builds/23959/steps/update/logs/patch) and I think this will work now:
http://build.chromium.org/p/tryserver.chromium/builders/win/builds/28765
http://build.chromium.org/p/tryserver.chromium/builders/linux/builds/23959
http://build.chromium.org/p/tryserver.chromium/builders/mac/builds/24460

[Adam Barth]

Those try results don't really make sense.  It's reporting errors in functions that have #define LOOSE_OWN_PTR at the top.

[David Levin]

(In reply to comment #10)
> Those try results don't really make sense.  It's reporting errors in functions that have #define LOOSE_OWN_PTR at the top.

oh that has to do with how I finally got the patch to apply: http://build.chromium.org/p/tryserver.chromium/builders/linux/builds/23959/steps/update/logs/patch

The reason I had to do this was a bit involved. (I was patching before your first change to OwnPtr.h but rolling to after it -- I didn't think of this issue or I could have handled it differently.)

[Adam Barth]

In any case, the windows results are helpful.  :)

[Adam Barth]

Ok.  I've fixed the error that the Windows try bot uncovered.  The Mac trybot also uncovered an error that I don't see in my upstream Chromium build.  Perhaps the feature is only enabled downstream?  In any case, I fixed it as well.

I'm going to try landing this patch with my fingers crossed.  I expect disaster.

[Adam Barth]

Comment on attachment 91397 [details]
Patch

Clearing flags on attachment: 91397

Committed r85167: <http://trac.webkit.org/changeset/85167>

[Adam Barth]

All reviewed patches have been landed.  Closing bug.

[Adam Barth]

Looks like it built cleanly on chromium-win.  Thanks Mr. Levin!