25028 2009-04-03 09:40:51 -0700 Restore FrameLoader::policyDocumentLoader to fix the Chromium build 2009-04-03 10:55:49 -0700 1 1 1 Unclassified WebKit Page Loading 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 fishd fishd darin oldest_to_newest 116432 0 fishd 2009-04-03 09:40:51 -0700 Restore FrameLoader::policyDocumentLoader to fix the Chromium build r42158 removed FrameLoader::policyDocumentLoader which was originally added in r32320 as part of upstreaming changes to WebCore required by the Chromium build. We found that there are some FrameLoaderClient callbacks that occur when the current DocumentLoader is m_policyDocumentLoader, and without an accessor to that, it is not possible from FrameLoaderClient to access the DocumentLoader. This is important since the FrameLoaderClient may have subclassed DocumentLoader to provide additional state (which is what Chromium does). I understand that the code I'm referring to is not yet upstreamed, so it makes sense that this change would have been made. It surely looked like there were no other consumers. Hopefully this is a reasonable method to restore on FrameLoader since it makes sense to be able to access the current DocumentLoader during policy callbacks to the FrameLoaderClient. Patch coming up... 116433 1 29229 fishd 2009-04-03 09:42:00 -0700 Created attachment 29229 v1 patch 116434 2 fishd 2009-04-03 09:43:46 -0700 > This is important since the FrameLoaderClient may have subclassed > DocumentLoader to provide additional state Clarification: What I mean is that from FrameLoaderClient::createDocumentLoader, the FrameLoaderClient implementation can allocate a class that subclasses DocumentLoader. Then from other FrameLoaderClient methods, that subclass is exposed via a down cast from DocumentLoader. 116436 3 29229 darin 2009-04-03 09:56:56 -0700 Comment on attachment 29229 v1 patch It would be good to know why this is needed. I think it’s likely a mistake. r=me on bringing it back 116437 4 darin 2009-04-03 09:57:52 -0700 (In reply to comment #0) > We found that there are some FrameLoaderClient callbacks that occur when the > current DocumentLoader is m_policyDocumentLoader, and without an accessor to > that, it is not possible from FrameLoaderClient to access the DocumentLoader. We should consider adding the DocumentLoader as an argument to those functions to solve the problem. 116448 5 fishd 2009-04-03 10:53:02 -0700 > We should consider adding the DocumentLoader as an argument to those functions > to solve the problem. That sounds like a good idea. The FrameLoaderClient method in question is dispatchDecidePolicyForNavigationAction. Landed as: http://trac.webkit.org/changeset/42202 116449 6 fishd 2009-04-03 10:55:49 -0700 The key issue is that the policyDocumentLoader doesn't become provisional until after dispatchDecidePolicyForNavigationAction OKs the navigation. 29229 2009-04-03 09:42:00 -0700 2009-04-03 09:56:56 -0700 v1 patch fl_1.txt text/plain 1234 fishd SW5kZXg6IENoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBDaGFuZ2VMb2cJKHJldmlzaW9uIDQyMjAw KQorKysgQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTMgQEAKKzIwMDktMDQt MDMgIERhcmluIEZpc2hlciAgPGRhcmluQGNocm9taXVtLm9yZz4KKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBSZXN0b3JlIEZyYW1lTG9hZGVyOjpwb2xp Y3lEb2N1bWVudExvYWRlciB0byBmaXggdGhlIENocm9taXVtIGJ1aWxkLgorICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjUwMjgKKworICAgICAgICAqIGxv YWRlci9GcmFtZUxvYWRlci5oOgorICAgICAgICAoV2ViQ29yZTo6RnJhbWVMb2FkZXI6OnBvbGlj eURvY3VtZW50TG9hZGVyKToKKwogMjAwOS0wNC0wMiAgTWljaGFlbCBOb3JkbWFuICA8bWljaGFl bG5AZ29vZ2xlLmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBieSBBbGV4ZXkgUHJvc2t1cnlha292 LgpJbmRleDogbG9hZGVyL0ZyYW1lTG9hZGVyLmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gbG9hZGVyL0ZyYW1l TG9hZGVyLmgJKHJldmlzaW9uIDQyMTk5KQorKysgbG9hZGVyL0ZyYW1lTG9hZGVyLmgJKHdvcmtp bmcgY29weSkKQEAgLTE2Niw2ICsxNjYsNyBAQCBuYW1lc3BhY2UgV2ViQ29yZSB7CiAKICAgICAg ICAgRG9jdW1lbnRMb2FkZXIqIGFjdGl2ZURvY3VtZW50TG9hZGVyKCkgY29uc3Q7CiAgICAgICAg IERvY3VtZW50TG9hZGVyKiBkb2N1bWVudExvYWRlcigpIGNvbnN0IHsgcmV0dXJuIG1fZG9jdW1l bnRMb2FkZXIuZ2V0KCk7IH0KKyAgICAgICAgRG9jdW1lbnRMb2FkZXIqIHBvbGljeURvY3VtZW50 TG9hZGVyKCkgY29uc3QgeyByZXR1cm4gbV9wb2xpY3lEb2N1bWVudExvYWRlci5nZXQoKTsgfQog ICAgICAgICBEb2N1bWVudExvYWRlciogcHJvdmlzaW9uYWxEb2N1bWVudExvYWRlcigpIGNvbnN0 IHsgcmV0dXJuIG1fcHJvdmlzaW9uYWxEb2N1bWVudExvYWRlci5nZXQoKTsgfQogICAgICAgICBG cmFtZVN0YXRlIHN0YXRlKCkgY29uc3QgeyByZXR1cm4gbV9zdGF0ZTsgfQogICAgICAgICBzdGF0 aWMgZG91YmxlIHRpbWVPZkxhc3RDb21wbGV0ZWRMb2FkKCk7Cg==