Closed
Bug 1257287
Opened 8 years ago
Closed 8 years ago
add a few accessors to nsIDocShell
Categories
(Core :: DOM: Navigation, defect)
Core
DOM: Navigation
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: tbsaunde, Unassigned)
Details
Attachments
(3 files)
6.70 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
14.97 KB,
patch
|
smaug
:
review-
|
Details | Diff | Splinter Review |
15.50 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
it lets us get rid of some do_GetInterface() calls
Reporter | ||
Comment 1•8 years ago
|
||
Attachment #8731381 -
Flags: review?(bugs)
Reporter | ||
Comment 2•8 years ago
|
||
Attachment #8731382 -
Flags: review?(bugs)
Updated•8 years ago
|
Attachment #8731381 -
Flags: review?(bugs) → review+
Comment 3•8 years ago
|
||
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-
Reporter | ||
Comment 4•8 years ago
|
||
Attachment #8732294 -
Flags: review?(bugs)
Reporter | ||
Comment 5•8 years ago
|
||
> >@@ -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•8 years ago
|
||
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/integration/mozilla-inbound/rev/d51de0a1c9ba https://hg.mozilla.org/integration/mozilla-inbound/rev/840c86ab171c
Comment 8•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d51de0a1c9ba https://hg.mozilla.org/mozilla-central/rev/840c86ab171c
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•