Change WebURLLoaderClient::willFollowRedirect() API to return bool

third_party/WebKit/Source/core/loader/DocumentLoader.cpp

 /*
  * Copyright (C) 2006, 2007, 2008 Apple Inc. All rights reserved.
  * Copyright (C) 2011 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
@@ skipping 303 matching lines @@
     if (!m_frame)
         return;
 
     m_applicationCacheHost->finishedLoadingMainResource();
     endWriting(m_writer.get());
     if (m_state < MainResourceDone)
         m_state = MainResourceDone;
     clearMainResourceHandle();
 }
 
-void DocumentLoader::redirectReceived(Resource* resource, ResourceRequest& request, const ResourceResponse& redirectResponse)
+bool DocumentLoader::redirectReceived(Resource* resource, ResourceRequest& request, const ResourceResponse& redirectResponse)
 {
     ASSERT_UNUSED(resource, resource == m_mainResource);
     ASSERT(!redirectResponse.isNull());
     m_request = request;
 
     // If the redirecting url is not allowed to display content from the target origin,
     // then block the redirect.
     const KURL& requestURL = m_request.url();
     RefPtr<SecurityOrigin> redirectingOrigin = SecurityOrigin::create(redirectResponse.url());
     if (!redirectingOrigin->canDisplay(requestURL)) {
         FrameLoader::reportLocalLoadFailed(m_frame, requestURL.getString());
         m_fetcher->stopFetching();
-        return;
+        return true;

[Nate Chapin, 2016/09/06 16:46:14]

If you return false here, you can probably remove the stopFetching() call and
get the same result.

[tyoshino (SeeGerritForStatus), 2016/09/07 10:52:28]

I agree that we can change the return value here and at L341 to false.

For the same reason as I wrote for the comment on didFail(), I'm not sure if
it's fine to omit it.

[Nate Chapin, 2016/09/07 17:05:45]

Hrm, maybe ResourceLoader::willFollowRedirect() should guarantee that it calls
didFail() any time it returns false? Requiring ResourceClients to do that work
seems unreliable.

[tyoshino (SeeGerritForStatus), 2016/09/14 14:53:47]

Any issue with only failing this RawResource? At this point, the main resource
is not yet loaded, but isn't it possible that m_fetcher is already used for
loading some subresources given e.g. some pre-load/fetch-ing directives, etc.?

[Nate Chapin, 2016/09/14 22:25:27]

Subresources can only be loaded once this DocumentLoader is committed. Redirects
are guaranteed to happen before commit, so you should have a strong guarantee
that the main resource is the only fetch in progress.

[tyoshino (SeeGerritForStatus), 2016/09/16 07:19:58]

OK. Removing stopFetching() calls here and at L340.

     }
     if (!frameLoader()->shouldContinueForNavigationPolicy(m_request, SubstituteData(), this, CheckContentSecurityPolicy, m_navigationType, NavigationPolicyCurrentTab, replacesCurrentHistoryItem(), isClientRedirect())) {
         m_fetcher->stopFetching();
-        return;
+        return true;
     }
 
     ASSERT(timing().fetchStart());
     appendRedirect(requestURL);
     didRedirect(redirectResponse.url(), requestURL);
     frameLoader()->client()->dispatchDidReceiveServerRedirectForProvisionalLoad();
+
+    return true;
 }
 
 static bool canShowMIMEType(const String& mimeType, LocalFrame* frame)
 {
     if (Platform::current()->mimeRegistry()->supportsMIMEType(mimeType) == WebMimeRegistry::IsSupported)
         return true;
     PluginData* pluginData = frame->pluginData();
     return !mimeType.isEmpty() && pluginData && pluginData->supportsMimeType(mimeType);
 }
 
@@ skipping 373 matching lines @@
 {
     m_writer = createWriterFor(init, mimeType(), m_writer ? m_writer->encoding() : emptyAtom, true, ForceSynchronousParsing);
     if (!source.isNull())
         m_writer->appendReplacingData(source);
     endWriting(m_writer.get());
 }
 
 DEFINE_WEAK_IDENTIFIER_MAP(DocumentLoader);
 
 } // namespace blink