Bug 34829 - [gtk] WebKitDownload's currentSize property is guint, should be guint64
Summary: [gtk] WebKitDownload's currentSize property is guint, should be guint64
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 34898
Blocks:
  Show dependency treegraph
 
Reported: 2010-02-10 21:06 PST by Diego Escalante Urrelo
Modified: 2010-02-17 12:26 PST (History)
3 users (show)

See Also:


Attachments
Quick *blind* fix (1.52 KB, patch)
2010-02-10 21:06 PST, Diego Escalante Urrelo
gustavo: review-
Details | Formatted Diff | Diff
2010-02-11 Diego Escalante Urrelo <descalante@igalia.com> (2.35 KB, patch)
2010-02-11 09:57 PST, Diego Escalante Urrelo
no flags Details | Formatted Diff | Diff
2010-02-11 Diego Escalante Urrelo <descalante@igalia.com> (2.35 KB, patch)
2010-02-11 11:22 PST, Diego Escalante Urrelo
no flags Details | Formatted Diff | Diff
2010-02-11 Diego Escalante Urrelo <descalante@igalia.com> (2.36 KB, patch)
2010-02-11 11:24 PST, Diego Escalante Urrelo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Diego Escalante Urrelo 2010-02-10 21:06:27 PST
Created attachment 48546 [details]
Quick *blind* fix

This seems to be causing this:
  https://bugzilla.gnome.org/show_bug.cgi?id=603395

I don't have the bandwidth nor disk to try that in real world, so I'm blindly betting on the attached patch :-).
Comment 1 Gustavo Noronha (kov) 2010-02-11 04:12:43 PST
I can get this tested. Perhaps we should use goffset, though?
Comment 2 Gustavo Noronha (kov) 2010-02-11 04:50:49 PST
Comment on attachment 48546 [details]
Quick *blind* fix

Your patch works. You get a warning on line 758, though, which you should fix before we land this (because Soup behaves correctly, and handles us a goffset instead). I wonder if we could move all our file size usage in WebKitDownload to goffset to be consistent/correct. I don't know why I didn't do that in the first place =(.
Comment 3 Diego Escalante Urrelo 2010-02-11 09:57:26 PST
Created attachment 48578 [details]
2010-02-11  Diego Escalante Urrelo  <descalante@igalia.com>

Reviewed by NOBODY (OOPS!).

        [gtk] WebKitDownload's currentSize property is guint, should be guint64
        https://bugs.webkit.org/show_bug.cgi?id=34829

        Actually use a guint64 for current-size property in WebKitDownload, we
        were using a guint which is likely the cause for wrong sizes for files
        over 4 Gb.

        * webkit/webkitdownload.cpp:
Comment 4 Diego Escalante Urrelo 2010-02-11 11:22:14 PST
Created attachment 48585 [details]
2010-02-11  Diego Escalante Urrelo  <descalante@igalia.com>

Reviewed by NOBODY (OOPS!).

        [gtk] WebKitDownload's currentSize property is guint, should be guint64
        https://bugs.webkit.org/show_bug.cgi?id=34829

        Actually use a guint64 for current-size property in WebKitDownload, we
        were using a guint which is likely the cause for wrong sizes for files
        over 4 Gb.

        * webkit/webkitdownload.cpp:
Comment 5 Diego Escalante Urrelo 2010-02-11 11:24:38 PST
Created attachment 48586 [details]
2010-02-11  Diego Escalante Urrelo  <descalante@igalia.com>

Reviewed by NOBODY (OOPS!).

        [gtk] WebKitDownload's currentSize property is guint, should be guint64
        https://bugs.webkit.org/show_bug.cgi?id=34829

        Actually use a guint64 for current-size property in WebKitDownload, we
        were using a guint which is likely the cause for wrong sizes for files
        over 4 Gb.

        * webkit/webkitdownload.cpp:
Comment 6 Gustavo Noronha (kov) 2010-02-11 11:28:37 PST
Comment on attachment 48586 [details]
2010-02-11  Diego Escalante Urrelo  <descalante@igalia.com>

Looks good, thanks!
Comment 7 WebKit Commit Bot 2010-02-11 23:04:45 PST
Comment on attachment 48586 [details]
2010-02-11  Diego Escalante Urrelo  <descalante@igalia.com>

Rejecting patch 48586 from commit-queue.

Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--quiet']" exit_code: 1
Running build-dumprendertree
Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests
Testing 12150 test cases.
websocket/tests/bufferedAmount-after-close.html -> crashed

Exiting early after 1 failures. 12118 tests run.
497.21s total testing time

12117 test cases (99%) succeeded
1 test case (<1%) crashed
7 test cases (<1%) had stderr output

Full output: http://webkit-commit-queue.appspot.com/results/258584
Comment 8 Gustavo Noronha (kov) 2010-02-13 04:05:15 PST
Comment on attachment 48586 [details]
2010-02-11  Diego Escalante Urrelo  <descalante@igalia.com>

The failure doesn't seem to be related.
Comment 9 WebKit Commit Bot 2010-02-14 21:31:31 PST
Comment on attachment 48586 [details]
2010-02-11  Diego Escalante Urrelo  <descalante@igalia.com>

Clearing flags on attachment: 48586

Committed r54762: <http://trac.webkit.org/changeset/54762>
Comment 10 WebKit Commit Bot 2010-02-14 21:31:38 PST
All reviewed patches have been landed.  Closing bug.
Comment 11 Eric Seidel (no email) 2010-02-17 12:26:01 PST
Crash was bug 34898.