Bug 113713 - [WK2] WebPageProxy's setViewNeedsDisplay() and viewSize() should check page validity.
Summary: [WK2] WebPageProxy's setViewNeedsDisplay() and viewSize() should check page v...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Marcelo Lira
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-01 12:15 PDT by Marcelo Lira
Modified: 2013-04-02 11:36 PDT (History)
3 users (show)

See Also:


Attachments
Patch (2.35 KB, patch)
2013-04-01 12:17 PDT, Marcelo Lira
benjamin: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Marcelo Lira 2013-04-01 12:15:18 PDT
WebPageProxy's setViewNeedsDisplay() and viewSize() should check page validity.

Soon after WebProcess dies there's still a call to those methods, and at the time the PageClient the WebPageProxy holds is invalid.
Comment 1 Marcelo Lira 2013-04-01 12:17:36 PDT
Created attachment 196002 [details]
Patch
Comment 2 Benjamin Poulain 2013-04-02 11:21:00 PDT
Comment on attachment 196002 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=196002&action=review

> Source/WebKit2/ChangeLog:4
> +        WebPageProxy's setViewNeedsDisplay() and viewSize() should check page validity.
> +        https://bugs.webkit.org/show_bug.cgi?id=113713

No they shouldn't.

You must explain _why_ the change is needed. And add a test unless you have a good reason.
Comment 3 Jesus Sanchez-Palencia 2013-04-02 11:23:27 PDT
(In reply to comment #2)
> (From update of attachment 196002 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=196002&action=review
> 
> > Source/WebKit2/ChangeLog:4
> > +        WebPageProxy's setViewNeedsDisplay() and viewSize() should check page validity.
> > +        https://bugs.webkit.org/show_bug.cgi?id=113713
> 
> No they shouldn't.
> 
> You must explain _why_ the change is needed. And add a test unless you have a good reason.


Since you r- this, so why they shouldn't check the validity then?
Comment 4 Anders Carlsson 2013-04-02 11:30:51 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 196002 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=196002&action=review
> > 
> > > Source/WebKit2/ChangeLog:4
> > > +        WebPageProxy's setViewNeedsDisplay() and viewSize() should check page validity.
> > > +        https://bugs.webkit.org/show_bug.cgi?id=113713
> > 
> > No they shouldn't.
> > 
> > You must explain _why_ the change is needed. And add a test unless you have a good reason.
> 
> 
> Since you r- this, so why they shouldn't check the validity then?

Other ports rely on being able to call setViewNeedsDisplay even if the page is invalid. You mustn’t break this contract.
Comment 5 Jesus Sanchez-Palencia 2013-04-02 11:36:10 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > (From update of attachment 196002 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=196002&action=review
> > > 
> > > > Source/WebKit2/ChangeLog:4
> > > > +        WebPageProxy's setViewNeedsDisplay() and viewSize() should check page validity.
> > > > +        https://bugs.webkit.org/show_bug.cgi?id=113713
> > > 
> > > No they shouldn't.
> > > 
> > > You must explain _why_ the change is needed. And add a test unless you have a good reason.
> > 
> > 
> > Since you r- this, so why they shouldn't check the validity then?
> 
> Other ports rely on being able to call setViewNeedsDisplay even if the page is invalid. You mustn’t break this contract.


Oh ok, thanks for the explanation! So I guess perhaps this is not the right way to check what Marcelo is looking for... Would checking if (m_pageClient) be acceptable here, since it is a raw pointer ?