Bug 46663 - FrameLoaderClient::didTransferChildFrameToNewDocument needs access to old page
Summary: FrameLoaderClient::didTransferChildFrameToNewDocument needs access to old page
Alias: None
Product: WebKit
Classification: Unclassified
Component: Frames (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Jenn Braithwaite
Depends on:
Blocks: 44713
  Show dependency treegraph
Reported: 2010-09-27 15:23 PDT by Jenn Braithwaite
Modified: 2010-10-31 18:06 PDT (History)
3 users (show)

See Also:

patch (26.49 KB, patch)
2010-09-27 17:25 PDT, Jenn Braithwaite
no flags Details | Formatted Diff | Diff
rollback api change (26.83 KB, patch)
2010-10-28 15:28 PDT, Jenn Braithwaite
dimich: review-
dimich: commit-queue-
Details | Formatted Diff | Diff
updated patch (26.97 KB, patch)
2010-10-28 16:32 PDT, Jenn Braithwaite
abarth: review+
abarth: commit-queue+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jenn Braithwaite 2010-09-27 15:23:52 PDT
Mac client needs access to the reparented frame's former page in order to update resource tracking (see bug 44713).
Comment 1 Jenn Braithwaite 2010-09-27 17:25:41 PDT
Created attachment 68996 [details]
Comment 2 Jenn Braithwaite 2010-09-28 11:53:09 PDT
This patch merely adds a parameter to  FrameLoaderClient::didTransferChildFrameToNewDocument(). Code to use it will be in a separate patch - probably the one that fixes bug 44713.
Comment 3 WebKit Commit Bot 2010-09-28 15:27:40 PDT
Comment on attachment 68996 [details]

Clearing flags on attachment: 68996

Committed r68576: <http://trac.webkit.org/changeset/68576>
Comment 4 WebKit Commit Bot 2010-09-28 15:27:45 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Jenn Braithwaite 2010-10-14 11:18:32 PDT
Turns out I don't need this API change after all because the update resource tracking trigger needs to go in WebCore and not the WebKit client layer.  What's the best way to revert this patch?   Or is it too late for a simple revert and I am better off entering a new bug and submitting a new patch to undo this change?
Comment 6 Dmitry Titov 2010-10-14 12:08:07 PDT
(In reply to comment #5)

Simple revert (with ChangeLog reflecting both changes) is just fine. No need for new bug.
Comment 7 Jenn Braithwaite 2010-10-28 15:28:34 PDT
Created attachment 72245 [details]
rollback api change
Comment 8 Dmitry Titov 2010-10-28 15:44:04 PDT
Comment on attachment 72245 [details]
rollback api change

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

Small ChangeLog nit.
r- since it's going via cq, so needs to be changed first.

> WebCore/ChangeLog:5
> +        Rollback:

Could you add revisions, as in: Rollback of <url to rev in trac>:

Also, since you edit it manually, lets add a reason for rollback (design changed, no need for parameter).

> WebKit/chromium/ChangeLog:5
> +        Rollback:

Ditto here (rev number and reason, also for all ChangeLogs)
Comment 9 Jenn Braithwaite 2010-10-28 16:32:41 PDT
Created attachment 72256 [details]
updated patch

Added revision info and link along with description to ChangeLogs.
Put back a few extra lines of code that are still needed: oldPage local var in Frame.cpp, plus include and class declarations for Page. Page is now used elsewhere in the file, so it's not safe to rollback those lines.
Comment 10 Adam Barth 2010-10-31 18:06:58 PDT
Comment on attachment 72256 [details]
updated patch

So many files.