Bug 135818 - RenderObject::repaintUsingContainer should take repaint container as reference.
Summary: RenderObject::repaintUsingContainer should take repaint container as reference.
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL:
Keywords:
Depends on:
Blocks: 135817
  Show dependency tree / graph
 
Reported: 2014-08-11 16:00 PDT by zalan
Modified: 2014-08-15 20:56 PDT (History)
7 users (show)

See Also:

Attachments
Patch (19.54 KB, patch)
2014-08-11 16:12 PDT, zalan 		simon.fraser: review+
zalan: commit-queue-
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 zalan 2014-08-11 16:00:48 PDT 
subtask of bug 135817
Comment 1 zalan 2014-08-11 16:12:53 PDT 
Created attachment 236411 [details]
Patch
Comment 2 Darin Adler 2014-08-11 22:09:28 PDT 
Comment on attachment 236411 [details]
Patch

This seems worse than the old version. All those null checks at all the call sites are messy.
Comment 3 zalan 2014-08-15 20:56:21 PDT 
Comment on attachment 236411 [details]
Patch

(In reply to comment #2)
> (From update of attachment 236411 [details])
> This seems worse than the old version. All those null checks at all the call sites are messy.
Yes, they are!
This is an incremental change. see Changlog entry:

"This is in preparation to ensure that we always pass a valid repaint container to repaint
functions. (tracked here webkit.org/b/135817). This changeset has explicit view()
calls which are going to be removed once containerForRepaint() starts returning container reference."

My plan is to enforce containerForRepaint() to return a reference. There's a chance that it's not possible without some major changes so I decided to do it incrementally. However, I wasn't planning to commit these intermediate changes until after the goal is met. (so those null checks should never land on trunk.)  
I should have been more vocal about it in the bug description.