RESOLVED FIXED92153
[BlackBerry] notifyContentRendered() call missing in two spots 
https://bugs.webkit.org/show_bug.cgi?id=92153
Summary [BlackBerry] notifyContentRendered() call missing in two spots
Jakob Petsovits
Reported 2012-07-24 13:30:03 PDT
From (the initial version of) the commit message: """ In dispatchDidFirstVisualLayout() and repaint() in immediate mode, we render but don't notify the WebPageClient that the content was modified. In the long run, we should probably keep track of the modified reason from within render() itself and use this to automatically notify the client from within blitContents() and/or invalidateWindow(), depending on the rendering path. That's somewhat of a medium-size undertaking though; for now, adding the call directly to the renderVisibleContents() call sites will do. """ Adam Treat suggested I move the notification to renderVisibleContents(). I thought about it, and I think it's a good idea but will require a more general solution as mentioned above. As I'll be occupied with other bug reports for a while, I think it's a good idea to get this fix right now, it fixes a real issue and doesn't introduce any hacks or whatnot. We can and should still do some refactoring, but I think it makes sense to go with the straightforward approach for now. Patch below. The related internal bug is RIM PR 173340.
Attachments
Patch, straightforward version (6.78 KB, patch)
2012-07-24 14:39 PDT, Jakob Petsovits 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Jakob Petsovits
Comment 1 2012-07-24 14:39:30 PDT
Created attachment 154144 [details] Patch, straightforward version Couldn't get webkit-patch to upload it for me this time, so uploading the output of git show instead which is of course also a patch.
Arvid Nilsson
Comment 2 2012-07-25 02:07:04 PDT
I can be internal reviewer for this, it looks good to me.
Antonio Gomes
Comment 3 2012-07-25 06:03:39 PDT
Comment on attachment 154144 [details] Patch, straightforward version I found it fragile to have to add notifyContentRendered calls in so many places. It is easy to miss one in the future. Lets go with adam's suggestion in the future please.
WebKit Review Bot
Comment 4 2012-07-25 06:19:07 PDT
Comment on attachment 154144 [details] Patch, straightforward version Clearing flags on attachment: 154144 Committed r123610: <http://trac.webkit.org/changeset/123610>
WebKit Review Bot
Comment 5 2012-07-25 06:19:11 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.