Closed Bug 1257287 Opened 4 years ago Closed 4 years ago

add a few accessors to nsIDocShell

Categories

(Core :: DOM: Navigation, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: tbsaunde, Unassigned)

Details

Attachments

(3 files)

it lets us get rid of some do_GetInterface() calls
Attachment #8731382 - Flags: review?(bugs)
Attachment #8731381 - Flags: review?(bugs) → review+
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.
Attachment #8731382 - Flags: review?(bugs) → review-
Attachment #8732294 - Flags: review?(bugs)
> >@@ -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 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.
Attachment #8732294 - Flags: review?(bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/d51de0a1c9ba
https://hg.mozilla.org/mozilla-central/rev/840c86ab171c
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.