<?xml version="1.0" encoding="UTF-8" standalone="yes" ?>
<!DOCTYPE bugzilla SYSTEM "https://bugs.webkit.org/page.cgi?id=bugzilla.dtd">

<bugzilla version="5.0.4.1"
          urlbase="https://bugs.webkit.org/"
          
          maintainer="admin@webkit.org"
>

    <bug>
          <bug_id>39182</bug_id>
          
          <creation_ts>2010-05-16 13:11:13 -0700</creation_ts>
          <short_desc>FrameLoader: factor applyUserAgent(), addExtraFieldsToRequest(), and addHTTPOriginIfNeeded() out of the FrameLoader class</short_desc>
          <delta_ts>2011-06-12 02:30:39 -0700</delta_ts>
          <reporter_accessible>1</reporter_accessible>
          <cclist_accessible>1</cclist_accessible>
          <classification_id>1</classification_id>
          <classification>Unclassified</classification>
          <product>WebKit</product>
          <component>WebCore Misc.</component>
          <version>528+ (Nightly build)</version>
          <rep_platform>All</rep_platform>
          <op_sys>All</op_sys>
          <bug_status>RESOLVED</bug_status>
          <resolution>WONTFIX</resolution>
          
          
          <bug_file_loc></bug_file_loc>
          <status_whiteboard></status_whiteboard>
          <keywords></keywords>
          <priority>P2</priority>
          <bug_severity>Normal</bug_severity>
          <target_milestone>---</target_milestone>
          
          <blocked>29947</blocked>
          <everconfirmed>1</everconfirmed>
          <reporter name="Chris Jerdonek">cjerdonek</reporter>
          <assigned_to name="Chris Jerdonek">cjerdonek</assigned_to>
          <cc>abarth</cc>
    
    <cc>cjerdonek</cc>
          

      

      

      

          <comment_sort_order>oldest_to_newest</comment_sort_order>  
          <long_desc isprivate="0" >
    <commentid>226524</commentid>
    <comment_count>0</comment_count>
    <who name="Chris Jerdonek">cjerdonek</who>
    <bug_when>2010-05-16 13:11:13 -0700</bug_when>
    <thetext>It looks like none of these methods that add information to a ResourceRequest (applyUserAgent, addExtraFieldsToRequest, and addHTTPOriginIfNeeded) require access to private FrameLoader data, so we should be able to make all of them non-member, non-friend.  Teasing these out will increase encapsulation a bit more.</thetext>
  </long_desc><long_desc isprivate="0" >
    <commentid>226569</commentid>
    <comment_count>1</comment_count>
    <who name="Adam Barth">abarth</who>
    <bug_when>2010-05-16 17:38:37 -0700</bug_when>
    <thetext>Hum...  It&apos;s not 100% clear to me where we&apos;d put these methods.  What did you have in mind?</thetext>
  </long_desc><long_desc isprivate="0" >
    <commentid>226574</commentid>
    <comment_count>2</comment_count>
    <who name="Chris Jerdonek">cjerdonek</who>
    <bug_when>2010-05-16 17:47:14 -0700</bug_when>
    <thetext>(In reply to comment #1)
&gt; Hum...  It&apos;s not 100% clear to me where we&apos;d put these methods.  What did you have in mind?

I had in mind declaring applyUserAgent() and addHTTPOriginIfNeeded() in FrameLoader.h outside of the class (suitably renamed).  addExtraFieldsToRequest() can be declared static (file-scope) in the .cpp file.</thetext>
  </long_desc><long_desc isprivate="0" >
    <commentid>226576</commentid>
    <comment_count>3</comment_count>
    <who name="Adam Barth">abarth</who>
    <bug_when>2010-05-16 18:01:02 -0700</bug_when>
    <thetext>Generally, we prefer keeping methods in the class, except for some kinds of utility methods.

One thing that might make sense is putting these methods on some sort of request object because they operate on the request.  You&apos;ll have to look to see which layer of request makes the most sense.  ResourceRequest might be slightly too low-level because the platform directory isn&apos;t allowed to depend on anything else in WebCore...</thetext>
  </long_desc><long_desc isprivate="0" >
    <commentid>419278</commentid>
    <comment_count>4</comment_count>
    <who name="Adam Barth">abarth</who>
    <bug_when>2011-06-12 02:30:39 -0700</bug_when>
    <thetext>Marking WONTFIX for now.  We can revisit this later if we still think this is a good idea.</thetext>
  </long_desc>
      
      

    </bug>

</bugzilla>