Merge weburlrequest_extradata_impl.cc/h into content/child/request_extra_data.cc/h (attempt #2)

content/renderer/render_frame_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/render_frame_impl.h"
 
 #include <map>
 #include <string>
 
 #include "base/auto_reset.h"
@@ skipping 127 matching lines @@
     return -1;
 
   if (item.httpBody().isNull())
     return -1;
 
   return item.httpBody().identifier();
 }
 
 WebURLResponseExtraDataImpl* GetExtraDataFromResponse(
     const WebURLResponse& response) {
-  return static_cast<WebURLResponseExtraDataImpl*>(
-      response.extraData());
+  return static_cast<WebURLResponseExtraDataImpl*>(response.extraData());
 }
 
 void GetRedirectChain(WebDataSource* ds, std::vector<GURL>* result) {
   // Replace any occurrences of swappedout:// with about:blank.
   const WebURL& blank_url = GURL(kAboutBlankURL);
   WebVector<WebURL> urls;
   ds->redirectChain(urls);
   result->reserve(urls.size());
   for (size_t i = 0; i < urls.size(); ++i) {
     if (urls[i] != GURL(kSwappedOutURL))
@@ skipping 1657 matching lines @@
     blink::WebURLRequest& request) {
   DCHECK(!frame_ || frame_ == frame);
   // FIXME(kohei): This will never be set.
   WebString custom_user_agent;
 
   DCHECK(!request.extraData());
 
   bool was_after_preconnect_request = true;
   // The args after |was_after_preconnect_request| are not used, and set to
   // correct values at |willSendRequest|.
-  request.setExtraData(new webkit_glue::WebURLRequestExtraDataImpl(
-      custom_user_agent, was_after_preconnect_request));
+  RequestExtraData* extra_data = new RequestExtraData();
+  extra_data->set_custom_user_agent(custom_user_agent);
+  extra_data->set_was_after_preconnect_request(was_after_preconnect_request);
+  request.setExtraData(extra_data);
 }
 
 void RenderFrameImpl::willSendRequest(
     blink::WebFrame* frame,
     unsigned identifier,
     blink::WebURLRequest& request,
     const blink::WebURLResponse& redirect_response) {
   DCHECK(!frame_ || frame_ == frame);
   // The request my be empty during tests.
   if (request.url().isEmpty())
@@ skipping 28 matching lines @@
 
   if (internal_data->is_cache_policy_override_set())
     request.setCachePolicy(internal_data->cache_policy_override());
 
   // The request's extra data may indicate that we should set a custom user
   // agent. This needs to be done here, after WebKit is through with setting the
   // user agent on its own.
   WebString custom_user_agent;
   bool was_after_preconnect_request = false;
   if (request.extraData()) {
-    webkit_glue::WebURLRequestExtraDataImpl* old_extra_data =
-        static_cast<webkit_glue::WebURLRequestExtraDataImpl*>(
+    RequestExtraData* old_extra_data =
+        static_cast<RequestExtraData*>(
             request.extraData());
     custom_user_agent = old_extra_data->custom_user_agent();
     was_after_preconnect_request =
         old_extra_data->was_after_preconnect_request();
 
     if (!custom_user_agent.isNull()) {
       if (custom_user_agent.isEmpty())
         request.clearHTTPHeaderField("User-Agent");
       else
         request.setHTTPHeaderField("User-Agent", custom_user_agent);
@@ skipping 29 matching lines @@
     }
   } else if (frame->dataSource()) {
     ServiceWorkerNetworkProvider* provider =
         ServiceWorkerNetworkProvider::FromDocumentState(
             DocumentState::FromDataSource(frame->dataSource()));
     provider_id = provider->provider_id();
   }
 
   int parent_routing_id = frame->parent() ?
       FromWebFrame(frame->parent())->GetRoutingID() : -1;
-  request.setExtraData(
-      new RequestExtraData(render_view_->visibilityState(),
-                           custom_user_agent,
-                           was_after_preconnect_request,
-                           routing_id_,
-                           (frame == top_frame),
-                           GURL(frame->document().securityOrigin().toString()),
-                           frame->parent() == top_frame,
-                           parent_routing_id,
-                           navigation_state->allow_download(),
-                           transition_type,
-                           should_replace_current_entry,
-                           navigation_state->transferred_request_child_id(),
-                           navigation_state->transferred_request_request_id(),
-                           provider_id));
+  RequestExtraData extra_data;

[michaeln, 2014/03/20 00:10:43]

whoa... this is stack allocated shouldn't this be heap alloced? what am i
missing?

    // Extra data associated with the underlying resource request. Resource
    // requests can be copied. If non-null, each copy of a resource requests
    // holds a pointer to the extra data, and the extra data pointer will be
    // deleted when the last resource request is destroyed. Setting the extra
    // data pointer will cause the underlying resource request to be
    // dissociated from any existing non-null extra data pointer.
    BLINK_PLATFORM_EXPORT ExtraData* extraData() const;
    BLINK_PLATFORM_EXPORT void setExtraData(ExtraData*);

[michaeln, 2014/03/20 00:48:25]

Let me rephrase that, its not really a question.

Please heap allocate the extra_data here since ownership of the ptr is passed to
the blink::WebURLRequest below. And keep as stack allocs in
plugin_url_fetcher.cc and resource_dispatcher_unittest.cc (which do not pass
ownership to WebURLRequest).

+  extra_data.set_visibility_state(render_view_->visibilityState());
+  extra_data.set_custom_user_agent(custom_user_agent);
+  extra_data.set_was_after_preconnect_request(was_after_preconnect_request);
+  extra_data.set_render_frame_id(routing_id_);
+  extra_data.set_is_main_frame(frame == top_frame);
+  extra_data.set_frame_origin(
+      GURL(frame->document().securityOrigin().toString()));
+  extra_data.set_parent_is_main_frame(frame->parent() == top_frame);
+  extra_data.set_parent_render_frame_id(parent_routing_id);
+  extra_data.set_allow_download(navigation_state->allow_download());
+  extra_data.set_transition_type(transition_type);
+  extra_data.set_should_replace_current_entry(should_replace_current_entry);
+  extra_data.set_transferred_request_child_id(
+      navigation_state->transferred_request_child_id());
+  extra_data.set_transferred_request_request_id(
+      navigation_state->transferred_request_request_id());
+  extra_data.set_service_worker_provider_id(provider_id);
+  request.setExtraData(&extra_data);
 
   DocumentState* top_document_state =
       DocumentState::FromDataSource(top_data_source);
   if (top_document_state) {
     // TODO(gavinp): separate out prefetching and prerender field trials
     // if the rel=prerender rel type is sticking around.
     if (request.targetType() == WebURLRequest::TargetIsPrefetch)
       top_document_state->set_was_prefetcher(true);
 
     if (was_after_preconnect_request)
@@ skipping 44 matching lines @@
   // If we are in view source mode, then just let the user see the source of
   // the server's error page.
   if (frame->isViewSourceModeEnabled())
     return;
 
   DocumentState* document_state =
       DocumentState::FromDataSource(frame->provisionalDataSource());
   int http_status_code = response.httpStatusCode();
 
   // Record page load flags.
-  WebURLResponseExtraDataImpl* extra_data = GetExtraDataFromResponse(response);
+  WebURLResponseExtraDataImpl* extra_data =
+      GetExtraDataFromResponse(response);
   if (extra_data) {
     document_state->set_was_fetched_via_spdy(
         extra_data->was_fetched_via_spdy());
     document_state->set_was_npn_negotiated(
         extra_data->was_npn_negotiated());
     document_state->set_npn_negotiated_protocol(
         extra_data->npn_negotiated_protocol());
     document_state->set_was_alternate_protocol_available(
         extra_data->was_alternate_protocol_available());
     document_state->set_connection_info(
@@ skipping 733 matching lines @@
   }
 
   Send(new FrameHostMsg_OpenURL(routing_id_, params));
 }
 
 void RenderFrameImpl::didChangeLoadProgress(double load_progress) {
   render_view_->didChangeLoadProgress(frame_, load_progress);
 }
 
 }  // namespace content