Bug 1339543 part 3 PuppetWidget should stop getting all edit commands before dispatching keyboard events which are synthesized for tests r?smaug draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 17 May 2017 15:34:11 +0900
changeset 581129 e8fea9d1aac6bf840a19f772aacd93e9c5c7168f
parent 581128 b0a7663a02601f42020543d671e1b7f0000ade8d
child 581130 ae6b5c1e3c6c712c5532e3ba5095e0d4ae824cc6
push id59777
push usermasayuki@d-toybox.com
push dateFri, 19 May 2017 09:49:08 +0000
reviewerssmaug
bugs1339543
milestone55.0a1
Bug 1339543 part 3 PuppetWidget should stop getting all edit commands before dispatching keyboard events which are synthesized for tests r?smaug Synthesized keyboard events in child process doesn't have edit commands when it's being dispatched. With the old design, PuppetWidget needed to store all edit commands for dispatching keyboard event but it's not necessary now because WidgetKeyboardEvent will get only necessary edit commands when WidgetKeyboardEvent::ExecuteEditCommands() is called. So, PuppetWidget should stop calling TabChild::RequestNativeKeyBindings() before dispatching keyboard events. This patch changes browser_audioTabIcon.js which becomes permanent orange with this change. MozReview-Commit-ID: 1eK1mUahRMO
browser/base/content/test/general/browser_audioTabIcon.js
widget/PuppetWidget.cpp
--- a/browser/base/content/test/general/browser_audioTabIcon.js
+++ b/browser/base/content/test/general/browser_audioTabIcon.js
@@ -2,17 +2,17 @@ const PAGE = "https://example.com/browse
 const TABATTR_REMOVAL_PREFNAME = "browser.tabs.delayHidingAudioPlayingIconMS";
 const INITIAL_TABATTR_REMOVAL_DELAY_MS = Services.prefs.getIntPref(TABATTR_REMOVAL_PREFNAME);
 
 async function wait_for_tab_playing_event(tab, expectPlaying) {
   if (tab.soundPlaying == expectPlaying) {
     ok(true, "The tab should " + (expectPlaying ? "" : "not ") + "be playing");
     return true;
   }
-  return await BrowserTestUtils.waitForEvent(tab, "TabAttrModified", false, (event) => {
+  return BrowserTestUtils.waitForEvent(tab, "TabAttrModified", false, (event) => {
     if (event.detail.changed.includes("soundplaying")) {
       is(tab.hasAttribute("soundplaying"), expectPlaying, "The tab should " + (expectPlaying ? "" : "not ") + "be playing");
       is(tab.soundPlaying, expectPlaying, "The tab should " + (expectPlaying ? "" : "not ") + "be playing");
       return true;
     }
     return false;
   });
 }
@@ -48,18 +48,16 @@ async function pause(tab, options) {
     // Use 10s to remove possibility of race condition with attr removal.
     Services.prefs.setIntPref(TABATTR_REMOVAL_PREFNAME, 10000);
   }
 
   try {
     let browser = tab.linkedBrowser;
     let awaitDOMAudioPlaybackStopped =
       BrowserTestUtils.waitForEvent(browser, "DOMAudioPlaybackStopped", "DOMAudioPlaybackStopped event should get fired after pause");
-    let awaitTabPausedAttrModified =
-      wait_for_tab_playing_event(tab, false);
     await ContentTask.spawn(browser, {}, async function() {
       let audio = content.document.querySelector("audio");
       audio.pause();
     });
 
     // If the tab has already be muted, it means the tab won't have soundplaying,
     // so we don't need to check this attribute.
     if (browser.audioMuted) {
@@ -68,17 +66,17 @@ async function pause(tab, options) {
 
     if (extendedDelay) {
       ok(tab.hasAttribute("soundplaying"), "The tab should still have the soundplaying attribute immediately after pausing");
 
       await awaitDOMAudioPlaybackStopped;
       ok(tab.hasAttribute("soundplaying"), "The tab should still have the soundplaying attribute immediately after DOMAudioPlaybackStopped");
     }
 
-    await awaitTabPausedAttrModified;
+    await wait_for_tab_playing_event(tab, false);
     ok(!tab.hasAttribute("soundplaying"), "The tab should not have the soundplaying attribute after the timeout has resolved");
   } finally {
     // Make sure other tests don't timeout if an exception gets thrown above.
     // Need to use setIntPref instead of clearUserPref because prefs_general.js
     // overrides the default value to help this and other tests run faster.
     Services.prefs.setIntPref(TABATTR_REMOVAL_PREFNAME, INITIAL_TABATTR_REMOVAL_DELAY_MS);
   }
 }
--- a/widget/PuppetWidget.cpp
+++ b/widget/PuppetWidget.cpp
@@ -343,24 +343,16 @@ PuppetWidget::DispatchEvent(WidgetGUIEve
              "Unexpected event dispatch!");
 
   MOZ_ASSERT(!aEvent->AsKeyboardEvent() ||
              aEvent->mFlags.mIsSynthesizedForTests ||
              aEvent->AsKeyboardEvent()->AreAllEditCommandsInitialized(),
     "Non-sysnthesized keyboard events should have edit commands for all types "
     "before dispatched");
 
-  AutoCacheNativeKeyCommands autoCache(this);
-  if (aEvent->mFlags.mIsSynthesizedForTests && !mNativeKeyCommandsValid) {
-    WidgetKeyboardEvent* keyEvent = aEvent->AsKeyboardEvent();
-    if (keyEvent) {
-      mTabChild->RequestNativeKeyBindings(&autoCache, keyEvent);
-    }
-  }
-
   if (aEvent->mClass == eCompositionEventClass) {
     // Store the latest native IME context of parent process's widget or
     // TextEventDispatcher if it's in this process.
     WidgetCompositionEvent* compositionEvent = aEvent->AsCompositionEvent();
 #ifdef DEBUG
     if (mNativeIMEContext.IsValid() &&
         mNativeIMEContext != compositionEvent->mNativeIMEContext) {
       RefPtr<TextComposition> composition =