0

Remove MimeHandlerService::AbortStream

This CL removes MimeHandlerService::AbortStream and all uses of it,
since it's a no-op. The pdf viewer currently calls it via
CancelBrowserDownload, which this CL also removes. The pdf request
actually gets stopped by resetting the chrome_pdf::URLLoaderWrapper,
which closes the connection on desctruction.

Bug: 934009
Change-Id: Ib3cc6fb1aa12270d39c16a6770d3726a3198682c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1696444
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Cr-Commit-Position: refs/heads/master@{#677545}
This commit is contained in:
Robbie McElrath
2019-07-15 23:28:55 +00:00
committed by Commit Bot
parent c9fd4b6d43
commit 44e2d300a1
22 changed files with 1 additions and 112 deletions

@ -88,15 +88,6 @@ class BrowserApi {
return this.streamInfo_; return this.streamInfo_;
} }
/**
* Aborts the stream.
*/
abortStream() {
if (chrome.mimeHandlerPrivate) {
chrome.mimeHandlerPrivate.abortStream();
}
}
/** /**
* Sets the browser zoom. * Sets the browser zoom.
* *

@ -1619,9 +1619,6 @@ class PluginController extends ContentController {
case 'scrollBy': case 'scrollBy':
this.viewport_.scrollBy(/** @type {!Point} */ (message.data)); this.viewport_.scrollBy(/** @type {!Point} */ (message.data));
break; break;
case 'cancelStreamUrl':
chrome.mimeHandlerPrivate.abortStream();
break;
case 'metadata': case 'metadata':
this.viewer_.setDocumentMetadata( this.viewer_.setDocumentMetadata(
message.data.title, message.data.bookmarks, message.data.title, message.data.bookmarks,

@ -105,17 +105,6 @@ var tests = [
.then(chrome.test.succeed); .then(chrome.test.succeed);
}, },
function testAbort() {
checkStreamDetails('testAbort.csv', false);
chrome.mimeHandlerPrivate.abortStream(function() {
fetchUrl(streamDetails.streamUrl).then(function(response) {
chrome.test.assertEq(0, response.status);
chrome.test.assertEq('error', response.data);
chrome.test.succeed();
});
});
},
function testNonAsciiHeaders() { function testNonAsciiHeaders() {
checkStreamDetails('testNonAsciiHeaders.csv', false); checkStreamDetails('testNonAsciiHeaders.csv', false);
chrome.test.assertEq(undefined, chrome.test.assertEq(undefined,

@ -1 +0,0 @@
Should not be read.
1 Should not be read.

@ -1,2 +0,0 @@
HTTP/1.0 200 OK
Content-Type: text/csv

@ -71,20 +71,6 @@ void MimeHandlerServiceImpl::GetStreamInfo(GetStreamInfoCallback callback) {
mojo::ConvertTo<mime_handler::StreamInfoPtr>(*stream_)); mojo::ConvertTo<mime_handler::StreamInfoPtr>(*stream_));
} }
void MimeHandlerServiceImpl::AbortStream(AbortStreamCallback callback) {
if (!stream_) {
std::move(callback).Run();
return;
}
stream_->Abort(base::Bind(&MimeHandlerServiceImpl::OnStreamClosed,
weak_factory_.GetWeakPtr(),
base::Passed(&callback)));
}
void MimeHandlerServiceImpl::OnStreamClosed(AbortStreamCallback callback) {
std::move(callback).Run();
}
} // namespace extensions } // namespace extensions
namespace mojo { namespace mojo {

@ -27,10 +27,6 @@ class MimeHandlerServiceImpl : public mime_handler::MimeHandlerService {
// mime_handler::MimeHandlerService overrides. // mime_handler::MimeHandlerService overrides.
void GetStreamInfo(GetStreamInfoCallback callback) override; void GetStreamInfo(GetStreamInfoCallback callback) override;
void AbortStream(AbortStreamCallback callback) override;
// Invoked by the callback used to abort |stream_|.
void OnStreamClosed(AbortStreamCallback callback);
// A handle to the stream being handled by the MimeHandlerViewGuest. // A handle to the stream being handled by the MimeHandlerViewGuest.
base::WeakPtr<StreamContainer> stream_; base::WeakPtr<StreamContainer> stream_;

@ -42,7 +42,6 @@
#include "net/dns/mock_host_resolver.h" #include "net/dns/mock_host_resolver.h"
#include "net/test/embedded_test_server/embedded_test_server.h" #include "net/test/embedded_test_server/embedded_test_server.h"
#include "net/test/embedded_test_server/http_request.h" #include "net/test/embedded_test_server/http_request.h"
#include "services/network/public/cpp/features.h"
#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h" #include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
#include "ui/base/ui_base_features.h" #include "ui/base/ui_base_features.h"
#include "url/url_constants.h" #include "url/url_constants.h"
@ -415,21 +414,6 @@ IN_PROC_BROWSER_TEST_P(MimeHandlerViewCrossProcessTest, Iframe) {
RunTest("test_iframe.html"); RunTest("test_iframe.html");
} }
IN_PROC_BROWSER_TEST_P(MimeHandlerViewCrossProcessTest, Abort) {
if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
// With the network service, abortStream isn't needed since we pass a Mojo
// pipe to the renderer. If the plugin chooses to cancel the main request
// (e.g. to make range requests instead), we are always guaranteed that the
// Mojo pipe will be broken which will cancel the request. This is different
// than without the network service, since stream URLs need to be explicitly
// closed if they weren't yet opened to avoid leaks.
// TODO(jam): once the network service is the only path, delete the
// abortStream mimeHandlerPrivate method and supporting code.
return;
}
RunTest("testAbort.csv");
}
IN_PROC_BROWSER_TEST_P(MimeHandlerViewCrossProcessTest, NonAsciiHeaders) { IN_PROC_BROWSER_TEST_P(MimeHandlerViewCrossProcessTest, NonAsciiHeaders) {
RunTest("testNonAsciiHeaders.csv"); RunTest("testNonAsciiHeaders.csv");
} }

@ -62,10 +62,6 @@ StreamContainer::StreamContainer(
StreamContainer::~StreamContainer() { StreamContainer::~StreamContainer() {
} }
void StreamContainer::Abort(const base::Closure& callback) {
callback.Run();
}
base::WeakPtr<StreamContainer> StreamContainer::GetWeakPtr() { base::WeakPtr<StreamContainer> StreamContainer::GetWeakPtr() {
return weak_factory_.GetWeakPtr(); return weak_factory_.GetWeakPtr();
} }

@ -36,9 +36,6 @@ class StreamContainer {
const GURL& original_url); const GURL& original_url);
~StreamContainer(); ~StreamContainer();
// Aborts the stream.
void Abort(const base::Closure& callback);
base::WeakPtr<StreamContainer> GetWeakPtr(); base::WeakPtr<StreamContainer> GetWeakPtr();
content::mojom::TransferrableURLLoaderPtr TakeTransferrableURLLoader(); content::mojom::TransferrableURLLoaderPtr TakeTransferrableURLLoader();

@ -33,11 +33,6 @@ interface MimeHandlerService {
// Returns information about the stream associated with this service instance. // Returns information about the stream associated with this service instance.
// If the stream has been aborted, |stream_info| will be null. // If the stream has been aborted, |stream_info| will be null.
GetStreamInfo() => (StreamInfo? stream_info); GetStreamInfo() => (StreamInfo? stream_info);
// Aborts the stream associated with this service instance. This is an
// idempotent operation.
AbortStream() => ();
}; };
// Provides a mime handler guest with control over beforeunload event handling // Provides a mime handler guest with control over beforeunload event handling

@ -28,7 +28,6 @@
boolean embedded; boolean embedded;
}; };
callback AbortCallback = void ();
callback GetStreamDetailsCallback = void (StreamInfo streamInfo); callback GetStreamDetailsCallback = void (StreamInfo streamInfo);
callback SetShowBeforeUnloadDialogCallback = void (); callback SetShowBeforeUnloadDialogCallback = void ();
@ -36,9 +35,6 @@
// Returns the StreamInfo for the stream for this context if there is one. // Returns the StreamInfo for the stream for this context if there is one.
[nocompile] static void getStreamInfo(GetStreamDetailsCallback callback); [nocompile] static void getStreamInfo(GetStreamDetailsCallback callback);
// Aborts the stream for this context if there is one.
[nocompile] static void abortStream(optional AbortCallback callback);
// Instructs the PluginDocument, if running in one, to show a dialog in // Instructs the PluginDocument, if running in one, to show a dialog in
// response to beforeunload events. // response to beforeunload events.
[nocompile] static void setShowBeforeUnloadDialog( [nocompile] static void setShowBeforeUnloadDialog(

@ -67,12 +67,6 @@ apiBridge.registerCustomHook(function(bindingsAPI) {
return streamInfoPromise.then(constructStreamInfoDict); return streamInfoPromise.then(constructStreamInfoDict);
}); });
utils.handleRequestWithPromiseDoNotUse(
apiFunctions, 'mimeHandlerPrivate', 'abortStream',
function() {
return servicePtr.abortStream().then(function() {});
});
utils.handleRequestWithPromiseDoNotUse( utils.handleRequestWithPromiseDoNotUse(
apiFunctions, 'mimeHandlerPrivate', 'setShowBeforeUnloadDialog', apiFunctions, 'mimeHandlerPrivate', 'setShowBeforeUnloadDialog',
function(showDialog) { function(showDialog) {

@ -37,8 +37,6 @@ class DocumentLoader {
virtual void OnDocumentComplete() = 0; virtual void OnDocumentComplete() = 0;
// Notification called when document loading is canceled. // Notification called when document loading is canceled.
virtual void OnDocumentCanceled() = 0; virtual void OnDocumentCanceled() = 0;
// Called when initial loader was closed.
virtual void CancelBrowserDownload() = 0;
}; };
virtual ~DocumentLoader() = default; virtual ~DocumentLoader() = default;

@ -250,10 +250,7 @@ void DocumentLoaderImpl::ContinueDownload() {
loader_.reset(); loader_.reset();
chunk_.Clear(); chunk_.Clear();
if (!is_partial_loader_active_) { is_partial_loader_active_ = true;
client_->CancelBrowserDownload();
is_partial_loader_active_ = true;
}
const uint32_t start = next_request.start() * DataStream::kChunkSize; const uint32_t start = next_request.start() * DataStream::kChunkSize;
const uint32_t length = const uint32_t length =

@ -207,7 +207,6 @@ class TestClient : public DocumentLoader::Client {
void OnNewDataReceived() override {} void OnNewDataReceived() override {}
void OnDocumentComplete() override {} void OnDocumentComplete() override {}
void OnDocumentCanceled() override {} void OnDocumentCanceled() override {}
void CancelBrowserDownload() override {}
std::unique_ptr<URLLoaderWrapper> CreateFullPageLoader() { std::unique_ptr<URLLoaderWrapper> CreateFullPageLoader() {
return std::unique_ptr<URLLoaderWrapper>( return std::unique_ptr<URLLoaderWrapper>(

@ -132,8 +132,6 @@ constexpr char kJSPositionX[] = "x";
constexpr char kJSPositionY[] = "y"; constexpr char kJSPositionY[] = "y";
// Scroll by (Plugin -> Page) // Scroll by (Plugin -> Page)
constexpr char kJSScrollByType[] = "scrollBy"; constexpr char kJSScrollByType[] = "scrollBy";
// Cancel the stream URL request (Plugin -> Page)
constexpr char kJSCancelStreamUrlType[] = "cancelStreamUrl";
// Navigate to the given URL (Plugin -> Page) // Navigate to the given URL (Plugin -> Page)
constexpr char kJSNavigateType[] = "navigate"; constexpr char kJSNavigateType[] = "navigate";
constexpr char kJSNavigateUrl[] = "url"; constexpr char kJSNavigateUrl[] = "url";
@ -1925,12 +1923,6 @@ uint32_t OutOfProcessInstance::GetBackgroundColor() {
return background_color_; return background_color_;
} }
void OutOfProcessInstance::CancelBrowserDownload() {
pp::VarDictionary message;
message.Set(kType, kJSCancelStreamUrlType);
PostMessage(message);
}
void OutOfProcessInstance::IsSelectingChanged(bool is_selecting) { void OutOfProcessInstance::IsSelectingChanged(bool is_selecting) {
pp::VarDictionary message; pp::VarDictionary message;
message.Set(kType, kJSSetIsSelectingType); message.Set(kType, kJSSetIsSelectingType);

@ -141,7 +141,6 @@ class OutOfProcessInstance : public pp::Instance,
void FormTextFieldFocusChange(bool in_focus) override; void FormTextFieldFocusChange(bool in_focus) override;
bool IsPrintPreview() override; bool IsPrintPreview() override;
uint32_t GetBackgroundColor() override; uint32_t GetBackgroundColor() override;
void CancelBrowserDownload() override;
void IsSelectingChanged(bool is_selecting) override; void IsSelectingChanged(bool is_selecting) override;
void SelectionChanged(const pp::Rect& left, const pp::Rect& right) override; void SelectionChanged(const pp::Rect& left, const pp::Rect& right) override;
void IsEditModeChanged(bool is_edit_mode) override; void IsEditModeChanged(bool is_edit_mode) override;

@ -259,9 +259,6 @@ class PDFEngine {
// Get the background color of the PDF. // Get the background color of the PDF.
virtual uint32_t GetBackgroundColor() = 0; virtual uint32_t GetBackgroundColor() = 0;
// Cancel browser initiated document download.
virtual void CancelBrowserDownload() {}
// Sets selection status. // Sets selection status.
virtual void IsSelectingChanged(bool is_selecting) {} virtual void IsSelectingChanged(bool is_selecting) {}

@ -869,10 +869,6 @@ void PDFiumEngine::OnDocumentCanceled() {
OnDocumentComplete(); OnDocumentComplete();
} }
void PDFiumEngine::CancelBrowserDownload() {
client_->CancelBrowserDownload();
}
void PDFiumEngine::FinishLoadingDocument() { void PDFiumEngine::FinishLoadingDocument() {
DCHECK(doc()); DCHECK(doc());
DCHECK(doc_loader_->IsDocumentComplete()); DCHECK(doc_loader_->IsDocumentComplete());

@ -137,7 +137,6 @@ class PDFiumEngine : public PDFEngine,
void OnNewDataReceived() override; void OnNewDataReceived() override;
void OnDocumentComplete() override; void OnDocumentComplete() override;
void OnDocumentCanceled() override; void OnDocumentCanceled() override;
void CancelBrowserDownload() override;
void KillFormFocus() override; void KillFormFocus() override;
uint32_t GetLoadedByteSize() override; uint32_t GetLoadedByteSize() override;
bool ReadLoadedBytes(uint32_t length, void* buffer) override; bool ReadLoadedBytes(uint32_t length, void* buffer) override;

@ -34,12 +34,6 @@ chrome.mimeHandlerPrivate.StreamInfo;
*/ */
chrome.mimeHandlerPrivate.getStreamInfo = function(callback) {}; chrome.mimeHandlerPrivate.getStreamInfo = function(callback) {};
/**
* Aborts the stream for this context if there is one.
* @param {function():void=} callback
*/
chrome.mimeHandlerPrivate.abortStream = function(callback) {};
/** /**
* Instructs the PluginDocument, if running in one, to show a dialog in response * Instructs the PluginDocument, if running in one, to show a dialog in response
* to beforeunload events. * to beforeunload events.