RESOLVED FIXED 30581
Add getter for FrameLoader::m_suppressOpenerInNewFrame
https://bugs.webkit.org/show_bug.cgi?id=30581
Summary Add getter for FrameLoader::m_suppressOpenerInNewFrame
Nate Chapin
Reported 2009-10-20 10:36:37 PDT
Forgot to include a getter method in http://trac.webkit.org/changeset/49809 for m_suppressOpenerInNewFrame.
Attachments
patch (1.16 KB, patch)
2009-10-20 10:39 PDT, Nate Chapin
abarth: review-
added more descriptive changelog entry (1.29 KB, patch)
2009-10-20 11:08 PDT, Nate Chapin
abarth: review+
Nate Chapin
Comment 1 2009-10-20 10:39:51 PDT
Adam Barth
Comment 2 2009-10-20 10:46:39 PDT
Comment on attachment 41512 [details] patch The ChangeLog does not explain why we need this getter. Why do we need it?
Nate Chapin
Comment 3 2009-10-20 10:57:45 PDT
You're right, oops. Chromium is looking for a definitive way to determine whether a link was noreferrer and therefore is safe to open in a new process. If the opener is being suppressed, that should be a good indication (better than any heuristic) that an entirely new context is the correct decision.
Adam Barth
Comment 4 2009-10-20 11:01:03 PDT
Ugg. The problem is that this isn't really a property of the FrameLoader but a property of the current FrameLoadTask (which is a concept that doesn't really exist at the moment). I guess it's ok to dig this hole a bit deeper... I'll try to remember not to review this getter even though it looks like dead code. If you post an updated patch with a better ChangeLog, I can review it again.
Nate Chapin
Comment 5 2009-10-20 11:08:10 PDT
Created attachment 41515 [details] added more descriptive changelog entry Yeah, sorry to make the FrameLoader mess worse :-( If you would like me to help at all with cleanup of FrameLoader, I think I'm now on the hook.
Adam Barth
Comment 6 2009-10-20 11:11:26 PDT
Comment on attachment 41515 [details] added more descriptive changelog entry Ok. Thanks for offering to help with FrameLoader. The next step is somewhat involved (splitting out the document-specific processing), but there might be parallelizable tasks after that.
Nate Chapin
Comment 7 2009-10-21 14:34:19 PDT
Note You need to log in before you can comment on or make changes to this bug.