Bug 119658 - Remove didNewFirstVisuallyNonEmptyLayout from WebKit2
Summary: Remove didNewFirstVisuallyNonEmptyLayout from WebKit2
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Beth Dakin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-08-10 23:06 PDT by Beth Dakin
Modified: 2013-08-12 13:55 PDT (History)
5 users (show)

See Also:


Attachments
Patch (18.05 KB, patch)
2013-08-10 23:09 PDT, Beth Dakin
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Beth Dakin 2013-08-10 23:06:38 PDT
We should remove or at least deprecate didNewFirstVisuallyNonEmptyLayout from WebKit2. We think it will be okay to remove it entirely, so I will try that first.
Comment 1 Beth Dakin 2013-08-10 23:09:29 PDT
Created attachment 208486 [details]
Patch
Comment 2 Darin Adler 2013-08-11 10:29:51 PDT
Won’t this create problems for someone using an older Safari with a newer WebKit?
Comment 3 Beth Dakin 2013-08-11 11:08:30 PDT
(In reply to comment #2)
> Won’t this create problems for someone using an older Safari with a newer WebKit?

Sam's opinion was that the lost functionality would be acceptable in nightly builds, and that if we did find it to be a bigger problem than anticipated, we could roll this patch out and deprecate the function instead.
Comment 4 Darin Adler 2013-08-12 09:37:52 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > Won’t this create problems for someone using an older Safari with a newer WebKit?
> 
> Sam's opinion was that the lost functionality would be acceptable in nightly builds

Got it. I couldn’t tell from anything in the patch that we’d considered this when deciding it was OK. Even if it was just your opinion, I’d be fine with it. From the patch it wasn’t clear it was an intentional decision.
Comment 5 Darin Adler 2013-08-12 09:39:54 PDT
Comment on attachment 208486 [details]
Patch

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

> Source/WebKit2/UIProcess/WebLoaderClient.h:66
> +    // FIXME: We should consider removing didFirstVisuallyNonEmptyLayoutForFrame since its functionality is replaced by didLayout.

I would like a shorter comment. I’m not sure the word “functionality” adds anything here.

    // FIXME: Remove didFirstVisuallyNonEmptyLayoutForFrame; clients should use didLayout instead.
Comment 6 Beth Dakin 2013-08-12 13:55:01 PDT
Thanks Darin! I shortened the comment and added some information to the Changelog about why we think this is okay to remove.

I committed the change with http://trac.webkit.org/changeset/153956