Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(5218)

Unified Diff: chrome/browser/ui/webui/nacl_ui.cc

Issue 14860020: chrome://nacl asserts (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 7 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/ui/webui/nacl_ui.cc
diff --git a/chrome/browser/ui/webui/nacl_ui.cc b/chrome/browser/ui/webui/nacl_ui.cc
index fd84893e8ea6f7a4d79da59dbc347b3aec6c2924..4edc7fa2d4989b0466d2a58e1567a5b55a4e97cc 100644
--- a/chrome/browser/ui/webui/nacl_ui.cc
+++ b/chrome/browser/ui/webui/nacl_ui.cc
@@ -40,6 +40,7 @@
#include "base/win/windows_version.h"
#endif
+using content::BrowserThread;
using content::PluginService;
using content::UserMetricsAction;
using content::WebUIMessageHandler;
@@ -66,6 +67,28 @@ content::WebUIDataSource* CreateNaClUIHTMLSource() {
//
////////////////////////////////////////////////////////////////////////////////
+class NaClDOMHandler;
+
+class NaClDOMHandlerProxy : public
+ base::RefCountedThreadSafe<NaClDOMHandlerProxy> {
+ public:
+ explicit NaClDOMHandlerProxy(NaClDOMHandler* handler);
+ // A helper to check if pnacl path exists.
+ // The check is done on the FILE thread.
+ void validatePnaclPath();
jvoung (off chromium) 2013/05/15 20:28:28 style: Method names start with capital letter?
+
+ void setHandler(NaClDOMHandler* handler);
+
+ private:
+ // A helper callback that receives the result of checking if pnacl path
jvoung (off chromium) 2013/05/15 20:28:28 In the comments, could you make it "PNaCl" instead
+ // exists. It switches back to the UI thread and saves the result.
+ void validatePnaclPathCallback(bool is_valid);
+
+ NaClDOMHandler* handler_;
+
+ DISALLOW_COPY_AND_ASSIGN(NaClDOMHandlerProxy);
+};
+
// The handler for JavaScript messages for the about:flags page.
class NaClDOMHandler : public WebUIMessageHandler {
public:
@@ -81,6 +104,10 @@ class NaClDOMHandler : public WebUIMessageHandler {
// Callback for the NaCl plugin information.
void OnGotPlugins(const std::vector<webkit::WebPluginInfo>& plugins);
+ // A helper callback that receives the result of checking if pnacl path
+ // exists. It switches back to the UI thread and saves the result.
+ void validatePnaclPathCallback(bool is_valid);
jvoung (off chromium) 2013/05/15 20:28:28 style: Method names start with capital letter
+
private:
// Called when enough information is gathered to return data back to the page.
void MaybeRespondToPage();
@@ -98,18 +125,74 @@ class NaClDOMHandler : public WebUIMessageHandler {
// Whether the plugin information is ready.
bool has_plugin_info_;
+ // Whether pnacl path was validated. PathService can return a path
+ // that does not exists, so it needs to be validated.
+ bool pnacl_path_validated_;
+ bool pnacl_path_exists_;
+
+ // A proxy for handling cross threads messages.
+ scoped_refptr<NaClDOMHandlerProxy> proxy_;
+
DISALLOW_COPY_AND_ASSIGN(NaClDOMHandler);
};
+NaClDOMHandlerProxy::NaClDOMHandlerProxy(NaClDOMHandler* handler)
+ : handler_(handler) {
+}
+
+void NaClDOMHandlerProxy::validatePnaclPath()
+{
jvoung (off chromium) 2013/05/15 20:28:28 put curly brace on previous line
+ if (!BrowserThread::CurrentlyOn(BrowserThread::FILE)) {
+ BrowserThread::PostTask(
+ BrowserThread::FILE, FROM_HERE,
+ base::Bind(&NaClDOMHandlerProxy::validatePnaclPath, this));
+ return;
+ }
+
+ bool is_valid = true;
+ base::FilePath pnacl_path;
+ bool got_path = PathService::Get(chrome::DIR_PNACL_COMPONENT, &pnacl_path);
+ // The PathService may return an empty string if PNaCl is not yet installed.
+ // However, do not trust that the path returned by the PathService exists.
+ // Check for existence here.
+ if (!got_path || pnacl_path.empty() || !file_util::PathExists(pnacl_path)) {
+ is_valid = false;
+ }
+ handler_->validatePnaclPathCallback(is_valid);
+}
+
+void NaClDOMHandlerProxy::validatePnaclPathCallback(bool is_valid)
+{
jvoung (off chromium) 2013/05/15 20:28:28 same (curly)
+ if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
+ BrowserThread::PostTask(
+ BrowserThread::UI, FROM_HERE,
+ base::Bind(&NaClDOMHandlerProxy::validatePnaclPathCallback,
+ this, is_valid));
+ return;
+ }
+
+ handler_->validatePnaclPathCallback(is_valid);
+}
+
+void NaClDOMHandlerProxy::setHandler(NaClDOMHandler* handler) {
+ handler_ = handler;
+}
+
NaClDOMHandler::NaClDOMHandler()
: weak_ptr_factory_(this),
page_has_requested_data_(false),
- has_plugin_info_(false) {
+ has_plugin_info_(false),
+ pnacl_path_validated_(false),
+ pnacl_path_exists_(false),
+ proxy_(new NaClDOMHandlerProxy(this)) {
PluginService::GetInstance()->GetPlugins(base::Bind(
&NaClDOMHandler::OnGotPlugins, weak_ptr_factory_.GetWeakPtr()));
}
NaClDOMHandler::~NaClDOMHandler() {
+ if (proxy_) {
jvoung (off chromium) 2013/05/15 20:28:28 indentation 2 spaces instead of 4
+ proxy_->setHandler(NULL);
+ }
}
void NaClDOMHandler::RegisterMessages() {
@@ -154,6 +237,7 @@ void NaClDOMHandler::OnGotPlugins(
}
void NaClDOMHandler::PopulatePageInformation(DictionaryValue* naclInfo) {
+ DCHECK(pnacl_path_validated_);
// Store Key-Value pairs of about-information.
scoped_ptr<ListValue> list(new ListValue());
@@ -233,10 +317,7 @@ void NaClDOMHandler::PopulatePageInformation(DictionaryValue* naclInfo) {
// Obtain the version of the PNaCl translator.
base::FilePath pnacl_path;
bool got_path = PathService::Get(chrome::DIR_PNACL_COMPONENT, &pnacl_path);
- // The PathService may return an empty string if PNaCl is not yet installed.
- // However, do not trust that the path returned by the PathService exists.
- // Check for existence here.
- if (!got_path || pnacl_path.empty() || !file_util::PathExists(pnacl_path)) {
+ if (!got_path || pnacl_path.empty() || !pnacl_path_exists_) {
AddPair(list.get(),
ASCIIToUTF16("PNaCl translator"),
ASCIIToUTF16("Not installed"));
@@ -254,12 +335,25 @@ void NaClDOMHandler::PopulatePageInformation(DictionaryValue* naclInfo) {
naclInfo->Set("naclInfo", list.release());
}
+void NaClDOMHandler::validatePnaclPathCallback(bool is_valid)
+{
jvoung (off chromium) 2013/05/15 20:28:28 style: put curly on the previous line
+ pnacl_path_validated_ = true;
+ pnacl_path_exists_ = is_valid;
+ MaybeRespondToPage();
+}
+
void NaClDOMHandler::MaybeRespondToPage() {
// Don't reply until everything is ready. The page will show a 'loading'
// message until then.
if (!page_has_requested_data_ || !has_plugin_info_)
return;
+ if (!pnacl_path_validated_) {
+ DCHECK(proxy_);
jvoung (off chromium) 2013/05/15 20:28:28 make DCHECK indentation consistent with the rest o
+ proxy_->validatePnaclPath();
+ return;
+ }
+
DictionaryValue naclInfo;
PopulatePageInformation(&naclInfo);
web_ui()->CallJavascriptFunction("nacl.returnNaClInfo", naclInfo);
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698