Bug 119142 - REGRESION(r151091) ASSERTION FAILED: m_readableData || m_writableData
Summary: REGRESION(r151091) ASSERTION FAILED: m_readableData || m_writableData
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Allan Sandfeld Jensen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-07-26 04:36 PDT by Allan Sandfeld Jensen
Modified: 2013-07-30 04:13 PDT (History)
3 users (show)

See Also:

Attachments
Patch (1.35 KB, patch)
2013-07-26 04:38 PDT, Allan Sandfeld Jensen 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Allan Sandfeld Jensen 2013-07-26 04:36:01 PDT 
http/tests/misc/drag-over-iframe-invalid-source.html asserts with 'ASSERTION FAILED: m_readableData || m_writableData'. The problem seems to be that the r151091 refactoring left us with an incorrect assert.
Comment 1 Allan Sandfeld Jensen 2013-07-26 04:38:28 PDT 
Created attachment 207523 [details]
Patch
Comment 2 Jocelyn Turcotte 2013-07-30 03:19:29 PDT 
Arun, what do you intend with this assert?
Comment 3 Arunprasad Rajkumar 2013-07-30 03:24:51 PDT 
(In reply to comment #2)
> Arun, what do you intend with this assert?

Either m_readableData or m_writableData should be valid, and not both.

I just moved the legacy to new interface, old code also had this assertion.

But anyhow I run all LayoutTests to confirm, Don't know how I missed this :(
Comment 4 Arunprasad Rajkumar 2013-07-30 03:35:18 PDT 
(In reply to comment #3)
> But anyhow I run all LayoutTests to confirm, Don't know how I missed this :(

I remember running all test case before raising for a review, whether that test case is added newly?(atleast after a commit build-bot might have run LayoutTests right?, i'm wondering how it is missed)
Comment 5 Jocelyn Turcotte 2013-07-30 03:38:02 PDT 
The assertion in this case is that "at least one of the two" is non-null when readData() is called. So if you just ported it I think we can just remove it then, I was wondering if you were intending that readData shouldn't be called in this case.
Thanks.
Comment 6 Arunprasad Rajkumar 2013-07-30 03:41:20 PDT 
(In reply to comment #5)
> The assertion in this case is that "at least one of the two" is non-null when readData() is called. So if you just ported it I think we can just remove it then, I was wondering if you were intending that readData shouldn't be called in this case.
> Thanks.

'ASSERTION FAILED: m_readableData || m_writableData', in this case both were null right? Is it expected?
Comment 7 Allan Sandfeld Jensen 2013-07-30 03:47:26 PDT 
(In reply to comment #6)
> (In reply to comment #5)
> > The assertion in this case is that "at least one of the two" is non-null when readData() is called. So if you just ported it I think we can just remove it then, I was wondering if you were intending that readData shouldn't be called in this case.
> > Thanks.
> 
> 'ASSERTION FAILED: m_readableData || m_writableData', in this case both were null right? Is it expected?

That is expected. The old assert you ported was a bit longer and tested another value you removed.
Comment 8 WebKit Commit Bot 2013-07-30 04:13:11 PDT 
Comment on attachment 207523 [details]
Patch

Clearing flags on attachment: 207523

Committed r153461: <http://trac.webkit.org/changeset/153461>
Comment 9 WebKit Commit Bot 2013-07-30 04:13:13 PDT 
All reviewed patches have been landed.  Closing bug.