Merge weburlrequest_extradata_impl.cc/h into content/child/request_extra_data.cc/h

Merge weburlrequest_extradata_impl.cc/h into content/child/request_extra_data.cc/h. Change to empty constructor and use individual setters for each value.

BUG=338338
TBR=darin@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=257655

[jam, 2014-03-04 19:25:40]

I think we can simplify this further. some of this complexity was because before
we had to avoid layering violations from src\webkit to src\content, but now that
that code is moving, this problem doesn't exist. More specifically, we don't
need two blink extra data derived-classes.

So we can get rid of WebURLRequestExtraDataImpl in favor of RequestExtraData.
The latter should move from having a constructor that takes everything to an
empty constructor and having set methods for all the member variables. Then it
can be created either in content/renderer/pepper/url_request_info_util.cc's
CreateWebURLRequest, RenderFrameImpl::willRequestAfterPreconnect, and
RenderFrameImpl::willSendRequest (if it's not created already).

[jam, 2014-03-12 00:32:01]

lgtm with comment

https://codereview.chromium.org/182383015/diff/20001/content/child/request_ex...
File content/child/request_extra_data.h (right):

https://codereview.chromium.org/182383015/diff/20001/content/child/request_ex...
content/child/request_extra_data.h:19: class CONTENT_EXPORT RequestExtraData
actually, since there's no logic, this should just be a struct with no setters
and getters. sorry i didn't say that earlier

[pilgrim_google, 2014-03-12 13:44:26]

On 2014/03/12 00:32:01, jam wrote:
> actually, since there's no logic, this should just be a struct with no setters
> and getters. sorry i didn't say that earlier

ಠ_ಠ

[pilgrim_google, 2014-03-12 13:52:21]

On 2014/03/12 00:32:01, jam wrote:
> lgtm with comment
> 
>
https://codereview.chromium.org/182383015/diff/20001/content/child/request_ex...
> File content/child/request_extra_data.h (right):
> 
>
https://codereview.chromium.org/182383015/diff/20001/content/child/request_ex...
> content/child/request_extra_data.h:19: class CONTENT_EXPORT RequestExtraData
> actually, since there's no logic, this should just be a struct with no setters
> and getters. sorry i didn't say that earlier

Also, that may be difficult because it inherits from a class in Blink that has a
bunch of other stuff in it.

[pilgrim_google, 2014-03-17 14:28:59]

The CQ bit was checked by pilgrim@chromium.org

[commit-bot: I haz the power, 2014-03-17 14:29:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pilgrim@chromium.org/182383015/60001

[commit-bot: I haz the power, 2014-03-17 14:49:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-17 14:49:12]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[pilgrim_google, 2014-03-17 15:11:21]

The CQ bit was checked by pilgrim@chromium.org

[commit-bot: I haz the power, 2014-03-17 15:11:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pilgrim@chromium.org/182383015/60001

[jamesr, 2014-03-17 21:41:22]

lgtm

[commit-bot: I haz the power, 2014-03-18 06:36:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-18 06:36:03]

The commit queue went berserk retrying too often for a
seemingly flaky test on builder android_dbg:
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...

[pilgrim_google, 2014-03-18 13:22:19]

The CQ bit was checked by pilgrim@chromium.org

[commit-bot: I haz the power, 2014-03-18 13:22:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pilgrim@chromium.org/182383015/80001

[commit-bot: I haz the power, 2014-03-18 16:22:28]

Change committed as 257655

[jam, 2014-03-18 18:28:12]

https://codereview.chromium.org/182383015/diff/80001/content/child/resource_d...
File content/child/resource_dispatcher_unittest.cc (right):

https://codereview.chromium.org/182383015/diff/80001/content/child/resource_d...
content/child/resource_dispatcher_unittest.cc:176: RequestExtraData* extra_data
= new RequestExtraData();
looks like this is what caused the asan leak which caused this cl to be
reverted.
seems like making it be an object on the stack as before will fix this.

i'm looking at this because it would be good to land this soon again, as another
cl from michaeln depends on it

[pilgrim_google, 2014-03-18 18:48:16]

On 2014/03/18 18:28:12, jam wrote:
>
https://codereview.chromium.org/182383015/diff/80001/content/child/resource_d...
> File content/child/resource_dispatcher_unittest.cc (right):
> 
>
https://codereview.chromium.org/182383015/diff/80001/content/child/resource_d...
> content/child/resource_dispatcher_unittest.cc:176: RequestExtraData*
extra_data
> = new RequestExtraData();
> looks like this is what caused the asan leak which caused this cl to be
> reverted.
> seems like making it be an object on the stack as before will fix this.
> 
> i'm looking at this because it would be good to land this soon again, as
another
> cl from michaeln depends on it

https://codereview.chromium.org/203853003/

[michaeln, 2014-03-18 20:11:06]

drive-by-comment

https://codereview.chromium.org/182383015/diff/80001/content/renderer/render_...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/182383015/diff/80001/content/renderer/render_...
content/renderer/render_frame_impl.cc:1828:
extra_data->set_was_after_preconnect_request(was_after_preconnect_request);
looks like this leaves data members uninitialized which could be dicey, should
the default ctor set default values?

[jam, 2014-03-18 21:25:34]

https://codereview.chromium.org/182383015/diff/80001/content/renderer/render_...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/182383015/diff/80001/content/renderer/render_...
content/renderer/render_frame_impl.cc:1828:
extra_data->set_was_after_preconnect_request(was_after_preconnect_request);
On 2014/03/18 20:11:07, michaeln wrote:
> looks like this leaves data members uninitialized which could be dicey, should
> the default ctor set default values?

good point, I didn't notice that RequestExtraData's constructor doesn't
intialize to sane values. Mark: can you change it so that it sets the values up.
then things like plugin_url_fetcher and Michael's new usage only have to change
what they need instead of initializing everything

[pilgrim_google, 2014-03-19 19:57:38]

On 2014/03/18 21:25:34, jam wrote:
>
https://codereview.chromium.org/182383015/diff/80001/content/renderer/render_...
> File content/renderer/render_frame_impl.cc (right):
> 
>
https://codereview.chromium.org/182383015/diff/80001/content/renderer/render_...
> content/renderer/render_frame_impl.cc:1828:
> extra_data->set_was_after_preconnect_request(was_after_preconnect_request);
> On 2014/03/18 20:11:07, michaeln wrote:
> > looks like this leaves data members uninitialized which could be dicey,
should
> > the default ctor set default values?
> 
> good point, I didn't notice that RequestExtraData's constructor doesn't
> intialize to sane values. Mark: can you change it so that it sets the values
up.
> then things like plugin_url_fetcher and Michael's new usage only have to
change
> what they need instead of initializing everything

Fixed in https://codereview.chromium.org/203853003/