Skip to content

Commit

Permalink
Fix crash during the sync page loading in tor(guest) window
Browse files Browse the repository at this point in the history
Sync page should be disabled for now because BraveSyncService isn't
instantiated for OTR profile.
Instead, sync is disabled page is loaded.
  • Loading branch information
simonhong committed Feb 18, 2019
1 parent 7689d49 commit 55eb56a
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 9 deletions.
19 changes: 15 additions & 4 deletions browser/ui/webui/sync/sync_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class SyncUIDOMHandler : public WebUIMessageHandler,
std::unique_ptr<brave_sync::Settings> settings,
std::unique_ptr<brave_sync::SyncDevices> devices);

brave_sync::BraveSyncService *sync_service_; // NOT OWNED
brave_sync::BraveSyncService *sync_service_ = nullptr; // NOT OWNED

base::WeakPtrFactory<SyncUIDOMHandler> weak_ptr_factory_;

Expand Down Expand Up @@ -119,11 +119,12 @@ void SyncUIDOMHandler::RegisterMessages() {
void SyncUIDOMHandler::Init() {
Profile* profile = Profile::FromWebUI(web_ui());
sync_service_ = brave_sync::BraveSyncServiceFactory::GetForProfile(profile);
DCHECK(sync_service_);
sync_service_->AddObserver(this);
if (sync_service_)
sync_service_->AddObserver(this);
}

void SyncUIDOMHandler::SetupSyncHaveCode(const base::ListValue* args) {
DCHECK(sync_service_);
std::string sync_words, device_name;
if (!args->GetString(0, &sync_words) || !args->GetString(1, &device_name))
// TODO(bridiver) - does this need to report an error?
Expand All @@ -133,6 +134,7 @@ void SyncUIDOMHandler::SetupSyncHaveCode(const base::ListValue* args) {
}

void SyncUIDOMHandler::SetupSyncNewToSync(const base::ListValue* args) {
DCHECK(sync_service_);
std::string sync_words, device_name;
if (!args->GetString(0, &device_name))
// TODO(bridiver) - does this need to report an error?
Expand All @@ -142,20 +144,24 @@ void SyncUIDOMHandler::SetupSyncNewToSync(const base::ListValue* args) {
}

void SyncUIDOMHandler::PageLoaded(const base::ListValue* args) {
DCHECK(sync_service_);
LoadSyncSettingsView();
}

void SyncUIDOMHandler::NeedSyncWords(const base::ListValue* args) {
DCHECK(sync_service_);
sync_service_->GetSyncWords();
}

void SyncUIDOMHandler::NeedSyncQRcode(const base::ListValue* args) {
DCHECK(sync_service_);
std::string seed = sync_service_->GetSeed();
web_ui()->CallJavascriptFunctionUnsafe(
"sync_ui_exports.haveSeedForQrCode", base::Value(seed));
}

void SyncUIDOMHandler::SyncBookmarks(const base::ListValue* args) {
DCHECK(sync_service_);
bool new_value;
if (!args->GetBoolean(0, &new_value))
// TODO(bridiver) - does this need to report an error?
Expand All @@ -165,6 +171,7 @@ void SyncUIDOMHandler::SyncBookmarks(const base::ListValue* args) {
}

void SyncUIDOMHandler::SyncBrowsingHistory(const base::ListValue* args) {
DCHECK(sync_service_);
bool new_value;
if (!args->GetBoolean(0, &new_value))
// TODO(bridiver) - does this need to report an error?
Expand All @@ -174,6 +181,7 @@ void SyncUIDOMHandler::SyncBrowsingHistory(const base::ListValue* args) {
}

void SyncUIDOMHandler::SyncSavedSiteSettings(const base::ListValue* args) {
DCHECK(sync_service_);
bool new_value;
if (!args->GetBoolean(0, &new_value))
// TODO(bridiver) - does this need to report an error?
Expand All @@ -183,6 +191,7 @@ void SyncUIDOMHandler::SyncSavedSiteSettings(const base::ListValue* args) {
}

void SyncUIDOMHandler::DeleteDevice(const base::ListValue* args) {
DCHECK(sync_service_);
int i_device_id = -1;
if (!args->GetInteger(0, &i_device_id) || i_device_id == -1) {
DCHECK(false) << "[Brave Sync] could not get device id";
Expand All @@ -193,6 +202,7 @@ void SyncUIDOMHandler::DeleteDevice(const base::ListValue* args) {
}

void SyncUIDOMHandler::ResetSync(const base::ListValue* args) {
DCHECK(sync_service_);
sync_service_->OnResetSync();
}

Expand Down Expand Up @@ -240,7 +250,8 @@ SyncUI::SyncUI(content::WebUI* web_ui, const std::string& name)
: BasicUI(web_ui, name,
kBraveSyncGenerated,
kBraveSyncGeneratedSize,
IDR_BRAVE_SYNC_HTML) {
Profile::FromWebUI(web_ui)->IsOffTheRecord() ?
IDR_BRAVE_SYNC_DISABLED_HTML : IDR_BRAVE_SYNC_HTML) {
auto handler_owner = std::make_unique<SyncUIDOMHandler>();
SyncUIDOMHandler* handler = handler_owner.get();
web_ui->AddMessageHandler(std::move(handler_owner));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ void BraveRenderViewContextMenu::AppendBraveLinkItems() {
bool BraveRenderViewContextMenu::IsCommandIdEnabled(int id) const {
switch (id) {
case IDC_CONTENT_CONTEXT_OPENLINKTOR:
return params_.link_url.is_valid() && !browser_context_->IsTorProfile();
return params_.link_url.is_valid() &&
IsURLAllowedInIncognito(params_.link_url, browser_context_) &&
!browser_context_->IsTorProfile();
default:
return RenderViewContextMenu_Chromium::IsCommandIdEnabled(id);
}
Expand Down
5 changes: 1 addition & 4 deletions components/brave_sync/brave_sync_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,7 @@ void BraveSyncService::RemoveObserver(BraveSyncServiceObserver* observer) {
bool BraveSyncService::is_enabled() {
const base::CommandLine& command_line =
*base::CommandLine::ForCurrentProcess();
if (command_line.HasSwitch(switches::kDisableBraveSync))
return false;
else
return true;
return !command_line.HasSwitch(switches::kDisableBraveSync);
}

} // namespace brave_sync
1 change: 1 addition & 0 deletions components/brave_sync/resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
<include name="IDR_BRAVE_SYNC_EXTENSION_BUNDLE_JS" file="brave_sync/extension/brave-sync/bundles/bundle.js" type="BINDATA" />
<include name="IDR_BRAVE_SYNC_EXTENSION_CRYPTO_JS" file="brave_sync/extension/brave-crypto/browser/crypto.js" type="BINDATA" />
<include name="IDR_BRAVE_SYNC_HTML" file="brave_sync/ui/brave_sync.html" type="BINDATA" />
<include name="IDR_BRAVE_SYNC_DISABLED_HTML" file="brave_sync/ui/brave_sync_disabled.html" type="BINDATA" />
</includes>
</release>
</grit>
11 changes: 11 additions & 0 deletions components/brave_sync/ui/brave_sync_disabled.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width">
<title>Sync(Disabled)</title>
</head>
<body>
Sync in tor(guest) window is disabled.
</body>
</html>

0 comments on commit 55eb56a

Please sign in to comment.