Eliminate mime_util dependency from WebFrameLoaderClient

Eliminate mime_util dependency from WebFrameLoaderClient.

BUG=24604
TEST=None

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=30675

[Mohamed Mansour, 2009-10-28 23:37:27]

I have looked in the implementation of "net::IsSupportedMimeType" and it is
checking image and nonimage mimemap so I did the same for this one.

I don't know if this is what you wanted, by using SimpleWebMimeRegistryImpl

[darin (slow to review), 2009-10-28 23:59:02]

sorry for not making this clear in the bug, but what i was hoping for here was
to add a new method to WebMimeRegistry that is called supportsMimeType.

it would have a prototype like this:

  virtual SupportsType supportsMIMEType(const WebString& mimeType) = 0;

then, the implementation of that would call directly to
net::IsSupportedMimeType(...)

this way we both avoid the direct dependency on net/base/mime_util.h and also
avoid duplicating the logic of  net::IsSupportedMimeType.

[Mohamed Mansour, 2009-10-29 22:49:03]

Is this how you want it?

[darin (slow to review), 2009-10-30 04:13:20]

http://codereview.chromium.org/344018/diff/1002/1004
File webkit/glue/simple_webmimeregistry_impl.cc (right):

http://codereview.chromium.org/344018/diff/1002/1004#newcode29
Line 29: namespace webkit_glue {
nit: add a new line below this one to preserve the whitespace that was there
before.

http://codereview.chromium.org/344018/diff/1002/1006
File webkit/glue/webframeloaderclient_impl.cc (right):

http://codereview.chromium.org/344018/diff/1002/1006#newcode1067
Line 1067: webkit_glue::SimpleWebMimeRegistryImpl mime_registry_;
instead of allocating a new SimpleWebMimeRegistryImpl, please just use
WebKit::webKitClient()->mimeRegistry()->supportsMIMEType(...)

and i think you should check the result against WebMimeRegistry::IsSupported

[Mohamed Mansour, 2009-10-30 04:41:11]

http://codereview.chromium.org/344018/diff/1002/1004
File webkit/glue/simple_webmimeregistry_impl.cc (right):

http://codereview.chromium.org/344018/diff/1002/1004#newcode29
Line 29: namespace webkit_glue {
On 2009/10/30 04:13:20, darin wrote:
> nit: add a new line below this one to preserve the whitespace that was there
> before.

Done.

http://codereview.chromium.org/344018/diff/1002/1006
File webkit/glue/webframeloaderclient_impl.cc (right):

http://codereview.chromium.org/344018/diff/1002/1006#newcode1067
Line 1067: webkit_glue::SimpleWebMimeRegistryImpl mime_registry_;
On 2009/10/30 04:13:20, darin wrote:
> instead of allocating a new SimpleWebMimeRegistryImpl, please just use
> WebKit::webKitClient()->mimeRegistry()->supportsMIMEType(...)
> 
> and i think you should check the result against WebMimeRegistry::IsSupported

Done.

[darin (slow to review), 2009-10-30 04:50:32]

http://codereview.chromium.org/344018/diff/5001/5005
File webkit/glue/webframeloaderclient_impl.cc (right):

http://codereview.chromium.org/344018/diff/5001/5005#newcode1070
Line 1070: if
(WebKit::webKitClient()->mimeRegistry()->supportsMIMEType(webkit_glue::StringToWebString(mime_type))
==
nit: this exceeds 80 chars, but... don't worry about it.

why?  because this file no longer exists.  it was moved to
webkit/api/src/FrameLoaderClientImpl.cpp just now.  sorry to cause you more work
:(

[Mohamed Mansour, 2009-10-30 04:53:42]

> nit: this exceeds 80 chars, but... don't worry about it.
Thats is what I am confused the stuff there are not respecting Chromium
guidelines (more Webkit) so I didn't know what to do. I will use Chromium
guidelines from now on.

> why?  because this file no longer exists.  it was moved to
> webkit/api/src/FrameLoaderClientImpl.cpp just now.  sorry to cause you more
work
Np, I just saw your commit, I will do this tomorrow night. In the meantime, what
should GURL be transformed into? I can remove those as well. Something like KURL
? WebURL ?

[darin (slow to review), 2009-10-30 05:11:07]

http://codereview.chromium.org/344018/diff/5001/5005
File webkit/glue/webframeloaderclient_impl.cc (right):

http://codereview.chromium.org/344018/diff/5001/5005#newcode1070
Line 1070: if
(WebKit::webKitClient()->mimeRegistry()->supportsMIMEType(webkit_glue::StringToWebString(mime_type))
==
once you move this into FrameLoaderClientImpl.cpp, you'll be able to just pass a
WebCore::String type to a function taking a WebKit::WebString, so the
webkit_glue:: helper function used above just goes away.

[Mohamed Mansour, 2009-10-31 02:38:54]

I rebased, you can recheck if its what you want.

[darin (slow to review), 2009-10-31 22:35:49]

LGTM

http://codereview.chromium.org/344018/diff/8001/8003
File webkit/api/src/FrameLoaderClientImpl.cpp (right):

http://codereview.chromium.org/344018/diff/8001/8003#newcode1121
Line 1121: if
(WebKit::webKitClient()->mimeRegistry()->supportsMIMEType(mimeType) ==
feel free to not line wrap here since webkit style does not impose a 80 char
line limit.  (webkit developers would normally not line wrap here)

[jam, 2009-11-01 02:01:45]

Why did this change get committed?  It was red on the trybots, and these same
tests are now failing on the buildbot.  Please revert asap, as it's not cool
leaving the tree red.