From 15e2b2335ffc8667b349f8c0d653b44a0ef09700 Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Sun, 12 Mar 2017 03:33:26 +0530 Subject: [PATCH] address review comments --- .../atom_resource_dispatcher_host_delegate.cc | 23 +++++++++------------- .../atom_resource_dispatcher_host_delegate.h | 2 -- atom/browser/ui/webui/pdf_viewer_handler.cc | 15 ++++++-------- atom/browser/ui/webui/pdf_viewer_handler.h | 1 - atom/browser/ui/webui/pdf_viewer_ui.cc | 6 +++--- 5 files changed, 18 insertions(+), 29 deletions(-) diff --git a/atom/browser/atom_resource_dispatcher_host_delegate.cc b/atom/browser/atom_resource_dispatcher_host_delegate.cc index 42b1bd4..abd2fb5 100644 --- a/atom/browser/atom_resource_dispatcher_host_delegate.cc +++ b/atom/browser/atom_resource_dispatcher_host_delegate.cc @@ -14,6 +14,7 @@ #include "content/public/browser/stream_info.h" #include "net/base/escape.h" #include "net/ssl/client_cert_store.h" +#include "net/url_request/url_request.h" #include "url/gurl.h" #if defined(USE_NSS_CERTS) @@ -60,9 +61,10 @@ void HandleExternalProtocolInUI( permission_helper->RequestOpenExternalPermission(callback, has_user_gesture); } -void OnPdfStreamCreated(const GURL& original_url, - const content::ResourceRequestInfo::WebContentsGetter& - web_contents_getter) { +void OnPdfResourceIntercepted( + const GURL& original_url, + const content::ResourceRequestInfo::WebContentsGetter& + web_contents_getter) { content::WebContents* web_contents = web_contents_getter.Run(); if (!web_contents) return; @@ -124,20 +126,13 @@ bool AtomResourceDispatcherHostDelegate::ShouldInterceptResourceAsStream( content::ResourceRequestInfo::ForRequest(request); if (mime_type == "application/pdf" && info->IsMainFrame()) { *origin = GURL(kPdfViewerUIOrigin); + content::BrowserThread::PostTask( + BrowserThread::UI, FROM_HERE, + base::Bind(&OnPdfResourceIntercepted, request->url(), + info->GetWebContentsGetterForRequest())); return true; } return false; } -void AtomResourceDispatcherHostDelegate::OnStreamCreated( - net::URLRequest* request, - std::unique_ptr stream) { - const content::ResourceRequestInfo* info = - content::ResourceRequestInfo::ForRequest(request); - content::BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - base::Bind(&OnPdfStreamCreated, stream->original_url, - info->GetWebContentsGetterForRequest())); -} - } // namespace atom diff --git a/atom/browser/atom_resource_dispatcher_host_delegate.h b/atom/browser/atom_resource_dispatcher_host_delegate.h index b4d7780..eda744d 100644 --- a/atom/browser/atom_resource_dispatcher_host_delegate.h +++ b/atom/browser/atom_resource_dispatcher_host_delegate.h @@ -29,8 +29,6 @@ class AtomResourceDispatcherHostDelegate const std::string& mime_type, GURL* origin, std::string* payload) override; - void OnStreamCreated(net::URLRequest* request, - std::unique_ptr stream) override; private: DISALLOW_COPY_AND_ASSIGN(AtomResourceDispatcherHostDelegate); diff --git a/atom/browser/ui/webui/pdf_viewer_handler.cc b/atom/browser/ui/webui/pdf_viewer_handler.cc index c369e2d..be2f337 100644 --- a/atom/browser/ui/webui/pdf_viewer_handler.cc +++ b/atom/browser/ui/webui/pdf_viewer_handler.cc @@ -45,8 +45,7 @@ void CreateResponseHeadersDictionary(const net::HttpResponseHeaders* headers, void PopulateStreamInfo(base::DictionaryValue* stream_info, content::StreamInfo* stream, const std::string& original_url) { - std::unique_ptr headers_dict( - new base::DictionaryValue); + auto headers_dict = base::MakeUnique(); auto stream_url = stream->handle->GetURL().spec(); CreateResponseHeadersDictionary(stream->response_headers.get(), headers_dict.get()); @@ -58,17 +57,16 @@ void PopulateStreamInfo(base::DictionaryValue* stream_info, } // namespace PdfViewerHandler::PdfViewerHandler(const std::string& src) - : stream_(nullptr), original_url_(src), initialized_(true) {} + : stream_(nullptr), original_url_(src) {} PdfViewerHandler::~PdfViewerHandler() {} void PdfViewerHandler::SetPdfResourceStream(content::StreamInfo* stream) { stream_ = stream; - if (!initialized_) { + if (!!initialize_callback_id_.get()) { auto list = base::MakeUnique(); list->Set(0, std::move(initialize_callback_id_)); Initialize(list.get()); - initialized_ = true; } } @@ -110,15 +108,14 @@ void PdfViewerHandler::Initialize(const base::ListValue* args) { CHECK(args->Get(0, &callback_id)); if (stream_) { + CHECK(!initialize_callback_id_.get()); AllowJavascript(); - std::unique_ptr stream_info( - new base::DictionaryValue); + auto stream_info = base::MakeUnique(); PopulateStreamInfo(stream_info.get(), stream_, original_url_); ResolveJavascriptCallback(*callback_id, *stream_info); } else { initialize_callback_id_ = callback_id->CreateDeepCopy(); - initialized_ = false; } } @@ -172,7 +169,7 @@ void PdfViewerHandler::GetStrings(const base::ListValue* args) { const base::Value* callback_id; CHECK(args->Get(0, &callback_id)); - std::unique_ptr result(new base::DictionaryValue); + auto result = base::MakeUnique(); // TODO(deepak1556): Generate strings from components/pdf_strings.grdp. #define SET_STRING(id, resource) result->SetString(id, resource) SET_STRING("passwordPrompt", diff --git a/atom/browser/ui/webui/pdf_viewer_handler.h b/atom/browser/ui/webui/pdf_viewer_handler.h index 70274e3..7576b06 100644 --- a/atom/browser/ui/webui/pdf_viewer_handler.h +++ b/atom/browser/ui/webui/pdf_viewer_handler.h @@ -49,7 +49,6 @@ class PdfViewerHandler : public content::WebUIMessageHandler { std::unique_ptr initialize_callback_id_; content::StreamInfo* stream_; std::string original_url_; - bool initialized_; DISALLOW_COPY_AND_ASSIGN(PdfViewerHandler); }; diff --git a/atom/browser/ui/webui/pdf_viewer_ui.cc b/atom/browser/ui/webui/pdf_viewer_ui.cc index f233f91..580d183 100644 --- a/atom/browser/ui/webui/pdf_viewer_ui.cc +++ b/atom/browser/ui/webui/pdf_viewer_ui.cc @@ -141,9 +141,9 @@ class PdfViewerUI::ResourceRequester content::StreamContext* stream_context = content::GetStreamContextForResourceContext(resource_context); - std::unique_ptr handler( - new content::StreamResourceHandler(request.get(), - stream_context->registry(), origin)); + std::unique_ptr handler = + base::MakeUnique( + request.get(), stream_context->registry(), origin); info->set_is_stream(true); stream_info_.reset(new content::StreamInfo); stream_info_->handle = -- 2.7.4