Use prlog for focusmanager logging

RESOLVED FIXED in mozilla17

Status

()

Core
Event Handling
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Neil Deakin (not available until Aug 9), Assigned: Neil Deakin (not available until Aug 9))

Tracking

Trunk
mozilla17
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Created attachment 645749 [details] [diff] [review]
switch logging

The current logging requires enabling a macro. Better would be to use prlogging to enable logging without rebuilding.
Attachment #645749 - Flags: review?(bugs)

Comment 1

5 years ago
Comment on attachment 645749 [details] [diff] [review]
switch logging


>+#ifdef PR_LOGGING
>+
>+// Two types of focus pr logging are available:
>+//   'Focus' for normal focus manager calls
>+//   'FocusNavigation' for tab and document navigation
>+PRLogModuleInfo* gFocusLog;
>+PRLogModuleInfo* gFocusNavigationLog;
>+
>+#define LOGFOCUS(args) PR_LOG(gFocusLog, 4, args)
>+#define LOGFOCUSNAVIGATION(args) PR_LOG(gFocusNavigationLog, 4, args)
>+
>+#define LOGTAG(log, format, content)                            \
>+  {                                                             \
>+    nsAutoString tag(NS_LITERAL_STRING("(none)"));              \
>+    if (content)                                                \
>+      content->Tag()->ToString(tag);                            \
if (expr) {
  stmt;
}
And you could just use Tag()->ToUTF8String()


>+#ifdef PR_LOGGING
>+  LOGFOCUS(("<<MoveFocus begin Type: %d Flags: %x>>", aType, aFlags));
>+
>+  if (PR_LOG_TEST(gFocusLog, PR_LOG_DEBUG)) {
>+    nsCOMPtr<nsPIDOMWindow> focusedWindow = mFocusedWindow;
>+    if (focusedWindow) {
>+      nsCOMPtr<nsIDocument> doc = do_QueryInterface(focusedWindow->GetExtantDocument());
No need for this QI anymore.
There is GetExtantDoc(). And doc could be nsIDocument*


>+#ifdef PR_LOGGING
>+  if (PR_LOG_TEST(gFocusLog, PR_LOG_DEBUG)) {
>+    LOGFOCUS(("Window %p Raised [Currently: %p %p]", aWindow, mActiveWindow.get(), mFocusedWindow.get()));
>+    nsCAutoString spec;
>+    nsCOMPtr<nsIDocument> doc = do_QueryInterface(window->GetExtantDocument());
Ditto

>+#ifdef PR_LOGGING
>+  if (PR_LOG_TEST(gFocusLog, PR_LOG_DEBUG)) {
>+    LOGFOCUS(("Window %p Lowered [Currently: %p %p]", aWindow, mActiveWindow.get(), mFocusedWindow.get()));
>+    nsCAutoString spec;
>+    nsCOMPtr<nsIDocument> doc = do_QueryInterface(window->GetExtantDocument());
And here. And elsewhere...

>     if (doc) {
>       doc->GetDocumentURI()->GetSpec(spec);
>-      printf(" [%p] Active Window: %s", mActiveWindow.get(), spec.get());
>+      LOGFOCUS(("  Lowered Window: %s", spec.get()));
>+    }
>+    if (mActiveWindow) {
>+      doc = do_QueryInterface(mActiveWindow->GetExtantDocument());
>+      if (doc) {
>+        doc->GetDocumentURI()->GetSpec(spec);
>+        LOGFOCUS(("  Active Window: %s", spec.get()));
>+      }
Hmm, GetDocumentURI can return null as the method name indicates.
Attachment #645749 - Flags: review?(bugs) → review-
Created attachment 645764 [details] [diff] [review]
switch logging, patch 2
Attachment #645749 - Attachment is obsolete: true
Attachment #645764 - Flags: review?(bugs)

Comment 3

5 years ago
Comment on attachment 645764 [details] [diff] [review]
switch logging, patch 2


>+#ifdef PR_LOGGING
>+  LOGFOCUS(("<<MoveFocus begin Type: %d Flags: %x>>", aType, aFlags));
>+
>+  if (PR_LOG_TEST(gFocusLog, PR_LOG_DEBUG)) {
>+    nsCOMPtr<nsPIDOMWindow> focusedWindow = mFocusedWindow;
Hmm, there shouldn't be need for this to be nsCOMPtr



>@@ -690,17 +709,17 @@ nsFocusManager::WindowRaised(nsIDOMWindo
>     baseWindow->SetVisibility(true);
>   }
> 
>   // inform the DOM window that it has activated, so that the active attribute
>   // is updated on the window
>   window->ActivateOrDeactivate(true);
> 
>   // send activate event
>-  nsCOMPtr<nsIDocument> document = do_QueryInterface(window->GetExtantDocument());
>+  nsIDocument* document = window->GetExtantDoc();
>   nsContentUtils::DispatchTrustedEvent(document,
Perhaps just
nsContentUtils::DispatchTrustedEvent(window->GetExtantDoc(),


>   // send deactivate event
>-  nsCOMPtr<nsIDocument> document = do_QueryInterface(window->GetExtantDocument());
>+  nsIDocument* document = window->GetExtantDoc();
>   nsContentUtils::DispatchTrustedEvent(document,
Ditto
Attachment #645764 - Flags: review?(bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/de293780ba8a
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.