1257287 - add a few accessors to nsIDocShell

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Trevor Saunders (:tbsaunde)]

it lets us get rid of some do_GetInterface() calls

Comment 1

[Trevor Saunders (:tbsaunde)]

Attachment: add nsIDocShell::GetEdItingSession()

Comment 2

[Trevor Saunders (:tbsaunde)]

Attachment: add nsIDocShell.tabChild

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8731382 [details] [diff] [review]
add nsIDocShell.tabChild

>+++ b/dom/base/nsCCUncollectableMarker.cpp
>@@ -298,17 +298,17 @@ MarkWindowList(nsISimpleEnumerator* aWindowList, bool aCleanupJS,
>   nsCOMPtr<nsISupports> iter;
>   while (NS_SUCCEEDED(aWindowList->GetNext(getter_AddRefs(iter))) &&
>          iter) {
>     if (nsCOMPtr<nsPIDOMWindowOuter> window = do_QueryInterface(iter)) {
>       nsCOMPtr<nsIDocShell> rootDocShell = window->GetDocShell();
> 
>       MarkDocShell(rootDocShell, aCleanupJS, aPrepareForCC);
> 
>-      nsCOMPtr<nsITabChild> tabChild = do_GetInterface(rootDocShell);
>+      nsCOMPtr<nsITabChild> tabChild = rootDocShell->GetTabChild();
Technically you need a null check here now. rootDocShell could be null.

>@@ -864,17 +864,17 @@ nsFocusManager::WindowShown(mozIDOMWindowProxy* aWindow, bool aNeedsFocus)
>       doc = mFocusedWindow->GetExtantDoc();
>       if (doc && doc->GetDocumentURI()) {
>         doc->GetDocumentURI()->GetSpec(spec);
>         LOGFOCUS((" Focused Window: %s", spec.get()));
>       }
>     }
>   }
> 
>-  if (nsCOMPtr<nsITabChild> child = do_GetInterface(window->GetDocShell())) {
>+  if (nsCOMPtr<nsITabChild> child = window->GetDocShell()->GetTabChild()) {
Same here. window->GetDocShell() might return null.


> nsGlobalWindow::UpdateCommands(const nsAString& anAction, nsISelection* aSel, int16_t aReason)
> {
>   // If this is a child process, redirect to the parent process.
>-  if (nsCOMPtr<nsITabChild> child = do_GetInterface(GetDocShell())) {
>+  if (nsCOMPtr<nsITabChild> child = GetDocShell()->GetTabChild()) {
This needs a null check

>@@ -10669,17 +10669,17 @@ nsGlobalWindow::ShowSlowScriptDialog()
>     Telemetry::Accumulate(Telemetry::SLOW_SCRIPT_PAGE_COUNT, 1);
>   }
>   mHasHadSlowScript = true;
> 
>   if (XRE_IsContentProcess() &&
>       ProcessHangMonitor::Get()) {
>     ProcessHangMonitor::SlowScriptAction action;
>     RefPtr<ProcessHangMonitor> monitor = ProcessHangMonitor::Get();
>-    nsCOMPtr<nsITabChild> child = do_GetInterface(GetDocShell());
>+    nsCOMPtr<nsITabChild> child = GetDocShell()->GetTabChild();
This as well


>@@ -976,17 +976,17 @@ EventStateManager::ExecuteAccessKey(nsTArray<uint32_t>& aAccessCharCodes,
>             fm->SetFocus(element, nsIFocusManager::FLAG_BYKEY);
>             focusChanged = true;
>           }
>         }
> 
>         if (focusChanged && aIsTrustedEvent) {
>           // If this is a child process, inform the parent that we want the focus, but
>           // pass false since we don't want to change the window order.
>-          nsCOMPtr<nsITabChild> child = do_GetInterface(mPresContext->GetDocShell());
>+          nsCOMPtr<nsITabChild> child = mPresContext->GetDocShell()->GetTabChild();
this too


>+++ b/dom/ipc/TabChild.h
>@@ -510,17 +510,17 @@ public:
> 
>   bool GetUpdateHitRegion() const { return mUpdateHitRegion; }
> 
>   void UpdateHitRegion(const nsRegion& aRegion);
> 
>   static inline TabChild*
>   GetFrom(nsIDocShell* aDocShell)
>   {
>-    nsCOMPtr<nsITabChild> tc = do_GetInterface(aDocShell);
>+    nsCOMPtr<nsITabChild> tc = aDocShell->GetTabChild();
>     return static_cast<TabChild*>(tc.get());
>   }
You're changing this null safe method to be non-null-safe. Please don't.

Comment 4

[Trevor Saunders (:tbsaunde)]

Attachment: add nsIDocShell.tabChild

Comment 5

[Trevor Saunders (:tbsaunde)]

> >@@ -976,17 +976,17 @@ EventStateManager::ExecuteAccessKey(nsTArray<uint32_t>& aAccessCharCodes,
> >             fm->SetFocus(element, nsIFocusManager::FLAG_BYKEY);
> >             focusChanged = true;
> >           }
> >         }
> > 
> >         if (focusChanged && aIsTrustedEvent) {
> >           // If this is a child process, inform the parent that we want the focus, but
> >           // pass false since we don't want to change the window order.
> >-          nsCOMPtr<nsITabChild> child = do_GetInterface(mPresContext->GetDocShell());
> >+          nsCOMPtr<nsITabChild> child = mPresContext->GetDocShell()->GetTabChild();
> this too

I'm not particularly convinced it being null is something that can happen, but I guess I just assume avoid fixing random regressions in this stuff.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8732294 [details] [diff] [review]
add nsIDocShell.tabChild

>+++ b/dom/base/nsFocusManager.cpp
>@@ -864,17 +864,17 @@ nsFocusManager::WindowShown(mozIDOMWindowProxy* aWindow, bool aNeedsFocus)
>       doc = mFocusedWindow->GetExtantDoc();
>       if (doc && doc->GetDocumentURI()) {
>         doc->GetDocumentURI()->GetSpec(spec);
>         LOGFOCUS((" Focused Window: %s", spec.get()));
>       }
>     }
>   }
> 
>-  if (nsCOMPtr<nsITabChild> child = do_GetInterface(window->GetDocShell())) {
>+  if (nsCOMPtr<nsITabChild> child = window->GetDocShell()->GetTabChild()) {
Null check GetDocShell() here.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d51de0a1c9ba
https://hg.mozilla.org/integration/mozilla-inbound/rev/840c86ab171c

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d51de0a1c9ba
https://hg.mozilla.org/mozilla-central/rev/840c86ab171c