Bug 32336 – Update Chromium Loader to pass through the new ResourceType::TargetType

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Mike Belshe]

FrameLoaderClientImpl.cpp previously would override the resource type as a generic TargetIsSubresource when the target is not a main or subframe.  Update to only modify the type if the resource is a frame; otherwise, leave the setting as is, since the ResourceRequest now initializes and sets the resource type properly.

[Mike Belshe]

Created attachment 44554 [details]
patch

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44554 [details] without any errors.

[Eric Seidel (no email)]

Comment on attachment 44554 [details]
patch

Looks sane.  Although you've already filled in the Reviewed by line, so I'm not sure what needs to be done for this patch if anything...

[Darin Fisher (:fishd, Google)]

Comment on attachment 44554 [details]
patch

> Index: WebKit/chromium/public/WebURLRequest.h

> -        TargetIsSubFrame,
> -        TargetIsSubResource,
> +        TargetIsSubframe,
> +        TargetIsSubresource,
> +        TargetIsStyleSheet,
> +        TargetIsScript,
> +        TargetIsFontResource,
> +        TargetIsImage,

Removal of webkit api is something we generally try to stage.
Otherwise, it puts burden on the person performing the webkit
update to land a two-sided patch :-(

I think I saw you send michaeln a patch for the Chromium side
of this, but in the future please preserve the old webkit api,
and then remove it in a subsequent patch once Chromium has
been updated.

Otherwise, r=me too.

[Michael Nordman]

> I think I saw you send michaeln a patch for the Chromium side
> of this, but in the future please preserve the old webkit api,
> and then remove it in a subsequent patch once Chromium has
> been updated.

Change looks great, having more nuanced info about what is being retrieved is really nice.

As mentioned in http://codereview.chromium.org/477008, can we stage these changes in without the need of the two-headed patch dance (you never know when the change that breaks 500 layout tests will just show up).

[Mike Belshe]

Created attachment 44579 [details]
followup patch to workaround horribly painful and broken chromium/webkit processes.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44579 [details] without any errors.

[WebKit Commit Bot]

Comment on attachment 44579 [details]
followup patch to workaround horribly painful and broken chromium/webkit processes.

Clearing flags on attachment: 44579

Committed r51967: <http://trac.webkit.org/changeset/51967>

[Eric Seidel (no email)]

I'm confused as to if this should still be open or not?

[Eric Seidel (no email)]

I believe this has been landed, so marking closed.  Please re-open if I'm wrong.