Bug 44591 - Deploy adoptPtr to WebCore/loader
Summary: Deploy adoptPtr to WebCore/loader
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Adam Barth
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-25 00:45 PDT by Adam Barth
Modified: 2010-08-26 00:28 PDT (History)
5 users (show)

See Also:


Attachments
Patch (19.45 KB, patch)
2010-08-25 00:49 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch for landing (18.40 KB, patch)
2010-08-25 09:47 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch for landing (12.42 KB, patch)
2010-08-25 11:40 PDT, Adam Barth
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2010-08-25 00:45:31 PDT
Deploy adoptPtr to WebCore/loader
Comment 1 Adam Barth 2010-08-25 00:49:12 PDT
Created attachment 65382 [details]
Patch
Comment 2 WebKit Review Bot 2010-08-25 02:01:42 PDT
Attachment 65382 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/3805075
Comment 3 Adam Barth 2010-08-25 09:47:53 PDT
Created attachment 65429 [details]
Patch for landing
Comment 4 Darin Adler 2010-08-25 10:17:00 PDT
Comment on attachment 65429 [details]
Patch for landing

I don't think that using adoptPtr on classes derived from CachedResource is helpful until we figure out the ownership approach. Adding adoptPtr/leakPtr to call sites doesn't do a lot of harm, but I also don't think it does much good.

Since no icon database is ever deleted, it is too bad we are compiling code that calls the icon database destructor. It might be better for us to come up with a different way to handle immortal objects, rather than using OwnPtr which will compile in never-called code paths that destroy the object. I suggest omitting the IconDatabase changes.
Comment 5 Adam Barth 2010-08-25 11:40:01 PDT
Created attachment 65451 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2010-08-25 19:32:14 PDT
Comment on attachment 65451 [details]
Patch for landing

Rejecting patch 65451 from commit-queue.

Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--wait-for-httpd', '--ignore-tests', 'compositing,media', '--quiet']" exit_code: 1
Running build-dumprendertree
Compiling Java tests
make: Nothing to be done for `default'.
Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests
Testing 20823 test cases.
svg/dom/SVGScriptElement/script-set-href.svg -> failed

Exiting early after 1 failures. 18347 tests run.
542.96s total testing time

18346 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
26 test cases (<1%) had stderr output

Full output: http://queues.webkit.org/results/3740638
Comment 7 Eric Seidel (no email) 2010-08-25 23:20:39 PDT
Comment on attachment 65451 [details]
Patch for landing

Another flaky test.
Comment 8 WebKit Commit Bot 2010-08-26 00:28:17 PDT
Comment on attachment 65451 [details]
Patch for landing

Clearing flags on attachment: 65451

Committed r66083: <http://trac.webkit.org/changeset/66083>
Comment 9 WebKit Commit Bot 2010-08-26 00:28:22 PDT
All reviewed patches have been landed.  Closing bug.