Bug 1215818 - part 3: Add telemetry probe to collect IM share on Linux r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 20 Jun 2018 14:55:46 +0900
changeset 809094 46861f427e5df0ad322f19e9955f9abf051cdacd
parent 809093 d7927cc4eebbdb3a03fc7ad95c572711c8581693
child 809580 1a13f09b7b6065988771011b3b7ff362216c004a
push id113543
push usermasayuki@d-toybox.com
push dateThu, 21 Jun 2018 07:08:37 +0000
reviewersm_kato
bugs1215818
milestone62.0a1
Bug 1215818 - part 3: Add telemetry probe to collect IM share on Linux r?m_kato Different from Windows and macOS, we cannot check if active keyboard layout works as "IME" actually. Therefore, this patch add the telemetry probe to the dispatcher of eCompositionStart. However, composition string is also used by some Wester keyboard layouts which have dead keys. So, the meaning of the result is deferent from the other platforms, but it must be useful information which IM (e.g., fcitx, ibus) is used by most users. MozReview-Commit-ID: A7vYuGtcrRw
toolkit/components/telemetry/Scalars.yaml
widget/gtk/IMContextWrapper.cpp
widget/gtk/IMContextWrapper.h
--- a/toolkit/components/telemetry/Scalars.yaml
+++ b/toolkit/components/telemetry/Scalars.yaml
@@ -1763,16 +1763,33 @@ widget:
     expires: never
     kind: boolean
     notification_emails:
       - mnakano@mozilla.com
     release_channel_collection: opt-out
     record_in_processes:
       - 'main'
 
+  ime_name_on_linux:
+    bug_numbers:
+      - 1215818
+    description: >
+      Name of active IM (e.g., xim, fcitx, ibus, etc) which was actually set by
+      env on Linux.  Different from Windows and macOS, this value includes
+      non-IME users even though this is recoded when first compositionstart
+      event because dead key is also implemented by IME on Linux.
+    keyed: true
+    expires: never
+    kind: boolean
+    notification_emails:
+      - mnakano@mozilla.com
+    release_channel_collection: opt-out
+    record_in_processes:
+      - 'main'
+
 # The following section contains memory reporter counters.
 memoryreporter:
   max_ghost_windows:
     bug_numbers:
       - 1454724
     description: >
       The maximum number of leaked ghost windows seen.
     expires: "66"
--- a/widget/gtk/IMContextWrapper.cpp
+++ b/widget/gtk/IMContextWrapper.cpp
@@ -9,16 +9,17 @@
 
 #include "IMContextWrapper.h"
 #include "nsGtkKeyUtils.h"
 #include "nsWindow.h"
 #include "mozilla/AutoRestore.h"
 #include "mozilla/Likely.h"
 #include "mozilla/MiscEvents.h"
 #include "mozilla/Preferences.h"
+#include "mozilla/Telemetry.h"
 #include "mozilla/TextEventDispatcher.h"
 #include "mozilla/TextEvents.h"
 #include "WritingModes.h"
 
 namespace mozilla {
 namespace widget {
 
 LazyLogModule gGtkIMLog("nsGtkIMModuleWidgets");
@@ -309,16 +310,56 @@ static bool
 IsFcitxInSyncMode()
 {
     // See fcitx_im_context_class_init() in src/frontend/gtk2/fcitximcontext.c
     // https://github.com/fcitx/fcitx/blob/78b98d9230dc9630e99d52e3172bdf440ffd08c4/src/frontend/gtk2/fcitximcontext.c#L395-L398
     return GetFcitxBoolEnv("IBUS_ENABLE_SYNC_MODE") ||
            GetFcitxBoolEnv("FCITX_ENABLE_SYNC_MODE");
 }
 
+nsDependentCSubstring
+IMContextWrapper::GetIMName() const
+{
+    const char* contextIDChar =
+        gtk_im_multicontext_get_context_id(GTK_IM_MULTICONTEXT(mContext));
+    if (!contextIDChar) {
+        return nsDependentCSubstring();
+    }
+
+    nsDependentCSubstring im(contextIDChar, strlen(contextIDChar));
+
+    // If the context is XIM, actual engine must be specified with
+    // |XMODIFIERS=@im=foo|.
+    const char* xmodifiersChar = PR_GetEnv("XMODIFIERS");
+    if (!im.EqualsLiteral("xim") || !xmodifiersChar) {
+        return im;
+    }
+
+    nsDependentCString xmodifiers(xmodifiersChar);
+    int32_t atIMValueStart = xmodifiers.Find("@im=") + 4;
+    if (atIMValueStart < 4 ||
+        xmodifiers.Length() <= static_cast<size_t>(atIMValueStart)) {
+        return im;
+    }
+
+    int32_t atIMValueEnd =
+        xmodifiers.Find("@", false, atIMValueStart);
+    if (atIMValueEnd > atIMValueStart) {
+         return nsDependentCSubstring(xmodifiersChar + atIMValueStart,
+                                      atIMValueEnd - atIMValueStart);
+    }
+
+    if (atIMValueEnd == kNotFound) {
+        return nsDependentCSubstring(xmodifiersChar + atIMValueStart,
+                                     strlen(xmodifiersChar) - atIMValueStart);
+    }
+
+    return im;
+}
+
 void
 IMContextWrapper::Init()
 {
     MozContainer* container = mOwnerWindow->GetMozContainer();
     MOZ_ASSERT(container, "container is null");
     GdkWindow* gdkWindow = gtk_widget_get_window(GTK_WIDGET(container));
 
     // NOTE: gtk_im_*_new() abort (kill) the whole process when it fails.
@@ -334,41 +375,17 @@ IMContextWrapper::Init()
     g_signal_connect(mContext, "delete_surrounding",
         G_CALLBACK(IMContextWrapper::OnDeleteSurroundingCallback), this);
     g_signal_connect(mContext, "commit",
         G_CALLBACK(IMContextWrapper::OnCommitCompositionCallback), this);
     g_signal_connect(mContext, "preedit_start",
         G_CALLBACK(IMContextWrapper::OnStartCompositionCallback), this);
     g_signal_connect(mContext, "preedit_end",
         G_CALLBACK(IMContextWrapper::OnEndCompositionCallback), this);
-    nsDependentCSubstring im;
-    const char* contextIDChar =
-        gtk_im_multicontext_get_context_id(GTK_IM_MULTICONTEXT(mContext));
-    const char* xmodifiersChar = PR_GetEnv("XMODIFIERS");
-    if (contextIDChar) {
-        im.Rebind(contextIDChar, strlen(contextIDChar));
-        // If the context is XIM, actual engine must be specified with
-        // |XMODIFIERS=@im=foo|.
-        if (im.EqualsLiteral("xim") && xmodifiersChar) {
-            nsDependentCString xmodifiers(xmodifiersChar);
-            int32_t atIMValueStart = xmodifiers.Find("@im=") + 4;
-            if (atIMValueStart >= 4 &&
-                xmodifiers.Length() > static_cast<size_t>(atIMValueStart)) {
-                int32_t atIMValueEnd =
-                    xmodifiers.Find("@", false, atIMValueStart);
-                if (atIMValueEnd > atIMValueStart) {
-                    im.Rebind(xmodifiersChar + atIMValueStart,
-                              atIMValueEnd - atIMValueStart);
-                } else if (atIMValueEnd == kNotFound) {
-                    im.Rebind(xmodifiersChar + atIMValueStart,
-                              strlen(xmodifiersChar) - atIMValueStart);
-                }
-            }
-        }
-    }
+    nsDependentCSubstring im = GetIMName();
     if (im.EqualsLiteral("ibus")) {
         mIMContextID = IMContextID::eIBus;
         mIsIMInAsyncKeyHandlingMode = !IsIBusInSyncMode();
         // Although ibus has key snooper mode, it's forcibly disabled on Firefox
         // in default settings by its whitelist since we always send key events
         // to IME before handling shortcut keys.  The whitelist can be
         // customized with env, IBUS_NO_SNOOPER_APPS, but we don't need to
         // support such rare cases for reducing maintenance cost.
@@ -431,21 +448,24 @@ IMContextWrapper::Init()
 
     // Dummy context
     mDummyContext = gtk_im_multicontext_new();
     gtk_im_context_set_client_window(mDummyContext, gdkWindow);
 
     MOZ_LOG(gGtkIMLog, LogLevel::Info,
         ("0x%p Init(), mOwnerWindow=%p, mContext=%p (im=\"%s\"), "
          "mIsIMInAsyncKeyHandlingMode=%s, mIsKeySnooped=%s, "
-         "mSimpleContext=%p, mDummyContext=%p, contextIDChar=\"%s\", "
-         "xmodifiersChar=\"%s\"",
+         "mSimpleContext=%p, mDummyContext=%p, "
+         "gtk_im_multicontext_get_context_id()=\"%s\", "
+         "PR_GetEnv(\"XMODIFIERS\")=\"%s\"",
          this, mOwnerWindow, mContext, nsAutoCString(im).get(),
          ToChar(mIsIMInAsyncKeyHandlingMode), ToChar(mIsKeySnooped),
-         mSimpleContext, mDummyContext, contextIDChar, xmodifiersChar));
+         mSimpleContext, mDummyContext,
+         gtk_im_multicontext_get_context_id(GTK_IM_MULTICONTEXT(mContext)),
+         PR_GetEnv("XMODIFIERS")));
 }
 
 IMContextWrapper::~IMContextWrapper()
 {
     if (this == sLastFocusedContext) {
         sLastFocusedContext = nullptr;
     }
     MOZ_LOG(gGtkIMLog, LogLevel::Info,
@@ -1971,16 +1991,35 @@ IMContextWrapper::DispatchCompositionSta
     if (NS_WARN_IF(NS_FAILED(rv))) {
         MOZ_LOG(gGtkIMLog, LogLevel::Error,
             ("0x%p   DispatchCompositionStart(), FAILED, "
              "due to BeginNativeInputTransaction() failure",
              this));
         return false;
     }
 
+    static bool sHasSetTelemetry = false;
+    if (!sHasSetTelemetry) {
+        sHasSetTelemetry = true;
+        NS_ConvertUTF8toUTF16 im(GetIMName());
+        // 72 is kMaximumKeyStringLength in TelemetryScalar.cpp
+        if (im.Length() > 72) {
+            if (NS_IS_LOW_SURROGATE(im[72 - 1]) &&
+                NS_IS_HIGH_SURROGATE(im[72 - 2])) {
+                im.Truncate(72 - 2);
+            } else {
+                im.Truncate(72 - 1);
+            }
+            // U+2026 is "..."
+            im.Append(char16_t(0x2026));
+        }
+        Telemetry::ScalarSet(Telemetry::ScalarID::WIDGET_IME_NAME_ON_LINUX,
+                             im, true);
+    }
+
     MOZ_LOG(gGtkIMLog, LogLevel::Debug,
         ("0x%p   DispatchCompositionStart(), dispatching "
          "compositionstart... (mCompositionStart=%u)",
          this, mCompositionStart));
     mCompositionState = eCompositionState_CompositionStartDispatched;
     nsEventStatus status;
     dispatcher->StartComposition(status);
     if (lastFocusedWindow->IsDestroyed() ||
--- a/widget/gtk/IMContextWrapper.h
+++ b/widget/gtk/IMContextWrapper.h
@@ -123,16 +123,22 @@ public:
                 return "eScim";
             case IMContextID::eUim:
                 return "eUim";
             default:
                 return "eUnknown";
         }
     }
 
+    /**
+     * GetIMName() returns IM name associated with mContext.  If the context is
+     * xim, this look for actual engine from XMODIFIERS environment variable.
+     */
+    nsDependentCSubstring GetIMName() const;
+
 protected:
     ~IMContextWrapper();
 
     // Owner of an instance of this class. This should be top level window.
     // The owner window must release the contexts when it's destroyed because
     // the IME contexts need the native window.  If OnDestroyWindow() is called
     // with the owner window, it'll release IME contexts.  Otherwise, it'll
     // just clean up any existing composition if it's related to the destroying