Bug 30581 - Add getter for FrameLoader::m_suppressOpenerInNewFrame
Summary: Add getter for FrameLoader::m_suppressOpenerInNewFrame
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nate Chapin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-10-20 10:36 PDT by Nate Chapin
Modified: 2009-10-21 14:34 PDT (History)
2 users (show)

See Also:


Attachments
patch (1.16 KB, patch)
2009-10-20 10:39 PDT, Nate Chapin
abarth: review-
Details | Formatted Diff | Diff
added more descriptive changelog entry (1.29 KB, patch)
2009-10-20 11:08 PDT, Nate Chapin
abarth: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nate Chapin 2009-10-20 10:36:37 PDT
Forgot to include a getter method in http://trac.webkit.org/changeset/49809 for m_suppressOpenerInNewFrame.
Comment 1 Nate Chapin 2009-10-20 10:39:51 PDT
Created attachment 41512 [details]
patch
Comment 2 Adam Barth 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?
Comment 3 Nate Chapin 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.
Comment 4 Adam Barth 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.
Comment 5 Nate Chapin 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.
Comment 6 Adam Barth 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.
Comment 7 Nate Chapin 2009-10-21 14:34:19 PDT
This was landed in http://trac.webkit.org/changeset/49872.