777339 - Use prlog for focusmanager logging

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Neil Deakin]

Attachment: switch logging

The current logging requires enabling a macro. Better would be to use prlogging to enable logging without rebuilding.

Comment 1

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

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.

Comment 2

[Neil Deakin]

Attachment: switch logging, patch 2

Comment 3

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

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

Comment 4

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/de293780ba8a