Index: chrome/browser/extensions/api/web_navigation/web_navigation_api.cc |
diff --git a/chrome/browser/extensions/api/web_navigation/web_navigation_api.cc b/chrome/browser/extensions/api/web_navigation/web_navigation_api.cc |
index 6d45ec5856295bb365085ebcbf3cdc14a43bc81d..30e1836dd7934613b8a7d6ddd6bc2b6dbdb6166f 100644 |
--- a/chrome/browser/extensions/api/web_navigation/web_navigation_api.cc |
+++ b/chrome/browser/extensions/api/web_navigation/web_navigation_api.cc |
@@ -271,11 +271,28 @@ void WebNavigationTabObserver::DidStartNavigation( |
return; |
} |
- helpers::DispatchOnBeforeNavigate(navigation_handle); |
+ pending_on_before_navigate_event_ = |
+ helpers::CreateOnBeforeNavigateEvent(navigation_handle); |
+ |
+ // Only dispatch the onBeforeNavigate event if the associated WebContents |
+ // is already added to the tab strip. Otherwise the event should be delayed |
+ // and sent after the addition, to preserve the ordering of events. |
+ if (ExtensionTabUtil::GetTabById( |
Devlin
2016/12/02 20:17:11
I don't love that this is how we check this. It m
nasko
2016/12/02 21:40:45
I think there will be more fallout if we did that
Devlin
2016/12/02 21:58:31
// TODO(nasko|devlin): We have to do this check be
nasko
2016/12/02 23:41:39
Done.
|
+ ExtensionTabUtil::GetTabId(web_contents()), |
+ Profile::FromBrowserContext(web_contents()->GetBrowserContext()), |
Devlin
2016/12/02 20:17:11
nit: this can take a browser context, so no need f
nasko
2016/12/02 21:40:45
Done.
|
+ false, NULL, NULL, NULL, NULL)) { |
Devlin
2016/12/02 20:17:11
nit: nullptr in new code.
nasko
2016/12/02 21:40:45
Done.
|
+ DispatchOnBeforeNavigate(); |
+ } |
} |
void WebNavigationTabObserver::DidFinishNavigation( |
content::NavigationHandle* navigation_handle) { |
+ // If there has been a DidStartNavigation call before the tab was ready to |
+ // dispatch events, ensure that it is sent before processing the |
+ // DidFinishNavigation. |
+ // Note: This is exercised by WebNavigationApiTest.TargetBlankIncognito. |
+ DispatchOnBeforeNavigate(); |
Devlin
2016/12/02 20:17:11
Does this not have the same possible race? Or why
nasko
2016/12/02 21:40:45
This guarantees that if we are going to send a com
Devlin
2016/12/02 21:58:31
If the TabAdded notification is sent before DidSta
nasko
2016/12/02 23:41:39
The BrowserContext passed in should already be the
|
+ |
if (navigation_handle->HasCommitted() && !navigation_handle->IsErrorPage()) { |
HandleCommit(navigation_handle); |
return; |
@@ -391,6 +408,17 @@ void WebNavigationTabObserver::WebContentsDestroyed() { |
registrar_.RemoveAll(); |
} |
+void WebNavigationTabObserver::DispatchOnBeforeNavigate() { |
+ if (!pending_on_before_navigate_event_.get()) |
+ return; |
+ |
+ Profile* profile = |
+ Profile::FromBrowserContext(web_contents()->GetBrowserContext()); |
+ EventRouter* event_router = EventRouter::Get(profile); |
Devlin
2016/12/02 20:17:11
nit: use browser context directly.
nasko
2016/12/02 21:40:45
Done.
|
+ if (profile && event_router) |
Devlin
2016/12/02 20:17:11
can profile/context be null? (Or event router, fo
nasko
2016/12/02 21:40:45
I don't know whether the context can be null, poss
|
+ event_router->BroadcastEvent(std::move(pending_on_before_navigate_event_)); |
+} |
+ |
void WebNavigationTabObserver::HandleCommit( |
content::NavigationHandle* navigation_handle) { |
bool is_reference_fragment_navigation = IsReferenceFragmentNavigation( |