Skip to content

Commit e304812

Browse files
committed
Handle mojo disconnect events properly.
fix brave/brave-browser#16964
1 parent 178cdbd commit e304812

File tree

6 files changed

+30
-40
lines changed

6 files changed

+30
-40
lines changed

components/brave_ads/common/features.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -153,11 +153,11 @@ int AdNotificationInsetY() {
153153
kDefaultAdNotificationInsetY);
154154
}
155155

156+
#endif // !defined(OS_ANDROID)
157+
156158
bool IsRequestAdsEnabledApiEnabled() {
157159
return base::FeatureList::IsEnabled(kRequestAdsEnabledApi);
158160
}
159161

160-
#endif // !defined(OS_ANDROID)
161-
162162
} // namespace features
163163
} // namespace brave_ads

components/brave_ads/common/features.h

+3-4
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ namespace features {
1717

1818
extern const base::Feature kAdNotifications;
1919

20-
extern const base::Feature kRequestAdsEnabledApi;
21-
2220
bool IsAdNotificationsEnabled();
2321
int AdNotificationTimeout();
2422

@@ -33,10 +31,11 @@ double AdNotificationNormalizedDisplayCoordinateX();
3331
int AdNotificationInsetX();
3432
double AdNotificationNormalizedDisplayCoordinateY();
3533
int AdNotificationInsetY();
34+
#endif // !defined(OS_ANDROID)
3635

37-
bool IsRequestAdsEnabledApiEnabled();
36+
extern const base::Feature kRequestAdsEnabledApi;
3837

39-
#endif // !defined(OS_ANDROID)
38+
bool IsRequestAdsEnabledApiEnabled();
4039

4140
} // namespace features
4241
} // namespace brave_ads

components/brave_ads/renderer/brave_ads_js_handler.cc

+20-17
Original file line numberDiff line numberDiff line change
@@ -24,33 +24,19 @@ BraveAdsJSHandler::BraveAdsJSHandler(content::RenderFrame* render_frame)
2424

2525
BraveAdsJSHandler::~BraveAdsJSHandler() = default;
2626

27-
bool BraveAdsJSHandler::EnsureConnected() {
28-
if (!brave_ads_.is_bound()) {
29-
render_frame_->GetBrowserInterfaceBroker()->GetInterface(
30-
brave_ads_.BindNewPipeAndPassReceiver());
31-
}
32-
33-
return brave_ads_.is_bound();
34-
}
35-
3627
void BraveAdsJSHandler::AddJavaScriptObjectToFrame(
3728
v8::Local<v8::Context> context) {
3829
v8::Isolate* isolate = blink::MainThreadIsolate();
3930
v8::HandleScope handle_scope(isolate);
40-
if (context.IsEmpty())
31+
if (context.IsEmpty()) {
4132
return;
33+
}
4234

4335
v8::Context::Scope context_scope(context);
4436

4537
BindFunctionsToObject(isolate, context);
4638
}
4739

48-
void BraveAdsJSHandler::ResetRemote(content::RenderFrame* render_frame) {
49-
render_frame_ = render_frame;
50-
brave_ads_.reset();
51-
EnsureConnected();
52-
}
53-
5440
void BraveAdsJSHandler::BindFunctionsToObject(v8::Isolate* isolate,
5541
v8::Local<v8::Context> context) {
5642
v8::Local<v8::Object> global = context->Global();
@@ -87,10 +73,27 @@ void BraveAdsJSHandler::BindFunctionToObject(
8773
.Check();
8874
}
8975

76+
bool BraveAdsJSHandler::EnsureConnected() {
77+
if (!brave_ads_.is_bound()) {
78+
render_frame_->GetBrowserInterfaceBroker()->GetInterface(
79+
brave_ads_.BindNewPipeAndPassReceiver());
80+
brave_ads_.set_disconnect_handler(base::BindOnce(
81+
&BraveAdsJSHandler::OnRemoteDisconnect, base::Unretained(this)));
82+
}
83+
84+
return brave_ads_.is_bound();
85+
}
86+
87+
void BraveAdsJSHandler::OnRemoteDisconnect() {
88+
brave_ads_.reset();
89+
EnsureConnected();
90+
}
91+
9092
v8::Local<v8::Promise> BraveAdsJSHandler::RequestAdsEnabled(
9193
v8::Isolate* isolate) {
92-
if (!EnsureConnected())
94+
if (!EnsureConnected()) {
9395
return v8::Local<v8::Promise>();
96+
}
9497

9598
v8::MaybeLocal<v8::Promise::Resolver> resolver =
9699
v8::Promise::Resolver::New(isolate->GetCurrentContext());

components/brave_ads/renderer/brave_ads_js_handler.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ class BraveAdsJSHandler final {
2828
~BraveAdsJSHandler();
2929

3030
void AddJavaScriptObjectToFrame(v8::Local<v8::Context> context);
31-
void ResetRemote(content::RenderFrame* render_frame);
3231

3332
private:
3433
// Adds a function to the provided object.
@@ -40,6 +39,7 @@ class BraveAdsJSHandler final {
4039
void BindFunctionsToObject(v8::Isolate* isolate,
4140
v8::Local<v8::Context> context);
4241
bool EnsureConnected();
42+
void OnRemoteDisconnect();
4343

4444
// A function to be called from JS
4545
v8::Local<v8::Promise> RequestAdsEnabled(v8::Isolate* isolate);

components/brave_ads/renderer/brave_ads_render_frame_observer.cc

+3-8
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ bool IsAllowedHost(const GURL& url) {
3535
BraveAdsRenderFrameObserver::BraveAdsRenderFrameObserver(
3636
content::RenderFrame* render_frame,
3737
int32_t world_id)
38-
: RenderFrameObserver(render_frame), world_id_(world_id) {}
38+
: RenderFrameObserver(render_frame), world_id_(world_id) {
39+
native_javascript_handle_ = std::make_unique<BraveAdsJSHandler>(render_frame);
40+
}
3941

4042
BraveAdsRenderFrameObserver::~BraveAdsRenderFrameObserver() {}
4143

@@ -51,13 +53,6 @@ void BraveAdsRenderFrameObserver::DidCreateScriptContext(
5153
if (!IsAllowedHost(url))
5254
return;
5355

54-
if (!native_javascript_handle_) {
55-
native_javascript_handle_ =
56-
std::make_unique<BraveAdsJSHandler>(render_frame());
57-
} else {
58-
native_javascript_handle_->ResetRemote(render_frame());
59-
}
60-
6156
native_javascript_handle_->AddJavaScriptObjectToFrame(context);
6257
}
6358

components/brave_rewards/resources/extension/brave_rewards/request_ads_enabled_panel.tsx

+1-8
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import { Store } from 'webext-redux'
99

1010
import Theme from 'brave-ui/theme/brave-default'
1111
import { ThemeProvider } from 'styled-components'
12-
import { initLocale } from 'brave-ui/helpers'
1312
require('emptykit.css')
1413
require('../../../../../ui/webui/resources/fonts/muli.css')
1514
require('../../../../../ui/webui/resources/fonts/poppins.css')
@@ -21,25 +20,19 @@ import { WithThemeVariables } from '../../shared/components/with_theme_variables
2120
import AdsEnablePanel from './components/ads_enable'
2221

2322
// Utils
24-
import { getMessage, getUIMessages } from './background/api/locale_api'
23+
import { getMessage } from './background/api/locale_api'
2524

2625
const store: Store<RewardsExtension.State> = new Store({
2726
portName: 'REWARDSPANEL'
2827
})
2928

3029
const localeContext = {
3130
getString: (key: string) => {
32-
// In order to normalize messages across extensions and WebUI, replace all
33-
// chrome.i18n message placeholders with $N marker patterns. UI components
34-
// are responsible for replacing these markers with appropriate text or
35-
// using the markers to build HTML.
3631
const subsitutions = ['$1', '$2', '$3', '$4', '$5', '$6', '$7', '$8', '$9']
3732
return getMessage(key, subsitutions)
3833
}
3934
}
4035

41-
initLocale(getUIMessages())
42-
4336
store.ready().then(
4437
() => {
4538
render(

0 commit comments

Comments
 (0)