From 60c7bbf4a8a8029dc2c7cf50d3882cf0e1e963b9 Mon Sep 17 00:00:00 2001 From: Lars T Hansen Date: Wed, 28 Jan 2015 09:34:33 +0100 Subject: [PATCH 01/53] Bug 1126406 - guard a test. r=me --- js/src/jit-test/tests/asm.js/bug1122338.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/js/src/jit-test/tests/asm.js/bug1122338.js b/js/src/jit-test/tests/asm.js/bug1122338.js index b63352b86ac..1e49fc0e19f 100644 --- a/js/src/jit-test/tests/asm.js/bug1122338.js +++ b/js/src/jit-test/tests/asm.js/bug1122338.js @@ -4,6 +4,9 @@ // as plain JS) as it should be. That gave rise to a difference in // output. +if (!this.SharedArrayBuffer) + quit(0); + // Original test g = (function(stdlib, n, heap) { From c1c55a435c3b0110ae7f8c95c9404f0fc58bc045 Mon Sep 17 00:00:00 2001 From: Jonathan Kew Date: Wed, 28 Jan 2015 09:03:28 +0000 Subject: [PATCH 02/53] Bug 1108177 - Implement harfbuzz glyph-extents callback, so that fallback mark positioning works in legacy truetype fonts. r=jdaggett --- gfx/thebes/gfxHarfBuzzShaper.cpp | 176 ++++++++++++++++++++----------- gfx/thebes/gfxHarfBuzzShaper.h | 19 +++- 2 files changed, 134 insertions(+), 61 deletions(-) diff --git a/gfx/thebes/gfxHarfBuzzShaper.cpp b/gfx/thebes/gfxHarfBuzzShaper.cpp index 3868cdb9731..0bf4ca69310 100644 --- a/gfx/thebes/gfxHarfBuzzShaper.cpp +++ b/gfx/thebes/gfxHarfBuzzShaper.cpp @@ -53,6 +53,7 @@ gfxHarfBuzzShaper::gfxHarfBuzzShaper(gfxFont *aFont) mUseFontGlyphWidths(false), mInitialized(false), mVerticalInitialized(false), + mLoadedLocaGlyf(false), mLocaLongOffsets(false) { } @@ -346,54 +347,13 @@ gfxHarfBuzzShaper::GetGlyphVOrigin(hb_codepoint_t aGlyph, } if (mVmtxTable) { - if (mLocaTable && mGlyfTable) { - // TrueType outlines: use glyph bbox + top sidebearing - uint32_t offset; // offset of glyph record in the 'glyf' table - uint32_t len; - const char* data = hb_blob_get_data(mLocaTable, &len); - if (mLocaLongOffsets) { - if ((aGlyph + 1) * sizeof(AutoSwap_PRUint32) > len) { - *aY = 0; - return; - } - const AutoSwap_PRUint32* offsets = - reinterpret_cast(data); - offset = offsets[aGlyph]; - if (offset == offsets[aGlyph + 1]) { - // empty glyph - *aY = 0; - return; - } - } else { - if ((aGlyph + 1) * sizeof(AutoSwap_PRUint16) > len) { - *aY = 0; - return; - } - const AutoSwap_PRUint16* offsets = - reinterpret_cast(data); - offset = uint16_t(offsets[aGlyph]); - if (offset == uint16_t(offsets[aGlyph + 1])) { - // empty glyph - *aY = 0; - return; - } - offset *= 2; - } - - struct Glyf { // we only need the bounding-box at the beginning - // of the glyph record, not the actual outline data - AutoSwap_PRInt16 numberOfContours; - AutoSwap_PRInt16 xMin; - AutoSwap_PRInt16 yMin; - AutoSwap_PRInt16 xMax; - AutoSwap_PRInt16 yMax; - }; - data = hb_blob_get_data(mGlyfTable, &len); - if (offset + sizeof(Glyf) > len) { + bool emptyGlyf; + const Glyf *glyf = FindGlyf(aGlyph, &emptyGlyf); + if (glyf) { + if (emptyGlyf) { *aY = 0; return; } - const Glyf* glyf = reinterpret_cast(data + offset); if (aGlyph >= uint32_t(mNumLongVMetrics)) { aGlyph = mNumLongVMetrics - 1; @@ -406,7 +366,8 @@ gfxHarfBuzzShaper::GetGlyphVOrigin(hb_codepoint_t aGlyph, int16_t(glyf->yMax))); return; } else { - // XXX TODO: CFF outlines - need to get glyph extents. + // XXX TODO: not a truetype font; need to get glyph extents + // via some other API? // For now, fall through to default code below. } } @@ -431,6 +392,112 @@ gfxHarfBuzzShaper::GetGlyphVOrigin(hb_codepoint_t aGlyph, *aY = -FloatToFixed(GetFont()->GetAdjustedSize() / 2); } +static hb_bool_t +HBGetGlyphExtents(hb_font_t *font, void *font_data, + hb_codepoint_t glyph, + hb_glyph_extents_t *extents, + void *user_data) +{ + const gfxHarfBuzzShaper::FontCallbackData *fcd = + static_cast(font_data); + return fcd->mShaper->GetGlyphExtents(glyph, extents); +} + +// Find the data for glyph ID |aGlyph| in the 'glyf' table, if present. +// Returns null if not found, otherwise pointer to the beginning of the +// glyph's data. Sets aEmptyGlyf true if there is no actual data; +// otherwise, it's guaranteed that we can read at least the bounding box. +const gfxHarfBuzzShaper::Glyf* +gfxHarfBuzzShaper::FindGlyf(hb_codepoint_t aGlyph, bool *aEmptyGlyf) const +{ + if (!mLoadedLocaGlyf) { + mLoadedLocaGlyf = true; // only try this once; if it fails, this + // isn't a truetype font + gfxFontEntry *entry = mFont->GetFontEntry(); + uint32_t len; + gfxFontEntry::AutoTable headTable(entry, + TRUETYPE_TAG('h','e','a','d')); + const HeadTable* head = + reinterpret_cast(hb_blob_get_data(headTable, + &len)); + if (len < sizeof(HeadTable)) { + return nullptr; + } + mLocaLongOffsets = int16_t(head->indexToLocFormat) > 0; + mLocaTable = entry->GetFontTable(TRUETYPE_TAG('l','o','c','a')); + mGlyfTable = entry->GetFontTable(TRUETYPE_TAG('g','l','y','f')); + } + + if (!mLocaTable || !mGlyfTable) { + // it's not a truetype font + return nullptr; + } + + uint32_t offset; // offset of glyph record in the 'glyf' table + uint32_t len; + const char* data = hb_blob_get_data(mLocaTable, &len); + if (mLocaLongOffsets) { + if ((aGlyph + 1) * sizeof(AutoSwap_PRUint32) > len) { + return nullptr; + } + const AutoSwap_PRUint32* offsets = + reinterpret_cast(data); + offset = offsets[aGlyph]; + *aEmptyGlyf = (offset == uint16_t(offsets[aGlyph + 1])); + } else { + if ((aGlyph + 1) * sizeof(AutoSwap_PRUint16) > len) { + return nullptr; + } + const AutoSwap_PRUint16* offsets = + reinterpret_cast(data); + offset = uint16_t(offsets[aGlyph]); + *aEmptyGlyf = (offset == uint16_t(offsets[aGlyph + 1])); + offset *= 2; + } + + data = hb_blob_get_data(mGlyfTable, &len); + if (offset + sizeof(Glyf) > len) { + return nullptr; + } + + return reinterpret_cast(data + offset); +} + +hb_bool_t +gfxHarfBuzzShaper::GetGlyphExtents(hb_codepoint_t aGlyph, + hb_glyph_extents_t *aExtents) const +{ + bool emptyGlyf; + const Glyf *glyf = FindGlyf(aGlyph, &emptyGlyf); + if (!glyf) { + // TODO: for non-truetype fonts, get extents some other way? + return false; + } + + if (emptyGlyf) { + aExtents->x_bearing = 0; + aExtents->y_bearing = 0; + aExtents->width = 0; + aExtents->height = 0; + return true; + } + + double f = mFont->FUnitsToDevUnitsFactor(); + aExtents->x_bearing = FloatToFixed(int16_t(glyf->xMin) * f); + aExtents->width = + FloatToFixed((int16_t(glyf->xMax) - int16_t(glyf->xMin)) * f); + + // Our y-coordinates are positive-downwards, whereas harfbuzz assumes + // positive-upwards; hence the apparently-reversed subtractions here. + aExtents->y_bearing = + FloatToFixed(int16_t(glyf->yMax) * f - + mFont->GetHorizontalMetrics().emAscent); + aExtents->height = + FloatToFixed((int16_t(glyf->yMin) - int16_t(glyf->yMax)) * f); + + return true; +} + static hb_bool_t HBGetContourPoint(hb_font_t *font, void *font_data, unsigned int point_index, hb_codepoint_t glyph, @@ -1066,6 +1133,9 @@ gfxHarfBuzzShaper::Initialize() hb_font_funcs_set_glyph_v_origin_func(sHBFontFuncs, HBGetGlyphVOrigin, nullptr, nullptr); + hb_font_funcs_set_glyph_extents_func(sHBFontFuncs, + HBGetGlyphExtents, + nullptr, nullptr); hb_font_funcs_set_glyph_contour_point_func(sHBFontFuncs, HBGetContourPoint, nullptr, nullptr); @@ -1225,20 +1295,6 @@ gfxHarfBuzzShaper::InitializeVertical() mVORGTable = nullptr; } } - } else if (mVmtxTable) { - // Otherwise, try to load loca and glyf tables so that we can read - // bounding boxes (needed to support vertical glyph origin). - uint32_t len; - gfxFontEntry::AutoTable headTable(entry, - TRUETYPE_TAG('h','e','a','d')); - const HeadTable* head = - reinterpret_cast(hb_blob_get_data(headTable, - &len)); - if (len >= sizeof(HeadTable)) { - mLocaLongOffsets = int16_t(head->indexToLocFormat) > 0; - mLocaTable = entry->GetFontTable(TRUETYPE_TAG('l','o','c','a')); - mGlyfTable = entry->GetFontTable(TRUETYPE_TAG('g','l','y','f')); - } } return true; diff --git a/gfx/thebes/gfxHarfBuzzShaper.h b/gfx/thebes/gfxHarfBuzzShaper.h index 10ca6459ab3..150eb851c5e 100644 --- a/gfx/thebes/gfxHarfBuzzShaper.h +++ b/gfx/thebes/gfxHarfBuzzShaper.h @@ -73,6 +73,9 @@ public: hb_position_t GetHKerning(uint16_t aFirstGlyph, uint16_t aSecondGlyph) const; + hb_bool_t GetGlyphExtents(hb_codepoint_t aGlyph, + hb_glyph_extents_t *aExtents) const; + static hb_script_t GetHBScriptUsedForShaping(int32_t aScript) { // Decide what harfbuzz script code will be used for shaping @@ -107,6 +110,17 @@ protected: bool InitializeVertical(); bool LoadHmtxTable(); + struct Glyf { // we only need the bounding-box at the beginning + // of the glyph record, not the actual outline data + AutoSwap_PRInt16 numberOfContours; + AutoSwap_PRInt16 xMin; + AutoSwap_PRInt16 yMin; + AutoSwap_PRInt16 xMax; + AutoSwap_PRInt16 yMax; + }; + + const Glyf *FindGlyf(hb_codepoint_t aGlyph, bool *aEmptyGlyf) const; + // harfbuzz face object: we acquire a reference from the font entry // on shaper creation, and release it in our destructor hb_face_t *mHBFace; @@ -161,7 +175,10 @@ protected: bool mInitialized; bool mVerticalInitialized; - bool mLocaLongOffsets; + + // these are set from the FindGlyf callback on first use of the glyf data + mutable bool mLoadedLocaGlyf; + mutable bool mLocaLongOffsets; }; #endif /* GFX_HARFBUZZSHAPER_H */ From 7802daf2c9fac45a45f165430895f867aa1b1b36 Mon Sep 17 00:00:00 2001 From: Jonathan Kew Date: Wed, 28 Jan 2015 09:03:30 +0000 Subject: [PATCH 03/53] Bug 1108177 - Reftest to check for fallback mark stacking. r=jdaggett --- .../text/fallback-mark-stacking-1-notref.html | 23 +++++++++++++++++++ .../text/fallback-mark-stacking-1.html | 23 +++++++++++++++++++ layout/reftests/text/reftest.list | 2 ++ 3 files changed, 48 insertions(+) create mode 100644 layout/reftests/text/fallback-mark-stacking-1-notref.html create mode 100644 layout/reftests/text/fallback-mark-stacking-1.html diff --git a/layout/reftests/text/fallback-mark-stacking-1-notref.html b/layout/reftests/text/fallback-mark-stacking-1-notref.html new file mode 100644 index 00000000000..85df2ebd160 --- /dev/null +++ b/layout/reftests/text/fallback-mark-stacking-1-notref.html @@ -0,0 +1,23 @@ + + + + + + + +These examples should NOT look the same: +
x̃̂ x̂̃
+ diff --git a/layout/reftests/text/fallback-mark-stacking-1.html b/layout/reftests/text/fallback-mark-stacking-1.html new file mode 100644 index 00000000000..2a6138f8f9d --- /dev/null +++ b/layout/reftests/text/fallback-mark-stacking-1.html @@ -0,0 +1,23 @@ + + + + + + + +These examples should NOT look the same: +
x̂̃ x̃̂
+ diff --git a/layout/reftests/text/reftest.list b/layout/reftests/text/reftest.list index 054aaf6f8df..d31f39e83a6 100644 --- a/layout/reftests/text/reftest.list +++ b/layout/reftests/text/reftest.list @@ -163,6 +163,8 @@ fails-if(cocoaWidget||Android||B2G) HTTP(..) == arabic-fallback-2.html arabic-fa fails-if(cocoaWidget||Android||B2G) HTTP(..) == arabic-fallback-3.html arabic-fallback-3-ref.html fails-if(!cocoaWidget&&!Android&&!B2G) HTTP(..) != arabic-fallback-4.html arabic-fallback-4-notref.html == arabic-marks-1.html arabic-marks-1-ref.html +# harfbuzz fallback mark stacking in the absence of GPOS: +HTTP(..) != fallback-mark-stacking-1.html fallback-mark-stacking-1-notref.html == 726392-1.html 726392-1-ref.html == 726392-2.html 726392-2-ref.html From 5ae13bc43a614eb728ae2edc90ab34ad5fb4ea7c Mon Sep 17 00:00:00 2001 From: Jonathan Kew Date: Wed, 28 Jan 2015 09:07:59 +0000 Subject: [PATCH 04/53] Bug 1126420 - Don't assert when intrinsic size values are used for a block-size property, just pretend they're 'auto' for now. r=dbaron --- layout/generic/nsBlockFrame.cpp | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/layout/generic/nsBlockFrame.cpp b/layout/generic/nsBlockFrame.cpp index f0aea1c517c..dfcb07bef7f 100644 --- a/layout/generic/nsBlockFrame.cpp +++ b/layout/generic/nsBlockFrame.cpp @@ -2921,10 +2921,22 @@ nsBlockFrame::AttributeChanged(int32_t aNameSpaceID, } static inline bool -IsNonAutoNonZeroHeight(const nsStyleCoord& aCoord) +IsNonAutoNonZeroBSize(const nsStyleCoord& aCoord) { - if (aCoord.GetUnit() == eStyleUnit_Auto) + nsStyleUnit unit = aCoord.GetUnit(); + if (unit == eStyleUnit_Auto || + // The enumerated values were originally aimed at inline-size + // (or width, as it was before logicalization). For now, let them + // return false here, so we treat them like 'auto' pending a + // real implementation. (See bug 1126420.) + // + // FIXME (bug 567039, bug 527285) + // This isn't correct for the 'fill' value, which should more + // likely (but not necessarily, depending on the available space) + // be returning true. + unit == eStyleUnit_Enumerated) { return false; + } if (aCoord.IsCoordPercentCalcUnit()) { // If we evaluate the length/percent/calc at a percentage basis of // both nscoord_MAX and 0, and it's zero both ways, then it's a zero @@ -2951,13 +2963,13 @@ nsBlockFrame::IsSelfEmpty() bool vertical = GetWritingMode().IsVertical(); if (vertical) { - if (IsNonAutoNonZeroHeight(position->mMinWidth) || - IsNonAutoNonZeroHeight(position->mWidth)) { + if (IsNonAutoNonZeroBSize(position->mMinWidth) || + IsNonAutoNonZeroBSize(position->mWidth)) { return false; } } else { - if (IsNonAutoNonZeroHeight(position->mMinHeight) || - IsNonAutoNonZeroHeight(position->mHeight)) { + if (IsNonAutoNonZeroBSize(position->mMinHeight) || + IsNonAutoNonZeroBSize(position->mHeight)) { return false; } } From deae8764f21faa5eb5ec810296094dc0fab4c4b3 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Tue, 27 Jan 2015 13:19:49 +1100 Subject: [PATCH 05/53] Bug 649142 - Part 9: Avoid transitioning border-end-color on the bookmark toolbar icon when switching between enabled and disabled states. r=Unfocused --- browser/themes/windows/browser.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/browser/themes/windows/browser.css b/browser/themes/windows/browser.css index e7cf59c91d3..204c3925747 100644 --- a/browser/themes/windows/browser.css +++ b/browser/themes/windows/browser.css @@ -753,7 +753,7 @@ toolbarbutton[sdk-button="true"][cui-areatype="toolbar"] > .toolbarbutton-icon { } #nav-bar .toolbarbutton-1 > .toolbarbutton-menubutton-button > .toolbarbutton-icon { - -moz-border-end: none; + -moz-border-end: none transparent; } #nav-bar .toolbarbutton-1 > .toolbarbutton-menubutton-dropmarker > .dropmarker-icon { From 41778cfef0c999641f8a932410e8c9c7766571b5 Mon Sep 17 00:00:00 2001 From: Bob Owen Date: Wed, 28 Jan 2015 11:21:24 +0000 Subject: [PATCH 06/53] Bug 1125865: Only log Windows sandbox violations to console when nsContentUtils is initialized. r=bbondy --- security/sandbox/chromium-shim/sandbox/win/loggingCallbacks.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/security/sandbox/chromium-shim/sandbox/win/loggingCallbacks.h b/security/sandbox/chromium-shim/sandbox/win/loggingCallbacks.h index 23a3b785778..75747744b96 100644 --- a/security/sandbox/chromium-shim/sandbox/win/loggingCallbacks.h +++ b/security/sandbox/chromium-shim/sandbox/win/loggingCallbacks.h @@ -103,7 +103,9 @@ Log(const char* aMessageType, NS_DebugBreak(NS_DEBUG_WARNING, nullptr, msg.c_str(), nullptr, -1); #endif - nsContentUtils::LogMessageToConsole(msg.c_str()); + if (nsContentUtils::IsInitialized()) { + nsContentUtils::LogMessageToConsole(msg.c_str()); + } } // Initialize sandbox logging if required. From aceb6ebcc0163135d8724f11518b227b0800379e Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Wed, 28 Jan 2015 10:51:00 +0100 Subject: [PATCH 07/53] Bug 1126409: Add SIMD check in test + move SIMD tests into jit-tests/tests/SIMD; r=h4writer --HG-- rename : js/src/jit-test/tests/ion/simd-bug1121299.js => js/src/jit-test/tests/SIMD/bug1121299.js rename : js/src/jit-test/tests/ion/simd-bug1123631.js => js/src/jit-test/tests/SIMD/bug1123631.js rename : js/src/jit-test/tests/ion/simd-unbox.js => js/src/jit-test/tests/SIMD/unbox.js extra : rebase_source : 755008a582298295fb6877341fbcc5bf529abbec --- .../tests/{ion/simd-bug1121299.js => SIMD/bug1121299.js} | 3 +++ .../tests/{ion/simd-bug1123631.js => SIMD/bug1123631.js} | 0 js/src/jit-test/tests/{ion/simd-unbox.js => SIMD/unbox.js} | 0 3 files changed, 3 insertions(+) rename js/src/jit-test/tests/{ion/simd-bug1121299.js => SIMD/bug1121299.js} (93%) rename js/src/jit-test/tests/{ion/simd-bug1123631.js => SIMD/bug1123631.js} (100%) rename js/src/jit-test/tests/{ion/simd-unbox.js => SIMD/unbox.js} (100%) diff --git a/js/src/jit-test/tests/ion/simd-bug1121299.js b/js/src/jit-test/tests/SIMD/bug1121299.js similarity index 93% rename from js/src/jit-test/tests/ion/simd-bug1121299.js rename to js/src/jit-test/tests/SIMD/bug1121299.js index 1c1c2b9eb70..c610e5b9615 100644 --- a/js/src/jit-test/tests/ion/simd-bug1121299.js +++ b/js/src/jit-test/tests/SIMD/bug1121299.js @@ -1,3 +1,6 @@ +if (!this.hasOwnProperty("SIMD")) + quit(); + setJitCompilerOption("baseline.warmup.trigger", 10); setJitCompilerOption("ion.warmup.trigger", 30); diff --git a/js/src/jit-test/tests/ion/simd-bug1123631.js b/js/src/jit-test/tests/SIMD/bug1123631.js similarity index 100% rename from js/src/jit-test/tests/ion/simd-bug1123631.js rename to js/src/jit-test/tests/SIMD/bug1123631.js diff --git a/js/src/jit-test/tests/ion/simd-unbox.js b/js/src/jit-test/tests/SIMD/unbox.js similarity index 100% rename from js/src/jit-test/tests/ion/simd-unbox.js rename to js/src/jit-test/tests/SIMD/unbox.js From f1abe83b3f58b3af16ae6071711e403c598621f4 Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Wed, 28 Jan 2015 22:36:53 +0900 Subject: [PATCH 08/53] Bug 936313 part.1 Remove DOM_KEY_LOCATION_MOBILE and DOM_KEY_LOCATION_JOYSTICK r=smaug+mwu+cpeterson, sr=smaug --- dom/base/nsDOMWindowUtils.cpp | 9 +--- dom/events/test/test_dom_keyboard_event.html | 16 ------- dom/interfaces/base/nsIDOMWindowUtils.idl | 4 +- dom/interfaces/events/nsIDOMKeyEvent.idl | 2 - dom/webidl/KeyboardEvent.webidl | 2 - mobile/android/base/GeckoEvent.java | 45 ------------------- .../mozmill/resource/stdlib/EventUtils.js | 6 --- .../mochitest/tests/SimpleTest/EventUtils.js | 6 --- widget/android/AndroidJavaWrappers.cpp | 23 ---------- widget/android/AndroidJavaWrappers.h | 8 ---- widget/android/nsWindow.cpp | 3 +- widget/gonk/nsAppShell.cpp | 3 +- 12 files changed, 6 insertions(+), 121 deletions(-) diff --git a/dom/base/nsDOMWindowUtils.cpp b/dom/base/nsDOMWindowUtils.cpp index 95f7d88e550..c78d9d39dc6 100644 --- a/dom/base/nsDOMWindowUtils.cpp +++ b/dom/base/nsDOMWindowUtils.cpp @@ -1220,8 +1220,7 @@ nsDOMWindowUtils::SendKeyEvent(const nsAString& aType, uint32_t locationFlag = (aAdditionalFlags & (KEY_FLAG_LOCATION_STANDARD | KEY_FLAG_LOCATION_LEFT | - KEY_FLAG_LOCATION_RIGHT | KEY_FLAG_LOCATION_NUMPAD | - KEY_FLAG_LOCATION_MOBILE | KEY_FLAG_LOCATION_JOYSTICK)); + KEY_FLAG_LOCATION_RIGHT | KEY_FLAG_LOCATION_NUMPAD)); switch (locationFlag) { case KEY_FLAG_LOCATION_STANDARD: event.location = nsIDOMKeyEvent::DOM_KEY_LOCATION_STANDARD; @@ -1235,12 +1234,6 @@ nsDOMWindowUtils::SendKeyEvent(const nsAString& aType, case KEY_FLAG_LOCATION_NUMPAD: event.location = nsIDOMKeyEvent::DOM_KEY_LOCATION_NUMPAD; break; - case KEY_FLAG_LOCATION_MOBILE: - event.location = nsIDOMKeyEvent::DOM_KEY_LOCATION_MOBILE; - break; - case KEY_FLAG_LOCATION_JOYSTICK: - event.location = nsIDOMKeyEvent::DOM_KEY_LOCATION_JOYSTICK; - break; default: if (locationFlag != 0) { return NS_ERROR_INVALID_ARG; diff --git a/dom/events/test/test_dom_keyboard_event.html b/dom/events/test/test_dom_keyboard_event.html index 47ca8c2fed7..4971cd1ba88 100644 --- a/dom/events/test/test_dom_keyboard_event.html +++ b/dom/events/test/test_dom_keyboard_event.html @@ -200,10 +200,6 @@ function testSynthesizedKeyLocation() event: { shiftKey: false, ctrlKey: false, altKey: false, metaKey: false, location: KeyboardEvent.DOM_KEY_LOCATION_NUMPAD }, }, - { key: "VK_DOWN", isModifier: false, - event: { shiftKey: false, ctrlKey: false, altKey: false, metaKey: false, - location: KeyboardEvent.DOM_KEY_LOCATION_JOYSTICK }, - }, { key: "5", isModifier: false, event: { shiftKey: false, ctrlKey: false, altKey: false, metaKey: false, location: KeyboardEvent.DOM_KEY_LOCATION_STANDARD }, @@ -212,14 +208,6 @@ function testSynthesizedKeyLocation() event: { shiftKey: false, ctrlKey: false, altKey: false, metaKey: false, location: KeyboardEvent.DOM_KEY_LOCATION_NUMPAD }, }, - { key: "5", isModifier: false, - event: { shiftKey: false, ctrlKey: false, altKey: false, metaKey: false, - location: KeyboardEvent.DOM_KEY_LOCATION_MOBILE }, - }, - { key: "VK_NUMPAD5", isModifier: false, - event: { shiftKey: false, ctrlKey: false, altKey: false, metaKey: false, - location: KeyboardEvent.DOM_KEY_LOCATION_MOBILE }, - }, { key: "+", isModifier: false, event: { shiftKey: false, ctrlKey: false, altKey: false, metaKey: false, location: KeyboardEvent.DOM_KEY_LOCATION_STANDARD }, @@ -259,12 +247,8 @@ function testSynthesizedKeyLocation() return "DOM_KEY_LOCATION_LEFT"; case KeyboardEvent.DOM_KEY_LOCATION_RIGHT: return "DOM_KEY_LOCATION_RIGHT"; - case KeyboardEvent.DOM_KEY_LOCATION_MOBILE: - return "DOM_KEY_LOCATION_MOBILE"; case KeyboardEvent.DOM_KEY_LOCATION_NUMPAD: return "DOM_KEY_LOCATION_NUMPAD"; - case KeyboardEvent.DOM_KEY_LOCATION_JOYSTICK: - return "DOM_KEY_LOCATION_JOYSTICK"; default: return "Invalid value (" + aLocation + ")"; } diff --git a/dom/interfaces/base/nsIDOMWindowUtils.idl b/dom/interfaces/base/nsIDOMWindowUtils.idl index 1c4d7e76d81..f75e9384b29 100644 --- a/dom/interfaces/base/nsIDOMWindowUtils.idl +++ b/dom/interfaces/base/nsIDOMWindowUtils.idl @@ -50,7 +50,7 @@ interface nsITranslationNodeList; interface nsIJSRAIIHelper; interface nsIContentPermissionRequest; -[scriptable, uuid(04db2684-f9ed-4d70-827d-3d5b87825238)] +[scriptable, uuid(5ed850de-2b57-4555-ac48-93292e852eab)] interface nsIDOMWindowUtils : nsISupports { /** @@ -556,8 +556,6 @@ interface nsIDOMWindowUtils : nsISupports { const unsigned long KEY_FLAG_LOCATION_LEFT = 0x0020; const unsigned long KEY_FLAG_LOCATION_RIGHT = 0x0040; const unsigned long KEY_FLAG_LOCATION_NUMPAD = 0x0080; - const unsigned long KEY_FLAG_LOCATION_MOBILE = 0x0100; - const unsigned long KEY_FLAG_LOCATION_JOYSTICK = 0x0200; boolean sendKeyEvent(in AString aType, in long aKeyCode, diff --git a/dom/interfaces/events/nsIDOMKeyEvent.idl b/dom/interfaces/events/nsIDOMKeyEvent.idl index 40808da8bdb..b9dd3a3973a 100644 --- a/dom/interfaces/events/nsIDOMKeyEvent.idl +++ b/dom/interfaces/events/nsIDOMKeyEvent.idl @@ -250,8 +250,6 @@ interface nsIDOMKeyEvent : nsIDOMUIEvent const unsigned long DOM_KEY_LOCATION_LEFT = 0x01; const unsigned long DOM_KEY_LOCATION_RIGHT = 0x02; const unsigned long DOM_KEY_LOCATION_NUMPAD = 0x03; - const unsigned long DOM_KEY_LOCATION_MOBILE = 0x04; - const unsigned long DOM_KEY_LOCATION_JOYSTICK = 0x05; readonly attribute unsigned long location; readonly attribute boolean repeat; diff --git a/dom/webidl/KeyboardEvent.webidl b/dom/webidl/KeyboardEvent.webidl index e8b6c40131f..274a6d6c227 100644 --- a/dom/webidl/KeyboardEvent.webidl +++ b/dom/webidl/KeyboardEvent.webidl @@ -21,8 +21,6 @@ interface KeyboardEvent : UIEvent const unsigned long DOM_KEY_LOCATION_LEFT = 0x01; const unsigned long DOM_KEY_LOCATION_RIGHT = 0x02; const unsigned long DOM_KEY_LOCATION_NUMPAD = 0x03; - const unsigned long DOM_KEY_LOCATION_MOBILE = 0x04; - const unsigned long DOM_KEY_LOCATION_JOYSTICK = 0x05; readonly attribute unsigned long location; readonly attribute boolean repeat; diff --git a/mobile/android/base/GeckoEvent.java b/mobile/android/base/GeckoEvent.java index 93bde7dec20..0fa41d6390d 100644 --- a/mobile/android/base/GeckoEvent.java +++ b/mobile/android/base/GeckoEvent.java @@ -115,26 +115,6 @@ public class GeckoEvent { } } - /** - * The DomKeyLocation enum encapsulates the DOM KeyboardEvent's constants. - * @see https://developer.mozilla.org/en-US/docs/DOM/KeyboardEvent#Key_location_constants - */ - @JNITarget - public enum DomKeyLocation { - DOM_KEY_LOCATION_STANDARD(0), - DOM_KEY_LOCATION_LEFT(1), - DOM_KEY_LOCATION_RIGHT(2), - DOM_KEY_LOCATION_NUMPAD(3), - DOM_KEY_LOCATION_MOBILE(4), - DOM_KEY_LOCATION_JOYSTICK(5); - - public final int value; - - private DomKeyLocation(int value) { - this.value = value; - } - } - // Encapsulation of common IME actions. @JNITarget public enum ImeAction { @@ -223,7 +203,6 @@ public class GeckoEvent { private int mRangeLineColor; private Location mLocation; private Address mAddress; - private DomKeyLocation mDomKeyLocation; private int mConnectionType; private boolean mIsWifi; @@ -313,30 +292,6 @@ public class GeckoEvent { mDOMPrintableKeyValue = k.getUnicodeChar(unmodifiedMetaState); } } - mDomKeyLocation = isJoystickButton(mKeyCode) ? DomKeyLocation.DOM_KEY_LOCATION_JOYSTICK - : DomKeyLocation.DOM_KEY_LOCATION_MOBILE; - } - - /** - * This method tests if a key is one of the described in: - * https://bugzilla.mozilla.org/show_bug.cgi?id=756504#c0 - * @param keyCode int with the key code (Android key constant from KeyEvent) - * @return true if the key is one of the listed above, false otherwise. - */ - private static boolean isJoystickButton(int keyCode) { - switch (keyCode) { - case KeyEvent.KEYCODE_DPAD_CENTER: - case KeyEvent.KEYCODE_DPAD_LEFT: - case KeyEvent.KEYCODE_DPAD_RIGHT: - case KeyEvent.KEYCODE_DPAD_DOWN: - case KeyEvent.KEYCODE_DPAD_UP: - return true; - default: - if (Versions.feature12Plus) { - return KeyEvent.isGamepadButton(keyCode); - } - return GeckoEvent.isGamepadButton(keyCode); - } } /** diff --git a/services/sync/tps/extensions/mozmill/resource/stdlib/EventUtils.js b/services/sync/tps/extensions/mozmill/resource/stdlib/EventUtils.js index dcc5e726f5d..a821ab2e017 100644 --- a/services/sync/tps/extensions/mozmill/resource/stdlib/EventUtils.js +++ b/services/sync/tps/extensions/mozmill/resource/stdlib/EventUtils.js @@ -552,12 +552,6 @@ function synthesizeKey(aKey, aEvent, aWindow) case KeyboardEvent.DOM_KEY_LOCATION_NUMPAD: flags |= utils.KEY_FLAG_LOCATION_NUMPAD; break; - case KeyboardEvent.DOM_KEY_LOCATION_MOBILE: - flags |= utils.KEY_FLAG_LOCATION_MOBILE; - break; - case KeyboardEvent.DOM_KEY_LOCATION_JOYSTICK: - flags |= utils.KEY_FLAG_LOCATION_JOYSTICK; - break; } } diff --git a/testing/mochitest/tests/SimpleTest/EventUtils.js b/testing/mochitest/tests/SimpleTest/EventUtils.js index 45799761bde..b09549f6655 100644 --- a/testing/mochitest/tests/SimpleTest/EventUtils.js +++ b/testing/mochitest/tests/SimpleTest/EventUtils.js @@ -586,12 +586,6 @@ function synthesizeKey(aKey, aEvent, aWindow) case KeyboardEvent.DOM_KEY_LOCATION_NUMPAD: flags |= utils.KEY_FLAG_LOCATION_NUMPAD; break; - case KeyboardEvent.DOM_KEY_LOCATION_MOBILE: - flags |= utils.KEY_FLAG_LOCATION_MOBILE; - break; - case KeyboardEvent.DOM_KEY_LOCATION_JOYSTICK: - flags |= utils.KEY_FLAG_LOCATION_JOYSTICK; - break; } } diff --git a/widget/android/AndroidJavaWrappers.cpp b/widget/android/AndroidJavaWrappers.cpp index 838567fcfbd..37a7804e3bf 100644 --- a/widget/android/AndroidJavaWrappers.cpp +++ b/widget/android/AndroidJavaWrappers.cpp @@ -40,7 +40,6 @@ jfieldID AndroidGeckoEvent::jDOMPrintableKeyValueField = 0; jfieldID AndroidGeckoEvent::jKeyCodeField = 0; jfieldID AndroidGeckoEvent::jScanCodeField = 0; jfieldID AndroidGeckoEvent::jMetaStateField = 0; -jfieldID AndroidGeckoEvent::jDomKeyLocationField = 0; jfieldID AndroidGeckoEvent::jFlagsField = 0; jfieldID AndroidGeckoEvent::jUnicodeCharField = 0; jfieldID AndroidGeckoEvent::jBaseUnicodeCharField = 0; @@ -72,9 +71,6 @@ jfieldID AndroidGeckoEvent::jGamepadValuesField = 0; jfieldID AndroidGeckoEvent::jPrefNamesField = 0; jfieldID AndroidGeckoEvent::jObjectField = 0; -jclass AndroidGeckoEvent::jDomKeyLocationClass = 0; -jfieldID AndroidGeckoEvent::jDomKeyLocationValueField = 0; - jclass AndroidPoint::jPointClass = 0; jfieldID AndroidPoint::jXField = 0; jfieldID AndroidPoint::jYField = 0; @@ -150,7 +146,6 @@ AndroidGeckoEvent::InitGeckoEventClass(JNIEnv *jEnv) jKeyCodeField = geckoEvent.getField("mKeyCode", "I"); jScanCodeField = geckoEvent.getField("mScanCode", "I"); jMetaStateField = geckoEvent.getField("mMetaState", "I"); - jDomKeyLocationField = geckoEvent.getField("mDomKeyLocation", "Lorg/mozilla/gecko/GeckoEvent$DomKeyLocation;"); jFlagsField = geckoEvent.getField("mFlags", "I"); jUnicodeCharField = geckoEvent.getField("mUnicodeChar", "I"); jBaseUnicodeCharField = geckoEvent.getField("mBaseUnicodeChar", "I"); @@ -182,11 +177,6 @@ AndroidGeckoEvent::InitGeckoEventClass(JNIEnv *jEnv) jGamepadValuesField = geckoEvent.getField("mGamepadValues", "[F"); jPrefNamesField = geckoEvent.getField("mPrefNames", "[Ljava/lang/String;"); jObjectField = geckoEvent.getField("mObject", "Ljava/lang/Object;"); - - // Init GeckoEvent.DomKeyLocation enum - AutoJNIClass domKeyLocation(jEnv, "org/mozilla/gecko/GeckoEvent$DomKeyLocation"); - jDomKeyLocationClass = domKeyLocation.getGlobalRef(); - jDomKeyLocationValueField = domKeyLocation.getField("value", "I"); } void @@ -389,18 +379,6 @@ AndroidGeckoEvent::UnionRect(nsIntRect const& aRect) mRect = aRect.Union(mRect); } -uint32_t -AndroidGeckoEvent::ReadDomKeyLocation(JNIEnv* jenv, jobject jGeckoEventObj) -{ - jobject enumObject = jenv->GetObjectField(jGeckoEventObj, - jDomKeyLocationField); - MOZ_ASSERT(enumObject); - int enumValue = jenv->GetIntField(enumObject, jDomKeyLocationValueField); - MOZ_ASSERT(enumValue >= nsIDOMKeyEvent::DOM_KEY_LOCATION_STANDARD && - enumValue <= nsIDOMKeyEvent::DOM_KEY_LOCATION_JOYSTICK); - return static_cast(enumValue); -} - void AndroidGeckoEvent::Init(JNIEnv *jenv, jobject jobj) { @@ -424,7 +402,6 @@ AndroidGeckoEvent::Init(JNIEnv *jenv, jobject jobj) case IME_KEY_EVENT: mTime = jenv->GetLongField(jobj, jTimeField); mMetaState = jenv->GetIntField(jobj, jMetaStateField); - mDomKeyLocation = ReadDomKeyLocation(jenv, jobj); mFlags = jenv->GetIntField(jobj, jFlagsField); mKeyCode = jenv->GetIntField(jobj, jKeyCodeField); mScanCode = jenv->GetIntField(jobj, jScanCodeField); diff --git a/widget/android/AndroidJavaWrappers.h b/widget/android/AndroidJavaWrappers.h index 48f12f0d355..0a966dda631 100644 --- a/widget/android/AndroidJavaWrappers.h +++ b/widget/android/AndroidJavaWrappers.h @@ -526,7 +526,6 @@ public: int KeyCode() { return mKeyCode; } int ScanCode() { return mScanCode; } int MetaState() { return mMetaState; } - uint32_t DomKeyLocation() { return mDomKeyLocation; } Modifiers DOMModifiers() const; bool IsAltPressed() const { return (mMetaState & AMETA_ALT_MASK) != 0; } bool IsShiftPressed() const { return (mMetaState & AMETA_SHIFT_MASK) != 0; } @@ -585,7 +584,6 @@ protected: nsTArray mToolTypes; nsIntRect mRect; int mFlags, mMetaState; - uint32_t mDomKeyLocation; int mKeyCode, mScanCode; int mUnicodeChar, mBaseUnicodeChar, mDOMPrintableKeyValue; int mRepeatCount; @@ -637,8 +635,6 @@ protected: void ReadDataField(JNIEnv *jenv); void ReadStringFromJString(nsString &aString, JNIEnv *jenv, jstring s); - uint32_t ReadDomKeyLocation(JNIEnv* jenv, jobject jGeckoEventObj); - static jclass jGeckoEventClass; static jfieldID jActionField; static jfieldID jTypeField; @@ -664,7 +660,6 @@ protected: static jfieldID jKeyCodeField; static jfieldID jScanCodeField; static jfieldID jMetaStateField; - static jfieldID jDomKeyLocationField; static jfieldID jFlagsField; static jfieldID jCountField; static jfieldID jStartField; @@ -701,9 +696,6 @@ protected: static jfieldID jObjectField; - static jclass jDomKeyLocationClass; - static jfieldID jDomKeyLocationValueField; - public: enum { NATIVE_POKE = 0, diff --git a/widget/android/nsWindow.cpp b/widget/android/nsWindow.cpp index 19e386245fe..ad34d8a7734 100644 --- a/widget/android/nsWindow.cpp +++ b/widget/android/nsWindow.cpp @@ -1540,7 +1540,8 @@ nsWindow::InitKeyEvent(WidgetKeyboardEvent& event, AndroidGeckoEvent& key, event.mIsRepeat = (event.message == NS_KEY_DOWN || event.message == NS_KEY_PRESS) && (!!(key.Flags() & AKEY_EVENT_FLAG_LONG_PRESS) || !!key.RepeatCount()); - event.location = key.DomKeyLocation(); + // XXX Compute the location from code value, later. + event.location = nsIDOMKeyboardEvent::DOM_KEY_LOCATION_STANDARD; event.time = key.Time(); if (gMenu) diff --git a/widget/gonk/nsAppShell.cpp b/widget/gonk/nsAppShell.cpp index acc588ac245..ebdcd7ea28d 100644 --- a/widget/gonk/nsAppShell.cpp +++ b/widget/gonk/nsAppShell.cpp @@ -308,7 +308,8 @@ KeyEventDispatcher::DispatchKeyEventInternal(uint32_t aEventMessage) } event.mCodeNameIndex = mDOMCodeNameIndex; event.modifiers = getDOMModifiers(mData.metaState); - event.location = nsIDOMKeyEvent::DOM_KEY_LOCATION_MOBILE; + // XXX Compute the location from code value, later. + event.location = nsIDOMKeyEvent::DOM_KEY_LOCATION_STANDARD; event.time = mData.timeMs; return nsWindow::DispatchInputEvent(event); } From 6e75a14df0f1a480ccf8183083e620b5fbfb3cfc Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Wed, 28 Jan 2015 22:36:53 +0900 Subject: [PATCH 09/53] Bug 936313 part.2 Compute DOM key location from code value on Android and Gonk r=smaug+mwu+cpeterson --- widget/TextEvents.h | 2 ++ widget/WidgetEventImpl.cpp | 51 +++++++++++++++++++++++++++++++++++++ widget/android/nsWindow.cpp | 4 +-- widget/gonk/nsAppShell.cpp | 15 +++++++---- 4 files changed, 65 insertions(+), 7 deletions(-) diff --git a/widget/TextEvents.h b/widget/TextEvents.h index e9ec1d22349..957a7b15ac5 100644 --- a/widget/TextEvents.h +++ b/widget/TextEvents.h @@ -180,6 +180,8 @@ public: GetDOMCodeName(mCodeNameIndex, aCodeName); } + static uint32_t ComputeLocationFromCodeValue(CodeNameIndex aCodeNameIndex); + static void GetDOMKeyName(KeyNameIndex aKeyNameIndex, nsAString& aKeyName); static void GetDOMCodeName(CodeNameIndex aCodeNameIndex, diff --git a/widget/WidgetEventImpl.cpp b/widget/WidgetEventImpl.cpp index d8b8f27a665..9386128a6ae 100644 --- a/widget/WidgetEventImpl.cpp +++ b/widget/WidgetEventImpl.cpp @@ -393,4 +393,55 @@ WidgetKeyboardEvent::GetCommandStr(Command aCommand) return kCommands[aCommand]; } +/* static */ uint32_t +WidgetKeyboardEvent::ComputeLocationFromCodeValue(CodeNameIndex aCodeNameIndex) +{ + // Following commented out cases are not defined in PhysicalKeyCodeNameList.h + // but are defined by D3E spec. So, they should be uncommented when the + // code values are defined in the header. + switch (aCodeNameIndex) { + case CODE_NAME_INDEX_AltLeft: + case CODE_NAME_INDEX_ControlLeft: + case CODE_NAME_INDEX_OSLeft: + case CODE_NAME_INDEX_ShiftLeft: + return nsIDOMKeyEvent::DOM_KEY_LOCATION_LEFT; + case CODE_NAME_INDEX_AltRight: + case CODE_NAME_INDEX_ControlRight: + case CODE_NAME_INDEX_OSRight: + case CODE_NAME_INDEX_ShiftRight: + return nsIDOMKeyEvent::DOM_KEY_LOCATION_RIGHT; + case CODE_NAME_INDEX_Numpad0: + case CODE_NAME_INDEX_Numpad1: + case CODE_NAME_INDEX_Numpad2: + case CODE_NAME_INDEX_Numpad3: + case CODE_NAME_INDEX_Numpad4: + case CODE_NAME_INDEX_Numpad5: + case CODE_NAME_INDEX_Numpad6: + case CODE_NAME_INDEX_Numpad7: + case CODE_NAME_INDEX_Numpad8: + case CODE_NAME_INDEX_Numpad9: + case CODE_NAME_INDEX_NumpadAdd: + case CODE_NAME_INDEX_NumpadBackspace: + // case CODE_NAME_INDEX_NumpadClear: + // case CODE_NAME_INDEX_NumpadClearEntry: + case CODE_NAME_INDEX_NumpadComma: + case CODE_NAME_INDEX_NumpadDecimal: + case CODE_NAME_INDEX_NumpadDivide: + case CODE_NAME_INDEX_NumpadEnter: + case CODE_NAME_INDEX_NumpadEqual: + // case CODE_NAME_INDEX_NumpadMemoryAdd: + // case CODE_NAME_INDEX_NumpadMemoryClear: + // case CODE_NAME_INDEX_NumpadMemoryRecall: + // case CODE_NAME_INDEX_NumpadMemoryStore: + case CODE_NAME_INDEX_NumpadMemorySubtract: + case CODE_NAME_INDEX_NumpadMultiply: + // case CODE_NAME_INDEX_NumpadParenLeft: + // case CODE_NAME_INDEX_NumpadParenRight: + case CODE_NAME_INDEX_NumpadSubtract: + return nsIDOMKeyEvent::DOM_KEY_LOCATION_NUMPAD; + default: + return nsIDOMKeyEvent::DOM_KEY_LOCATION_STANDARD; + } +} + } // namespace mozilla diff --git a/widget/android/nsWindow.cpp b/widget/android/nsWindow.cpp index ad34d8a7734..7d17657f408 100644 --- a/widget/android/nsWindow.cpp +++ b/widget/android/nsWindow.cpp @@ -1540,8 +1540,8 @@ nsWindow::InitKeyEvent(WidgetKeyboardEvent& event, AndroidGeckoEvent& key, event.mIsRepeat = (event.message == NS_KEY_DOWN || event.message == NS_KEY_PRESS) && (!!(key.Flags() & AKEY_EVENT_FLAG_LONG_PRESS) || !!key.RepeatCount()); - // XXX Compute the location from code value, later. - event.location = nsIDOMKeyboardEvent::DOM_KEY_LOCATION_STANDARD; + event.location = + WidgetKeyboardEvent::ComputeLocationFromCodeValue(event.mCodeNameIndex); event.time = key.Time(); if (gMenu) diff --git a/widget/gonk/nsAppShell.cpp b/widget/gonk/nsAppShell.cpp index ebdcd7ea28d..951c59c9b97 100644 --- a/widget/gonk/nsAppShell.cpp +++ b/widget/gonk/nsAppShell.cpp @@ -212,6 +212,7 @@ private: char16_t mUnmodifiedChar; uint32_t mDOMKeyCode; + uint32_t mDOMKeyLocation; KeyNameIndex mDOMKeyNameIndex; CodeNameIndex mDOMCodeNameIndex; char16_t mDOMPrintableKeyValue; @@ -246,9 +247,12 @@ private: }; KeyEventDispatcher::KeyEventDispatcher(const UserInputData& aData, - KeyCharacterMap* aKeyCharMap) : - mData(aData), mKeyCharMap(aKeyCharMap), mChar(0), mUnmodifiedChar(0), - mDOMPrintableKeyValue(0) + KeyCharacterMap* aKeyCharMap) + : mData(aData) + , mKeyCharMap(aKeyCharMap) + , mChar(0) + , mUnmodifiedChar(0) + , mDOMPrintableKeyValue(0) { // XXX Printable key's keyCode value should be computed with actual // input character. @@ -256,6 +260,8 @@ KeyEventDispatcher::KeyEventDispatcher(const UserInputData& aData, kKeyMapping[mData.key.keyCode] : 0; mDOMKeyNameIndex = GetKeyNameIndex(mData.key.keyCode); mDOMCodeNameIndex = GetCodeNameIndex(mData.key.scanCode); + mDOMKeyLocation = + WidgetKeyboardEvent::ComputeLocationFromCodeValue(mDOMCodeNameIndex); if (!mKeyCharMap.get()) { return; @@ -308,8 +314,7 @@ KeyEventDispatcher::DispatchKeyEventInternal(uint32_t aEventMessage) } event.mCodeNameIndex = mDOMCodeNameIndex; event.modifiers = getDOMModifiers(mData.metaState); - // XXX Compute the location from code value, later. - event.location = nsIDOMKeyEvent::DOM_KEY_LOCATION_STANDARD; + event.location = mDOMKeyLocation; event.time = mData.timeMs; return nsWindow::DispatchInputEvent(event); } From a71d3b1b7050f74313773c3c87e77c86d5418cf7 Mon Sep 17 00:00:00 2001 From: Kartikaya Gupta Date: Wed, 28 Jan 2015 08:58:47 -0500 Subject: [PATCH 10/53] Back out bug 1116588 for introducing the regression in bug 1126427. r=me --- layout/base/nsDisplayList.cpp | 4 ++++ layout/generic/nsFrame.cpp | 11 +++++++++++ 2 files changed, 15 insertions(+) diff --git a/layout/base/nsDisplayList.cpp b/layout/base/nsDisplayList.cpp index 9d3948e8726..2090e244167 100644 --- a/layout/base/nsDisplayList.cpp +++ b/layout/base/nsDisplayList.cpp @@ -3735,6 +3735,10 @@ already_AddRefed nsDisplayOpacity::BuildLayer(nsDisplayListBuilder* aBuilder, LayerManager* aManager, const ContainerLayerParameters& aContainerParameters) { + if (mOpacity == 0 && mFrame->GetContent() && + !nsLayoutUtils::HasAnimations(mFrame->GetContent(), eCSSProperty_opacity)) { + return nullptr; + } nsRefPtr container = aManager->GetLayerBuilder()-> BuildContainerLayerFor(aBuilder, aManager, mFrame, this, &mList, aContainerParameters, nullptr); diff --git a/layout/generic/nsFrame.cpp b/layout/generic/nsFrame.cpp index 751451c5555..fb8f705c2be 100644 --- a/layout/generic/nsFrame.cpp +++ b/layout/generic/nsFrame.cpp @@ -1948,6 +1948,17 @@ nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder* aBuilder, return; const nsStyleDisplay* disp = StyleDisplay(); + // We can stop right away if this is a zero-opacity stacking context and + // we're painting, and we're not animating opacity. Don't do this + // if we're going to compute plugin geometry, since opacity-0 plugins + // need to have display items built for them. + if (disp->mOpacity == 0.0 && aBuilder->IsForPainting() && + !aBuilder->WillComputePluginGeometry() && + !(disp->mWillChangeBitField & NS_STYLE_WILL_CHANGE_OPACITY) && + !nsLayoutUtils::HasAnimations(mContent, eCSSProperty_opacity)) { + return; + } + if (disp->mWillChangeBitField != 0) { aBuilder->AddToWillChangeBudget(this, GetSize()); } From 86621ebadcf2fb60cc6b2f579680ecd30b27d5e4 Mon Sep 17 00:00:00 2001 From: Patrick McManus Date: Fri, 23 Jan 2015 11:22:22 -0500 Subject: [PATCH 11/53] bug 1124717 - 1/4 h2/spdy fix spin when queuing post/put request over concurrent limit r=hurley --- netwerk/protocol/http/Http2Stream.cpp | 5 ++--- netwerk/protocol/http/SpdyStream31.cpp | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/netwerk/protocol/http/Http2Stream.cpp b/netwerk/protocol/http/Http2Stream.cpp index 84a262b48a6..6dde4273aa6 100644 --- a/netwerk/protocol/http/Http2Stream.cpp +++ b/netwerk/protocol/http/Http2Stream.cpp @@ -184,11 +184,10 @@ Http2Stream::ReadSegments(nsAHttpSegmentReader *reader, // If the sending flow control window is open (!mBlockedOnRwin) then // continue sending the request - if (!mBlockedOnRwin && mUpstreamState != GENERATING_HEADERS && + if (!mBlockedOnRwin && mOpenGenerated && !mTxInlineFrameUsed && NS_SUCCEEDED(rv) && (!*countRead)) { MOZ_ASSERT(!mQueued); MOZ_ASSERT(mRequestHeadersDone); - MOZ_ASSERT(mOpenGenerated); LOG3(("Http2Stream::ReadSegments %p 0x%X: Sending request data complete, " "mUpstreamState=%x\n",this, mStreamID, mUpstreamState)); if (mSentFin) { @@ -1234,7 +1233,7 @@ Http2Stream::OnReadSegment(const char *buf, if (mRequestHeadersDone && !mOpenGenerated) { if (!mSession->TryToActivate(this)) { LOG3(("Http2Stream::OnReadSegment %p cannot activate now. queued.\n", this)); - return NS_OK; + return *countRead ? NS_OK : NS_BASE_STREAM_WOULD_BLOCK; } if (NS_FAILED(rv = GenerateOpen())) { return rv; diff --git a/netwerk/protocol/http/SpdyStream31.cpp b/netwerk/protocol/http/SpdyStream31.cpp index 67a1db5d216..e1423847d7e 100644 --- a/netwerk/protocol/http/SpdyStream31.cpp +++ b/netwerk/protocol/http/SpdyStream31.cpp @@ -160,7 +160,7 @@ SpdyStream31::ReadSegments(nsAHttpSegmentReader *reader, // If the sending flow control window is open (!mBlockedOnRwin) then // continue sending the request - if (!mBlockedOnRwin && mUpstreamState != GENERATING_SYN_STREAM && + if (!mBlockedOnRwin && mSynFrameGenerated && !mTxInlineFrameUsed && NS_SUCCEEDED(rv) && (!*countRead)) { LOG3(("SpdyStream31::ReadSegments %p 0x%X: Sending request data complete, " "mUpstreamState=%x finondata=%d",this, mStreamID, @@ -1491,7 +1491,7 @@ SpdyStream31::OnReadSegment(const char *buf, if (mRequestHeadersDone && !mSynFrameGenerated) { if (!mSession->TryToActivate(this)) { LOG3(("SpdyStream31::OnReadSegment %p cannot activate now. queued.\n", this)); - return NS_OK; + return *countRead ? NS_OK : NS_BASE_STREAM_WOULD_BLOCK; } if (NS_FAILED(rv = GenerateSynFrame())) { return rv; From 74b794b1dcd3d19257e1f6d407012fccd6bf532a Mon Sep 17 00:00:00 2001 From: Patrick McManus Date: Mon, 26 Jan 2015 20:21:54 -0500 Subject: [PATCH 12/53] bug 1124717 - 2/4 update CI to node-http2 3.1.0 to fix flow control issue r=hurley a test for 1124717 ran into https://github.com/molnarg/node-http2/issues/89 which is fixed in this update --- testing/xpcshell/node-http2/HISTORY.md | 15 ++- testing/xpcshell/node-http2/README.md | 25 ++-- testing/xpcshell/node-http2/example/client.js | 9 +- testing/xpcshell/node-http2/example/server.js | 32 +++-- testing/xpcshell/node-http2/lib/http.js | 29 +++-- testing/xpcshell/node-http2/lib/index.js | 4 +- .../node-http2/lib/protocol/compressor.js | 8 +- .../xpcshell/node-http2/lib/protocol/flow.js | 9 +- .../node-http2/lib/protocol/framer.js | 28 ++--- .../xpcshell/node-http2/lib/protocol/index.js | 10 +- .../node-http2/lib/protocol/stream.js | 16 +-- testing/xpcshell/node-http2/package.json | 9 +- .../xpcshell/node-http2/test/compressor.js | 12 +- .../xpcshell/node-http2/test/connection.js | 2 +- testing/xpcshell/node-http2/test/flow.js | 2 +- testing/xpcshell/node-http2/test/http.js | 117 +++++++++++++++++- testing/xpcshell/node-http2/test/stream.js | 9 +- testing/xpcshell/node-http2/test/util.js | 2 +- 18 files changed, 239 insertions(+), 99 deletions(-) diff --git a/testing/xpcshell/node-http2/HISTORY.md b/testing/xpcshell/node-http2/HISTORY.md index 4c6057c9a2e..e8328227cf6 100644 --- a/testing/xpcshell/node-http2/HISTORY.md +++ b/testing/xpcshell/node-http2/HISTORY.md @@ -1,7 +1,20 @@ Version history =============== -### 3.0.0 (2014-08-XX) ### +### 3.1.0 (2014-12-11) ### + +* Upgrade to the latest draft: [draft-ietf-httpbis-http2-16] + * This involves some state transition changes that are technically incompatible with draft-14. If you need to be assured to interop on -14, continue using 3.0.1 + +[draft-ietf-httpbis-http2-16]: http://tools.ietf.org/html/draft-ietf-httpbis-http2-16 + +### 3.0.1 (2014-11-20) ### + +* Bugfix release. +* Fixed #81 and #87 +* Fixed a bug in flow control (without GitHub issue) + +### 3.0.0 (2014-08-25) ### * Re-join node-http2 and node-http2-protocol into one repository * API Changes diff --git a/testing/xpcshell/node-http2/README.md b/testing/xpcshell/node-http2/README.md index c59936f1c0c..4b02f01d606 100644 --- a/testing/xpcshell/node-http2/README.md +++ b/testing/xpcshell/node-http2/README.md @@ -1,7 +1,7 @@ node-http2 ========== -An HTTP/2 ([draft-ietf-httpbis-http2-14](http://tools.ietf.org/html/draft-ietf-httpbis-http2-14)) +An HTTP/2 ([draft-ietf-httpbis-http2-16](http://tools.ietf.org/html/draft-ietf-httpbis-http2-16)) client and server implementation for node.js. ![Travis CI status](https://travis-ci.org/molnarg/node-http2.svg?branch=master) @@ -116,12 +116,12 @@ point to understand the code. ### Test coverage ### To generate a code coverage report, run `npm test --coverage` (which runs very slowly, be patient). -Code coverage summary as of version 3.0.0: +Code coverage summary as of version 3.0.1: ``` -Statements : 91.85% ( 1747/1902 ) -Branches : 81.61% ( 688/843 ) -Functions : 90.95% ( 211/232 ) -Lines : 91.92% ( 1741/1894 ) +Statements : 92.09% ( 1759/1910 ) +Branches : 82.56% ( 696/843 ) +Functions : 91.38% ( 212/232 ) +Lines : 92.17% ( 1753/1902 ) ``` There's a hosted version of the detailed (line-by-line) coverage report @@ -151,12 +151,17 @@ $ HTTP2_LOG=info node ./example/client.js 'http://localhost:8080/server.js' >/de Contributors ------------ +The co-maintainer of the project is [Nick Hurley](https://github.com/todesschaf). + Code contributions are always welcome! People who contributed to node-http2 so far: -* Nick Hurley -* Mike Belshe -* Yoshihiro Iwanaga -* vsemogutor +* [Nick Hurley](https://github.com/todesschaf) +* [Mike Belshe](https://github.com/mbelshe) +* [Yoshihiro Iwanaga](https://github.com/iwanaga) +* [Igor Novikov](https://github.com/vsemogutor) +* [James Willcox](https://github.com/snorp) +* [David Björklund](https://github.com/kesla) +* [Patrick McManus](https://github.com/mcmanus) Special thanks to Google for financing the development of this module as part of their [Summer of Code program](https://developers.google.com/open-source/soc/) (project: [HTTP/2 prototype server diff --git a/testing/xpcshell/node-http2/example/client.js b/testing/xpcshell/node-http2/example/client.js index 89ad8a455c9..2687891b27e 100644 --- a/testing/xpcshell/node-http2/example/client.js +++ b/testing/xpcshell/node-http2/example/client.js @@ -2,18 +2,17 @@ var fs = require('fs'); var path = require('path'); var http2 = require('..'); +// Setting the global logger (optional) http2.globalAgent = new http2.Agent({ log: require('../test/util').createLogger('client') }); +// We use self signed certs in the example code so we ignore cert errors process.env.NODE_TLS_REJECT_UNAUTHORIZED = "0"; // Sending the request -// It would be `var request = http2.get(process.argv.pop());` if we wouldn't care about plain mode -var options = require('url').parse(process.argv.pop()); -options.plain = Boolean(process.env.HTTP2_PLAIN); -var request = http2.request(options); -request.end(); +var url = process.argv.pop(); +var request = process.env.HTTP2_PLAIN ? http2.raw.get(url) : http2.get(url); // Receiving the response request.on('response', function(response) { diff --git a/testing/xpcshell/node-http2/example/server.js b/testing/xpcshell/node-http2/example/server.js index 73cdd6b45e1..b9568235597 100644 --- a/testing/xpcshell/node-http2/example/server.js +++ b/testing/xpcshell/node-http2/example/server.js @@ -2,22 +2,12 @@ var fs = require('fs'); var path = require('path'); var http2 = require('..'); -var options = process.env.HTTP2_PLAIN ? { - plain: true -} : { - key: fs.readFileSync(path.join(__dirname, '/localhost.key')), - cert: fs.readFileSync(path.join(__dirname, '/localhost.crt')) -}; - -// Passing bunyan logger (optional) -options.log = require('../test/util').createLogger('server'); - // We cache one file to be able to do simple performance tests without waiting for the disk var cachedFile = fs.readFileSync(path.join(__dirname, './server.js')); var cachedUrl = '/server.js'; -// Creating the server -var server = http2.createServer(options, function(request, response) { +// The callback to handle requests +function onRequest(request, response) { var filename = path.join(__dirname, request.url); // Serving server.js from cache. Useful for microbenchmarks. @@ -44,6 +34,22 @@ var server = http2.createServer(options, function(request, response) { response.writeHead('404'); response.end(); } -}); +} +// Creating a bunyan logger (optional) +var log = require('../test/util').createLogger('server'); + +// Creating the server in plain or TLS mode (TLS mode is the default) +var server; +if (process.env.HTTP2_PLAIN) { + server = http2.raw.createServer({ + log: log + }, onRequest); +} else { + server = http2.createServer({ + log: log, + key: fs.readFileSync(path.join(__dirname, '/localhost.key')), + cert: fs.readFileSync(path.join(__dirname, '/localhost.crt')) + }, onRequest); +} server.listen(process.env.HTTP2_PORT || 8080); diff --git a/testing/xpcshell/node-http2/lib/http.js b/testing/xpcshell/node-http2/lib/http.js index 0376f3dc1a1..34739244c69 100644 --- a/testing/xpcshell/node-http2/lib/http.js +++ b/testing/xpcshell/node-http2/lib/http.js @@ -121,7 +121,7 @@ // // [1]: http://nodejs.org/api/https.html // [2]: http://nodejs.org/api/http.html -// [3]: http://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-8.1.3.2 +// [3]: http://tools.ietf.org/html/draft-ietf-httpbis-http2-16#section-8.1.2.4 // [expect-continue]: https://github.com/http2/http2-spec/issues/18 // [connect]: https://github.com/http2/http2-spec/issues/230 @@ -246,7 +246,7 @@ function IncomingMessage(stream) { } IncomingMessage.prototype = Object.create(PassThrough.prototype, { constructor: { value: IncomingMessage } }); -// [Request Header Fields](http://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-8.1.3.1) +// [Request Header Fields](http://tools.ietf.org/html/draft-ietf-httpbis-http2-16#section-8.1.2.3) // * `headers` argument: HTTP/2.0 request and response header fields carry information as a series // of key-value pairs. This includes the target URI for the request, the status code for the // response, as well as HTTP header fields. @@ -602,7 +602,7 @@ function IncomingRequest(stream) { } IncomingRequest.prototype = Object.create(IncomingMessage.prototype, { constructor: { value: IncomingRequest } }); -// [Request Header Fields](http://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-8.1.3.1) +// [Request Header Fields](http://tools.ietf.org/html/draft-ietf-httpbis-http2-16#section-8.1.2.3) // * `headers` argument: HTTP/2.0 request and response header fields carry information as a series // of key-value pairs. This includes the target URI for the request, the status code for the // response, as well as HTTP header fields. @@ -753,7 +753,8 @@ function requestRaw(options, callback) { if (typeof options === "string") { options = url.parse(options); } - if ((options.protocol && options.protocol !== "http:") || !options.plain) { + options.plain = true; + if (options.protocol && options.protocol !== "http:") { throw new Error('This interface only supports http-schemed URLs'); } return (options.agent || exports.globalAgent).request(options, callback); @@ -763,7 +764,8 @@ function requestTLS(options, callback) { if (typeof options === "string") { options = url.parse(options); } - if ((options.protocol && options.protocol !== "https:") || options.plain) { + options.plain = false; + if (options.protocol && options.protocol !== "https:") { throw new Error('This interface only supports https-schemed URLs'); } return (options.agent || exports.globalAgent).request(options, callback); @@ -773,7 +775,8 @@ function getRaw(options, callback) { if (typeof options === "string") { options = url.parse(options); } - if ((options.protocol && options.protocol !== "http:") || !options.plain) { + options.plain = true; + if (options.protocol && options.protocol !== "http:") { throw new Error('This interface only supports http-schemed URLs'); } return (options.agent || exports.globalAgent).get(options, callback); @@ -783,7 +786,8 @@ function getTLS(options, callback) { if (typeof options === "string") { options = url.parse(options); } - if ((options.protocol && options.protocol !== "https:") || options.plain) { + options.plain = false; + if (options.protocol && options.protocol !== "https:") { throw new Error('This interface only supports https-schemed URLs'); } return (options.agent || exports.globalAgent).get(options, callback); @@ -863,7 +867,7 @@ Agent.prototype.request = function request(options, callback) { request._start(endpoint.createStream(), options); } - // * HTTP/2 over TLS negotiated using NPN or ALPN + // * HTTP/2 over TLS negotiated using NPN or ALPN, or fallback to HTTPS1 else { var started = false; options.ALPNProtocols = supportedProtocols; @@ -876,7 +880,7 @@ Agent.prototype.request = function request(options, callback) { httpsRequest.on('socket', function(socket) { var negotiatedProtocol = socket.alpnProtocol || socket.npnProtocol; if (negotiatedProtocol != null) { // null in >=0.11.0, undefined in <0.11.0 - negotiated() + negotiated(); } else { socket.on('secureConnect', negotiated); } @@ -894,11 +898,12 @@ Agent.prototype.request = function request(options, callback) { endpoint.pipe(endpoint.socket).pipe(endpoint); } if (started) { + // ** In the meantime, an other connection was made to the same host... if (endpoint) { + // *** and it turned out to be HTTP2 and the request was multiplexed on that one, so we should close this one endpoint.close(); - } else { - httpsRequest.abort(); } + // *** otherwise, the fallback to HTTPS1 is already done. } else { if (endpoint) { self._log.info({ e: endpoint, server: options.host + ':' + options.port }, @@ -1079,7 +1084,7 @@ function IncomingResponse(stream) { } IncomingResponse.prototype = Object.create(IncomingMessage.prototype, { constructor: { value: IncomingResponse } }); -// [Response Header Fields](http://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-8.1.3.2) +// [Response Header Fields](http://tools.ietf.org/html/draft-ietf-httpbis-http2-16#section-8.1.2.4) // * `headers` argument: HTTP/2.0 request and response header fields carry information as a series // of key-value pairs. This includes the target URI for the request, the status code for the // response, as well as HTTP header fields. diff --git a/testing/xpcshell/node-http2/lib/index.js b/testing/xpcshell/node-http2/lib/index.js index 08a97ed50a7..60981c2cc1a 100644 --- a/testing/xpcshell/node-http2/lib/index.js +++ b/testing/xpcshell/node-http2/lib/index.js @@ -1,4 +1,4 @@ -// [node-http2][homepage] is an [HTTP/2 (draft 14)][http2] implementation for [node.js][node]. +// [node-http2][homepage] is an [HTTP/2 (draft 16)][http2] implementation for [node.js][node]. // // The core of the protocol is implemented in the protocol sub-directory. This directory provides // two important features on top of the protocol: @@ -10,7 +10,7 @@ // (which is in turn very similar to the [HTTP module API][node-http]). // // [homepage]: https://github.com/molnarg/node-http2 -// [http2]: http://tools.ietf.org/html/draft-ietf-httpbis-http2-14 +// [http2]: http://tools.ietf.org/html/draft-ietf-httpbis-http2-16 // [node]: http://nodejs.org/ // [node-https]: http://nodejs.org/api/https.html // [node-http]: http://nodejs.org/api/http.html diff --git a/testing/xpcshell/node-http2/lib/protocol/compressor.js b/testing/xpcshell/node-http2/lib/protocol/compressor.js index f11c19d25d0..f7c60dedd73 100644 --- a/testing/xpcshell/node-http2/lib/protocol/compressor.js +++ b/testing/xpcshell/node-http2/lib/protocol/compressor.js @@ -1123,10 +1123,10 @@ Compressor.prototype.compress = function compress(headers) { // separate header fields, each with one or more cookie-pairs. if (name == 'cookie') { if (!(value instanceof Array)) { - value = [value] + value = [value]; } value = Array.prototype.concat.apply([], value.map(function(cookie) { - return String(cookie).split(';').map(trim) + return String(cookie).split(';').map(trim); })); } @@ -1256,7 +1256,7 @@ Decompressor.prototype.decompress = function decompress(block) { // into a single octet string using the two octet delimiter of 0x3B, 0x20 (the ASCII // string "; "). if (('cookie' in headers) && (headers['cookie'] instanceof Array)) { - headers['cookie'] = headers['cookie'].join('; ') + headers['cookie'] = headers['cookie'].join('; '); } return headers; @@ -1340,5 +1340,5 @@ function cut(buffer, size) { } function trim(string) { - return string.trim() + return string.trim(); } diff --git a/testing/xpcshell/node-http2/lib/protocol/flow.js b/testing/xpcshell/node-http2/lib/protocol/flow.js index 384b09af92a..ec4cdc47583 100644 --- a/testing/xpcshell/node-http2/lib/protocol/flow.js +++ b/testing/xpcshell/node-http2/lib/protocol/flow.js @@ -19,7 +19,7 @@ exports.Flow = Flow; // * **setInitialWindow(size)**: the initial flow control window size can be changed *any time* // ([as described in the standard][1]) using this method // -// [1]: http://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-6.9.2 +// [1]: http://tools.ietf.org/html/draft-ietf-httpbis-http2-16#section-6.9.2 // API for child classes // --------------------- @@ -81,7 +81,9 @@ Flow.prototype._receive = function _receive(frame, callback) { // incoming frame is a WINDOW_UPDATE. // [1]: http://nodejs.org/api/stream.html#stream_writable_write_chunk_encoding_callback_1 Flow.prototype._write = function _write(frame, encoding, callback) { - if (frame.flags.END_STREAM || (frame.type === 'RST_STREAM')) { + var sentToUs = (this._flowControlId === undefined) || (frame.stream === this._flowControlId); + + if (sentToUs && (frame.flags.END_STREAM || (frame.type === 'RST_STREAM'))) { this._ended = true; } @@ -99,8 +101,7 @@ Flow.prototype._write = function _write(frame, encoding, callback) { this._receive(frame, callback); } - if ((frame.type === 'WINDOW_UPDATE') && - ((this._flowControlId === undefined) || (frame.stream === this._flowControlId))) { + if (sentToUs && (frame.type === 'WINDOW_UPDATE')) { this._updateWindow(frame); } }; diff --git a/testing/xpcshell/node-http2/lib/protocol/framer.js b/testing/xpcshell/node-http2/lib/protocol/framer.js index 8e4e162a236..086895e0d3d 100644 --- a/testing/xpcshell/node-http2/lib/protocol/framer.js +++ b/testing/xpcshell/node-http2/lib/protocol/framer.js @@ -146,7 +146,7 @@ Deserializer.prototype._transform = function _transform(chunk, encoding, done) { done(); }; -// [Frame Header](http://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-4.1) +// [Frame Header](http://tools.ietf.org/html/draft-ietf-httpbis-http2-16#section-4.1) // -------------------------------------------------------------- // // HTTP/2.0 frames share a common base format consisting of a 9-byte header followed by 0 to 2^24 - 1 @@ -269,7 +269,7 @@ Deserializer.commonHeader = function readCommonHeader(buffer, frame) { // * `typeSpecificAttributes`: a register of frame specific frame object attributes (used by // logging code and also serves as documentation for frame objects) -// [DATA Frames](http://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-6.1) +// [DATA Frames](http://tools.ietf.org/html/draft-ietf-httpbis-http2-16#section-6.1) // ------------------------------------------------------------ // // DATA frames (type=0x0) convey arbitrary, variable-length sequences of octets associated with a @@ -308,7 +308,7 @@ Deserializer.DATA = function readData(buffer, frame) { } }; -// [HEADERS](http://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-6.2) +// [HEADERS](http://tools.ietf.org/html/draft-ietf-httpbis-http2-16#section-6.2) // -------------------------------------------------------------- // // The HEADERS frame (type=0x1) allows the sender to create a stream. @@ -390,7 +390,7 @@ Deserializer.HEADERS = function readHeadersPriority(buffer, frame) { } }; -// [PRIORITY](http://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-6.3) +// [PRIORITY](http://tools.ietf.org/html/draft-ietf-httpbis-http2-16#section-6.3) // ------------------------------------------------------- // // The PRIORITY frame (type=0x2) specifies the sender-advised priority of a stream. @@ -435,7 +435,7 @@ Deserializer.PRIORITY = function readPriority(buffer, frame) { frame.priorityWeight = buffer.readUInt8(4); }; -// [RST_STREAM](http://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-6.4) +// [RST_STREAM](http://tools.ietf.org/html/draft-ietf-httpbis-http2-16#section-6.4) // ----------------------------------------------------------- // // The RST_STREAM frame (type=0x3) allows for abnormal termination of a stream. @@ -473,7 +473,7 @@ Deserializer.RST_STREAM = function readRstStream(buffer, frame) { } }; -// [SETTINGS](http://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-6.5) +// [SETTINGS](http://tools.ietf.org/html/draft-ietf-httpbis-http2-16#section-6.5) // ------------------------------------------------------- // // The SETTINGS frame (type=0x4) conveys configuration parameters that affect how endpoints @@ -580,7 +580,7 @@ definedSettings[4] = { name: 'SETTINGS_INITIAL_WINDOW_SIZE', flag: false }; // indicates the maximum size of a frame the receiver will allow. definedSettings[5] = { name: 'SETTINGS_MAX_FRAME_SIZE', flag: false }; -// [PUSH_PROMISE](http://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-6.6) +// [PUSH_PROMISE](http://tools.ietf.org/html/draft-ietf-httpbis-http2-16#section-6.6) // --------------------------------------------------------------- // // The PUSH_PROMISE frame (type=0x5) is used to notify the peer endpoint in advance of streams the @@ -641,7 +641,7 @@ Deserializer.PUSH_PROMISE = function readPushPromise(buffer, frame) { } }; -// [PING](http://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-6.7) +// [PING](http://tools.ietf.org/html/draft-ietf-httpbis-http2-16#section-6.7) // ----------------------------------------------- // // The PING frame (type=0x6) is a mechanism for measuring a minimal round-trip time from the @@ -671,7 +671,7 @@ Deserializer.PING = function readPing(buffer, frame) { frame.data = buffer; }; -// [GOAWAY](http://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-6.8) +// [GOAWAY](http://tools.ietf.org/html/draft-ietf-httpbis-http2-16#section-6.8) // --------------------------------------------------- // // The GOAWAY frame (type=0x7) informs the remote peer to stop creating streams on this connection. @@ -722,7 +722,7 @@ Deserializer.GOAWAY = function readGoaway(buffer, frame) { } }; -// [WINDOW_UPDATE](http://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-6.9) +// [WINDOW_UPDATE](http://tools.ietf.org/html/draft-ietf-httpbis-http2-16#section-6.9) // ----------------------------------------------------------------- // // The WINDOW_UPDATE frame (type=0x8) is used to implement flow control. @@ -760,7 +760,7 @@ Deserializer.WINDOW_UPDATE = function readWindowUpdate(buffer, frame) { } }; -// [CONTINUATION](http://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-6.10) +// [CONTINUATION](http://tools.ietf.org/html/draft-ietf-httpbis-http2-16#section-6.10) // ------------------------------------------------------------ // // The CONTINUATION frame (type=0x9) is used to continue a sequence of header block fragments. @@ -785,7 +785,7 @@ Deserializer.CONTINUATION = function readContinuation(buffer, frame) { frame.data = buffer; }; -// [ALTSVC](http://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-6.11) +// [ALTSVC](http://tools.ietf.org/html/draft-ietf-httpbis-alt-svc-04#section-4) // ------------------------------------------------------------ // // The ALTSVC frame (type=0xA) advertises the availability of an alternative service to the client. @@ -874,7 +874,7 @@ Deserializer.ALTSVC = function readAltSvc(buffer, frame) { frame.origin = buffer.toString('ascii', 9 + pidLength + hostLength); }; -// [BLOCKED](http://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-6.12) +// BLOCKED // ------------------------------------------------------------ // // The BLOCKED frame (type=0xB) indicates that the sender is unable to send data @@ -894,7 +894,7 @@ Serializer.BLOCKED = function writeBlocked(frame, buffers) { Deserializer.BLOCKED = function readBlocked(buffer, frame) { }; -// [Error Codes](http://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-7) +// [Error Codes](http://tools.ietf.org/html/draft-ietf-httpbis-http2-16#section-7) // ------------------------------------------------------------ var errorCodes = [ diff --git a/testing/xpcshell/node-http2/lib/protocol/index.js b/testing/xpcshell/node-http2/lib/protocol/index.js index c655f4b251b..166c1dbee0e 100644 --- a/testing/xpcshell/node-http2/lib/protocol/index.js +++ b/testing/xpcshell/node-http2/lib/protocol/index.js @@ -1,4 +1,4 @@ -// [node-http2-protocol][homepage] is an implementation of the [HTTP/2 (draft 14)][http2] +// [node-http2-protocol][homepage] is an implementation of the [HTTP/2 (draft 16)][http2] // framing layer for [node.js][node]. // // The main building blocks are [node.js streams][node-stream] that are connected through pipes. @@ -28,10 +28,10 @@ // between the binary and the JavaScript object representation of HTTP/2 frames // // [homepage]: https://github.com/molnarg/node-http2 -// [http2]: http://tools.ietf.org/html/draft-ietf-httpbis-http2-14 -// [http2-connheader]: http://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-3.5 -// [http2-stream]: http://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-5 -// [http2-streamstate]: http://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-5.1 +// [http2]: http://tools.ietf.org/html/draft-ietf-httpbis-http2-16 +// [http2-connheader]: http://tools.ietf.org/html/draft-ietf-httpbis-http2-16#section-3.5 +// [http2-stream]: http://tools.ietf.org/html/draft-ietf-httpbis-http2-16#section-5 +// [http2-streamstate]: http://tools.ietf.org/html/draft-ietf-httpbis-http2-16#section-5.1 // [node]: http://nodejs.org/ // [node-stream]: http://nodejs.org/api/stream.html // [node-https]: http://nodejs.org/api/https.html diff --git a/testing/xpcshell/node-http2/lib/protocol/stream.js b/testing/xpcshell/node-http2/lib/protocol/stream.js index 50abbe6fc43..c42de87a84e 100644 --- a/testing/xpcshell/node-http2/lib/protocol/stream.js +++ b/testing/xpcshell/node-http2/lib/protocol/stream.js @@ -352,7 +352,7 @@ Stream.prototype._finishing = function _finishing() { } }; -// [Stream States](http://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-5.1) +// [Stream States](http://tools.ietf.org/html/draft-ietf-httpbis-http2-16#section-5.1) // ---------------- // // +--------+ @@ -464,7 +464,7 @@ Stream.prototype._transition = function transition(sending, frame) { this._setState('HALF_CLOSED_REMOTE'); } else if (RST_STREAM) { this._setState('CLOSED'); - } else if (receiving && PRIORITY) { + } else if (PRIORITY) { /* No state change */ } else { connectionError = 'PROTOCOL_ERROR'; @@ -483,7 +483,7 @@ Stream.prototype._transition = function transition(sending, frame) { this._setState('CLOSED'); } else if (receiving && HEADERS) { this._setState('HALF_CLOSED_LOCAL'); - } else if (BLOCKED || (sending && PRIORITY)) { + } else if (BLOCKED || PRIORITY) { /* No state change */ } else { connectionError = 'PROTOCOL_ERROR'; @@ -518,7 +518,7 @@ Stream.prototype._transition = function transition(sending, frame) { case 'HALF_CLOSED_LOCAL': if (RST_STREAM || (receiving && frame.flags.END_STREAM)) { this._setState('CLOSED'); - } else if (BLOCKED || ALTSVC ||receiving || (sending && (PRIORITY || WINDOW_UPDATE))) { + } else if (BLOCKED || ALTSVC || receiving || PRIORITY || (sending && WINDOW_UPDATE)) { /* No state change */ } else { connectionError = 'PROTOCOL_ERROR'; @@ -538,7 +538,7 @@ Stream.prototype._transition = function transition(sending, frame) { case 'HALF_CLOSED_REMOTE': if (RST_STREAM || (sending && frame.flags.END_STREAM)) { this._setState('CLOSED'); - } else if (BLOCKED || ALTSVC ||sending || (receiving && (WINDOW_UPDATE || PRIORITY))) { + } else if (BLOCKED || ALTSVC || sending || PRIORITY || (receiving && WINDOW_UPDATE)) { /* No state change */ } else { connectionError = 'PROTOCOL_ERROR'; @@ -566,9 +566,9 @@ Stream.prototype._transition = function transition(sending, frame) { // causes a stream to become "reserved". If promised streams are not desired, a RST_STREAM // can be used to close any of those streams. case 'CLOSED': - if ((sending && RST_STREAM) || + if (PRIORITY || (sending && RST_STREAM) || (receiving && this._closedByUs && - (this._closedWithRst || WINDOW_UPDATE || PRIORITY || RST_STREAM || ALTSVC))) { + (this._closedWithRst || WINDOW_UPDATE || RST_STREAM || ALTSVC))) { /* No state change */ } else { streamError = 'STREAM_CLOSED'; @@ -637,7 +637,7 @@ Stream.prototype._transition = function transition(sending, frame) { this.emit('connectionError', connectionError); } else { this.reset(streamError); - this.emit('error', streamError) + this.emit('error', streamError); } } } diff --git a/testing/xpcshell/node-http2/package.json b/testing/xpcshell/node-http2/package.json index cfc9d042a63..3a1bad4686a 100644 --- a/testing/xpcshell/node-http2/package.json +++ b/testing/xpcshell/node-http2/package.json @@ -1,6 +1,6 @@ { "name": "http2", - "version": "3.0.0", + "version": "3.1.0", "description": "An HTTP/2 client and server implementation", "main": "lib/index.js", "engines" : { @@ -14,7 +14,7 @@ "bunyan": "*" }, "scripts": { - "test": "istanbul test _mocha -- --reporter spec --slow 200", + "test": "istanbul test _mocha -- --reporter spec --slow 500 --timeout 15000", "doc": "docco lib/* --output doc --layout parallel --css doc/docco.css" }, "repository": { @@ -36,7 +36,10 @@ "Nick Hurley", "Mike Belshe", "Yoshihiro Iwanaga", - "vsemogutor" + "Igor Novikov", + "James Willcox", + "David Björklund", + "Patrick McManus" ], "license": "MIT", "readmeFilename": "README.md" diff --git a/testing/xpcshell/node-http2/test/compressor.js b/testing/xpcshell/node-http2/test/compressor.js index a5e9b53bf34..f86baf5db92 100644 --- a/testing/xpcshell/node-http2/test/compressor.js +++ b/testing/xpcshell/node-http2/test/compressor.js @@ -406,21 +406,21 @@ describe('compressor.js', function() { expect(table.encode(new Buffer(decoded)).toString('hex')).to.equal(encoded); } }); - }) + }); describe('method decode(buffer)', function() { it('should return the Huffman decoded version of the input buffer', function() { var table = HuffmanTable.huffmanTable; for (var decoded in test_huffman_request) { var encoded = test_huffman_request[decoded]; - expect(table.decode(new Buffer(encoded, 'hex')).toString()).to.equal(decoded) + expect(table.decode(new Buffer(encoded, 'hex')).toString()).to.equal(decoded); } table = HuffmanTable.huffmanTable; for (decoded in test_huffman_response) { encoded = test_huffman_response[decoded]; - expect(table.decode(new Buffer(encoded, 'hex')).toString()).to.equal(decoded) + expect(table.decode(new Buffer(encoded, 'hex')).toString()).to.equal(decoded); } }); - }) + }); }); describe('HeaderSetCompressor', function() { @@ -569,7 +569,7 @@ describe('compressor.js', function() { var result = table.decode(table.encode(buffer)); expect(result).to.deep.equal(buffer); } - }) - }) + }); + }); }); }); diff --git a/testing/xpcshell/node-http2/test/connection.js b/testing/xpcshell/node-http2/test/connection.js index cc544162cdd..2c68857f7fe 100644 --- a/testing/xpcshell/node-http2/test/connection.js +++ b/testing/xpcshell/node-http2/test/connection.js @@ -131,7 +131,7 @@ describe('connection.js', function() { client_stream.on('data', function(data) { expect(data).to.deep.equal(response_data); done(); - }) + }); }); }); describe('server push', function() { diff --git a/testing/xpcshell/node-http2/test/flow.js b/testing/xpcshell/node-http2/test/flow.js index cdf30a92415..a77507a796c 100644 --- a/testing/xpcshell/node-http2/test/flow.js +++ b/testing/xpcshell/node-http2/test/flow.js @@ -254,7 +254,7 @@ describe('flow.js', function() { // Start piping flow1.pipe(flow2).pipe(flow1); - }) + }); }); }); }); diff --git a/testing/xpcshell/node-http2/test/http.js b/testing/xpcshell/node-http2/test/http.js index f1e3b238233..42bc68e3f88 100644 --- a/testing/xpcshell/node-http2/test/http.js +++ b/testing/xpcshell/node-http2/test/http.js @@ -124,7 +124,7 @@ describe('http.js', function() { var response = new http2.OutgoingResponse(stream); response.writeHead(200); - response.writeHead(404) + response.writeHead(404); }); }); describe('test scenario', function() { @@ -149,6 +149,66 @@ describe('http.js', function() { }); }); }); + describe('2 simple request in parallel', function() { + it('should work as expected', function(originalDone) { + var path = '/x'; + var message = 'Hello world'; + done = util.callNTimes(2, function() { + server.close(); + originalDone(); + }); + + var server = http2.createServer(options, function(request, response) { + expect(request.url).to.equal(path); + response.end(message); + }); + + server.listen(1234, function() { + http2.get('https://localhost:1234' + path, function(response) { + response.on('data', function(data) { + expect(data.toString()).to.equal(message); + done(); + }); + }); + http2.get('https://localhost:1234' + path, function(response) { + response.on('data', function(data) { + expect(data.toString()).to.equal(message); + done(); + }); + }); + }); + }); + }); + describe('100 simple request in a series', function() { + it('should work as expected', function(done) { + var path = '/x'; + var message = 'Hello world'; + + var server = http2.createServer(options, function(request, response) { + expect(request.url).to.equal(path); + response.end(message); + }); + + var n = 100; + server.listen(1242, function() { + doRequest(); + function doRequest() { + http2.get('https://localhost:1242' + path, function(response) { + response.on('data', function(data) { + expect(data.toString()).to.equal(message); + if (n) { + n -= 1; + doRequest(); + } else { + server.close(); + done(); + } + }); + }); + } + }); + }); + }); describe('request with payload', function() { it('should work as expected', function(done) { var path = '/x'; @@ -240,7 +300,6 @@ describe('http.js', function() { var message = 'Hello world'; var server = http2.raw.createServer({ - plain: true, log: util.serverLog }, function(request, response) { expect(request.url).to.equal(path); @@ -264,6 +323,30 @@ describe('http.js', function() { }); }); }); + describe('get over plain TCP', function() { + it('should work as expected', function(done) { + var path = '/x'; + var message = 'Hello world'; + + var server = http2.raw.createServer({ + log: util.serverLog + }, function(request, response) { + expect(request.url).to.equal(path); + response.end(message); + }); + + server.listen(1237, function() { + var request = http2.raw.get('http://localhost:1237/x', function(response) { + response.on('data', function(data) { + expect(data.toString()).to.equal(message); + server.close(); + done(); + }); + }); + request.end(); + }); + }); + }); describe('request to an HTTPS/1 server', function() { it('should fall back to HTTPS/1 successfully', function(done) { var path = '/x'; @@ -284,6 +367,36 @@ describe('http.js', function() { }); }); }); + describe('2 parallel request to an HTTPS/1 server', function() { + it('should fall back to HTTPS/1 successfully', function(originalDone) { + var path = '/x'; + var message = 'Hello world'; + done = util.callNTimes(2, function() { + server.close(); + originalDone(); + }); + + var server = https.createServer(options, function(request, response) { + expect(request.url).to.equal(path); + response.end(message); + }); + + server.listen(6789, function() { + http2.get('https://localhost:6789' + path, function(response) { + response.on('data', function(data) { + expect(data.toString()).to.equal(message); + done(); + }); + }); + http2.get('https://localhost:6789' + path, function(response) { + response.on('data', function(data) { + expect(data.toString()).to.equal(message); + done(); + }); + }); + }); + }); + }); describe('HTTPS/1 request to a HTTP/2 server', function() { it('should fall back to HTTPS/1 successfully', function(done) { var path = '/x'; diff --git a/testing/xpcshell/node-http2/test/stream.js b/testing/xpcshell/node-http2/test/stream.js index d37be1b78bb..87af55fc29d 100644 --- a/testing/xpcshell/node-http2/test/stream.js +++ b/testing/xpcshell/node-http2/test/stream.js @@ -112,7 +112,6 @@ var example_frames = [ var invalid_incoming_frames = { IDLE: [ { type: 'DATA', flags: {}, data: new Buffer(5) }, - { type: 'PRIORITY', flags: {}, priority: 1 }, { type: 'WINDOW_UPDATE', flags: {}, settings: {} }, { type: 'PUSH_PROMISE', flags: {}, headers: {} }, { type: 'RST_STREAM', flags: {}, error: 'CANCEL' } @@ -125,7 +124,6 @@ var invalid_incoming_frames = { ], RESERVED_REMOTE: [ { type: 'DATA', flags: {}, data: new Buffer(5) }, - { type: 'PRIORITY', flags: {}, priority: 1 }, { type: 'PUSH_PROMISE', flags: {}, headers: {} }, { type: 'WINDOW_UPDATE', flags: {}, settings: {} } ], @@ -143,13 +141,11 @@ var invalid_incoming_frames = { var invalid_outgoing_frames = { IDLE: [ { type: 'DATA', flags: {}, data: new Buffer(5) }, - { type: 'PRIORITY', flags: {}, priority: 1 }, { type: 'WINDOW_UPDATE', flags: {}, settings: {} }, { type: 'PUSH_PROMISE', flags: {}, headers: {} } ], RESERVED_LOCAL: [ { type: 'DATA', flags: {}, data: new Buffer(5) }, - { type: 'PRIORITY', flags: {}, priority: 1 }, { type: 'PUSH_PROMISE', flags: {}, headers: {} }, { type: 'WINDOW_UPDATE', flags: {}, settings: {} } ], @@ -169,7 +165,6 @@ var invalid_outgoing_frames = { HALF_CLOSED_REMOTE: [ ], CLOSED: [ - { type: 'PRIORITY', flags: {}, priority: 1 }, { type: 'WINDOW_UPDATE', flags: {}, settings: {} }, { type: 'HEADERS', flags: {}, headers: {}, priority: undefined }, { type: 'DATA', flags: {}, data: new Buffer(5) }, @@ -188,7 +183,7 @@ describe('stream.js', function() { stream.state = state; stream.once('connectionError', function() { connectionErrorHappened = true; }); stream._transition(false, invalid_frame); - expect(connectionErrorHappened) + expect(connectionErrorHappened); }); }); @@ -197,7 +192,7 @@ describe('stream.js', function() { stream.headers({}); stream.end(); stream.upstream.write({ type: 'HEADERS', headers:{}, flags: { END_STREAM: true }, count_change: util.noop }); - example_frames.forEach(function(invalid_frame) { + example_frames.slice(1).forEach(function(invalid_frame) { invalid_frame.count_change = util.noop; expect(stream._transition.bind(stream, false, invalid_frame)).to.throw('Uncaught, unspecified "error" event.'); }); diff --git a/testing/xpcshell/node-http2/test/util.js b/testing/xpcshell/node-http2/test/util.js index aea8fe70722..52c6a1be369 100644 --- a/testing/xpcshell/node-http2/test/util.js +++ b/testing/xpcshell/node-http2/test/util.js @@ -86,4 +86,4 @@ exports.shuffleBuffers = function shuffleBuffers(buffers) { } return output; -} +}; From bab538c3aa25e8f2ab48873c09cb23e863da6d13 Mon Sep 17 00:00:00 2001 From: Patrick McManus Date: Mon, 26 Jan 2015 16:06:44 -0500 Subject: [PATCH 13/53] bug 1124717 - 3/4 make h2/spdy default peer max concurrent setting configurable r=hurley --- modules/libpref/init/all.js | 1 + netwerk/protocol/http/ASpdySession.h | 2 ++ netwerk/protocol/http/Http2Session.cpp | 3 +-- netwerk/protocol/http/Http2Session.h | 1 - netwerk/protocol/http/SpdySession31.cpp | 3 +-- netwerk/protocol/http/SpdySession31.h | 1 - netwerk/protocol/http/nsHttpHandler.cpp | 9 +++++++++ netwerk/protocol/http/nsHttpHandler.h | 2 ++ 8 files changed, 16 insertions(+), 6 deletions(-) diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index a588e1b8240..dd19c9fe711 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -1319,6 +1319,7 @@ pref("network.http.spdy.ping-timeout", 8); pref("network.http.spdy.send-buffer-size", 131072); pref("network.http.spdy.allow-push", true); pref("network.http.spdy.push-allowance", 131072); +pref("network.http.spdy.default-concurrent", 100); // alt-svc allows separation of transport routing from // the origin host without using a proxy. diff --git a/netwerk/protocol/http/ASpdySession.h b/netwerk/protocol/http/ASpdySession.h index ad504d0ac79..d680b0b9309 100644 --- a/netwerk/protocol/http/ASpdySession.h +++ b/netwerk/protocol/http/ASpdySession.h @@ -50,6 +50,8 @@ public: // scenarios. const static uint32_t kInitialRwin = 256 * 1024 * 1024; + const static uint32_t kDefaultMaxConcurrent = 100; + // soft errors are errors that terminate a stream without terminating the // connection. In general non-network errors are stream errors as well // as network specific items like cancels. diff --git a/netwerk/protocol/http/Http2Session.cpp b/netwerk/protocol/http/Http2Session.cpp index a4c01bb003b..cc599cd2e4a 100644 --- a/netwerk/protocol/http/Http2Session.cpp +++ b/netwerk/protocol/http/Http2Session.cpp @@ -89,7 +89,6 @@ Http2Session::Http2Session(nsISocketTransport *aSocketTransport, uint32_t versio , mGoAwayReason(NO_HTTP_ERROR) , mGoAwayID(0) , mOutgoingGoAwayID(0) - , mMaxConcurrent(kDefaultMaxConcurrent) , mConcurrent(0) , mServerPushedResources(0) , mServerInitialStreamWindow(kDefaultRwin) @@ -119,7 +118,7 @@ Http2Session::Http2Session(nsISocketTransport *aSocketTransport, uint32_t versio mDecompressor.SetCompressor(&mCompressor); mPushAllowance = gHttpHandler->SpdyPushAllowance(); - + mMaxConcurrent = gHttpHandler->DefaultSpdyConcurrent(); mSendingChunkSize = gHttpHandler->SpdySendingChunkSize(); SendHello(); diff --git a/netwerk/protocol/http/Http2Session.h b/netwerk/protocol/http/Http2Session.h index 655f0abe0a8..af7eeb5b616 100644 --- a/netwerk/protocol/http/Http2Session.h +++ b/netwerk/protocol/http/Http2Session.h @@ -135,7 +135,6 @@ public: const static uint32_t kQueueTailRoom = 4096; const static uint32_t kQueueReserved = 1024; - const static uint32_t kDefaultMaxConcurrent = 100; const static uint32_t kMaxStreamID = 0x7800000; // This is a sentinel for a deleted stream. It is not a valid diff --git a/netwerk/protocol/http/SpdySession31.cpp b/netwerk/protocol/http/SpdySession31.cpp index 60aaf60036b..3912337174c 100644 --- a/netwerk/protocol/http/SpdySession31.cpp +++ b/netwerk/protocol/http/SpdySession31.cpp @@ -63,7 +63,6 @@ SpdySession31::SpdySession31(nsISocketTransport *aSocketTransport) , mCleanShutdown(false) , mDataPending(false) , mGoAwayID(0) - , mMaxConcurrent(kDefaultMaxConcurrent) , mConcurrent(0) , mServerPushedResources(0) , mServerInitialStreamWindow(kDefaultRwin) @@ -89,7 +88,7 @@ SpdySession31::SpdySession31(nsISocketTransport *aSocketTransport) zlibInit(); mPushAllowance = gHttpHandler->SpdyPushAllowance(); - + mMaxConcurrent = gHttpHandler->DefaultSpdyConcurrent(); mSendingChunkSize = gHttpHandler->SpdySendingChunkSize(); GenerateSettings(); diff --git a/netwerk/protocol/http/SpdySession31.h b/netwerk/protocol/http/SpdySession31.h index 595b06f4b96..4b153fab085 100644 --- a/netwerk/protocol/http/SpdySession31.h +++ b/netwerk/protocol/http/SpdySession31.h @@ -134,7 +134,6 @@ public: const static uint32_t kQueueTailRoom = 4096; const static uint32_t kQueueReserved = 1024; - const static uint32_t kDefaultMaxConcurrent = 100; const static uint32_t kMaxStreamID = 0x7800000; // This is a sentinel for a deleted stream. It is not a valid diff --git a/netwerk/protocol/http/nsHttpHandler.cpp b/netwerk/protocol/http/nsHttpHandler.cpp index ac30c27851a..71c1331bd33 100644 --- a/netwerk/protocol/http/nsHttpHandler.cpp +++ b/netwerk/protocol/http/nsHttpHandler.cpp @@ -194,6 +194,7 @@ nsHttpHandler::nsHttpHandler() , mSpdySendingChunkSize(ASpdySession::kSendingChunkSize) , mSpdySendBufferSize(ASpdySession::kTCPSendBufferSize) , mSpdyPushAllowance(32768) + , mDefaultSpdyConcurrent(ASpdySession::kDefaultMaxConcurrent) , mSpdyPingThreshold(PR_SecondsToInterval(58)) , mSpdyPingTimeout(PR_SecondsToInterval(8)) , mConnectTimeout(90000) @@ -1287,6 +1288,14 @@ nsHttpHandler::PrefsChanged(nsIPrefBranch *prefs, const char *pref) } } + if (PREF_CHANGED(HTTP_PREF("spdy.default-concurrent"))) { + rv = prefs->GetIntPref(HTTP_PREF("spdy.default-concurrent"), &val); + if (NS_SUCCEEDED(rv)) { + mDefaultSpdyConcurrent = + static_cast(std::max(std::min(val, 9999), 1)); + } + } + // The amount of seconds to wait for a spdy ping response before // closing the session. if (PREF_CHANGED(HTTP_PREF("spdy.send-buffer-size"))) { diff --git a/netwerk/protocol/http/nsHttpHandler.h b/netwerk/protocol/http/nsHttpHandler.h index 0d4a41f258a..78273ee9862 100644 --- a/netwerk/protocol/http/nsHttpHandler.h +++ b/netwerk/protocol/http/nsHttpHandler.h @@ -105,6 +105,7 @@ public: uint32_t SpdySendingChunkSize() { return mSpdySendingChunkSize; } uint32_t SpdySendBufferSize() { return mSpdySendBufferSize; } uint32_t SpdyPushAllowance() { return mSpdyPushAllowance; } + uint32_t DefaultSpdyConcurrent() { return mDefaultSpdyConcurrent; } PRIntervalTime SpdyPingThreshold() { return mSpdyPingThreshold; } PRIntervalTime SpdyPingTimeout() { return mSpdyPingTimeout; } bool AllowPush() { return mAllowPush; } @@ -487,6 +488,7 @@ private: uint32_t mSpdySendingChunkSize; uint32_t mSpdySendBufferSize; uint32_t mSpdyPushAllowance; + uint32_t mDefaultSpdyConcurrent; PRIntervalTime mSpdyPingThreshold; PRIntervalTime mSpdyPingTimeout; From 82f88215c7d523a41a1f9f26d8da278de28955ca Mon Sep 17 00:00:00 2001 From: Patrick McManus Date: Mon, 26 Jan 2015 16:41:47 -0500 Subject: [PATCH 14/53] bug 1124717 - 4/4 test r=hurley --- netwerk/test/unit/test_http2.js | 34 +++++++++++++++++++++++++ testing/xpcshell/moz-http2/moz-http2.js | 23 +++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/netwerk/test/unit/test_http2.js b/netwerk/test/unit/test_http2.js index 91b14be3084..8311a5af6e2 100644 --- a/netwerk/test/unit/test_http2.js +++ b/netwerk/test/unit/test_http2.js @@ -15,6 +15,7 @@ function generateContent(size) { var posts = []; posts.push(generateContent(10)); posts.push(generateContent(250000)); +posts.push(generateContent(128000)); // pre-calculated md5sums (in hex) of the above posts var md5s = ['f1b708bba17f1ce948dc979f4d7092bc', @@ -277,11 +278,20 @@ var Http2ConcurrentListener = function() {}; Http2ConcurrentListener.prototype = new Http2CheckListener(); Http2ConcurrentListener.prototype.count = 0; Http2ConcurrentListener.prototype.target = 0; +Http2ConcurrentListener.prototype.reset = 0; +Http2ConcurrentListener.prototype.recvdHdr = 0; Http2ConcurrentListener.prototype.onStopRequest = function(request, ctx, status) { this.count++; do_check_true(this.isHttp2Connection); + if (this.recvdHdr > 0) { + do_check_eq(request.getResponseHeader("X-Recvd"), this.recvdHdr); + } + if (this.count == this.target) { + if (this.reset > 0) { + prefs.setIntPref("network.http.spdy.default-concurrent", this.reset); + } run_next_test(); do_test_finished(); } @@ -290,6 +300,9 @@ Http2ConcurrentListener.prototype.onStopRequest = function(request, ctx, status) function test_http2_concurrent() { var concurrent_listener = new Http2ConcurrentListener(); concurrent_listener.target = 201; + concurrent_listener.reset = prefs.getIntPref("network.http.spdy.default-concurrent"); + prefs.setIntPref("network.http.spdy.default-concurrent", 100); + for (var i = 0; i < concurrent_listener.target; i++) { concurrent_channels[i] = makeChan("https://localhost:" + serverPort + "/750ms"); concurrent_channels[i].loadFlags = Ci.nsIRequest.LOAD_BYPASS_CACHE; @@ -297,6 +310,26 @@ function test_http2_concurrent() { } } +function test_http2_concurrent_post() { + var concurrent_listener = new Http2ConcurrentListener(); + concurrent_listener.target = 8; + concurrent_listener.recvdHdr = posts[2].length; + concurrent_listener.reset = prefs.getIntPref("network.http.spdy.default-concurrent"); + prefs.setIntPref("network.http.spdy.default-concurrent", 3); + + for (var i = 0; i < concurrent_listener.target; i++) { + concurrent_channels[i] = makeChan("https://localhost:" + serverPort + "/750msPost"); + concurrent_channels[i].loadFlags = Ci.nsIRequest.LOAD_BYPASS_CACHE; + var stream = Cc["@mozilla.org/io/string-input-stream;1"] + .createInstance(Ci.nsIStringInputStream); + stream.data = posts[2]; + var uchan = concurrent_channels[i].QueryInterface(Ci.nsIUploadChannel); + uchan.setUploadStream(stream, "text/plain", stream.available()); + concurrent_channels[i].requestMethod = "POST"; + concurrent_channels[i].asyncOpen(concurrent_listener, null); + } +} + // Test to make sure we get multiplexing right function test_http2_multiplex() { var chan1 = makeChan("https://localhost:" + serverPort + "/multiplex1"); @@ -625,6 +658,7 @@ function test_complete() { var tests = [ test_http2_post_big , test_http2_basic , test_http2_concurrent + , test_http2_concurrent_post , test_http2_basic_unblocked_dep , test_http2_nospdy , test_http2_push1 diff --git a/testing/xpcshell/moz-http2/moz-http2.js b/testing/xpcshell/moz-http2/moz-http2.js index 517beef1674..f6242bd1fd0 100644 --- a/testing/xpcshell/moz-http2/moz-http2.js +++ b/testing/xpcshell/moz-http2/moz-http2.js @@ -283,6 +283,29 @@ function handleRequest(req, res) { return; } + else if (u.pathname === "/750msPost") { + if (req.method != "POST") { + res.writeHead(405); + res.end('Unexpected method: ' + req.method); + return; + } + + var accum = 0; + req.on('data', function receivePostData(chunk) { + accum += chunk.length; + }); + req.on('end', function finishPost() { + res.setHeader('X-Recvd', accum); + var rl = new runlater(); + rl.req = req; + rl.resp = res; + setTimeout(executeRunLater, 750, rl); + return; + }); + + return; + } + else if (u.pathname === "/h11required_stream") { if (req.httpVersionMajor === 2) { h11required_conn = req.stream.connection; From 6354675424f29884d68223ae08a3b95749e2e165 Mon Sep 17 00:00:00 2001 From: Sotaro Ikeda Date: Wed, 28 Jan 2015 06:31:31 -0800 Subject: [PATCH 15/53] Bug 1123452 - Try to enter dormant state when document is hidden r=cpearce --- dom/media/MediaDecoder.cpp | 106 +++++++++++++++++++++++++++++++++---- dom/media/MediaDecoder.h | 23 ++++++++ 2 files changed, 120 insertions(+), 9 deletions(-) diff --git a/dom/media/MediaDecoder.cpp b/dom/media/MediaDecoder.cpp index 9688688f645..dcd5260963f 100644 --- a/dom/media/MediaDecoder.cpp +++ b/dom/media/MediaDecoder.cpp @@ -36,6 +36,9 @@ using namespace mozilla::layers; using namespace mozilla::dom; +// Default timeout msecs until try to enter dormant state by heuristic. +static const int DEFAULT_HEURISTIC_DORMANT_TIMEOUT_MSECS = 60000; + namespace mozilla { // Number of estimated seconds worth of data we need to have buffered @@ -123,27 +126,56 @@ void MediaDecoder::NotifyOwnerActivityChanged() MOZ_ASSERT(NS_IsMainThread()); ReentrantMonitorAutoEnter mon(GetReentrantMonitor()); - if (!mDecoderStateMachine || - !mDecoderStateMachine->IsDormantNeeded() || - mPlayState == PLAY_STATE_SHUTDOWN) { - return; - } - if (!mOwner) { NS_WARNING("MediaDecoder without a decoder owner, can't update dormant"); return; } + UpdateDormantState(false /* aDormantTimeout */, false /* aActivity */); + // Start dormant timer if necessary + StartDormantTimer(); +} + +void MediaDecoder::UpdateDormantState(bool aDormantTimeout, bool aActivity) +{ + MOZ_ASSERT(NS_IsMainThread()); + GetReentrantMonitor().AssertCurrentThreadIn(); + + if (!mDecoderStateMachine || + mPlayState == PLAY_STATE_SHUTDOWN || + !mOwner->GetVideoFrameContainer() || + !mDecoderStateMachine->IsDormantNeeded()) + { + return; + } + bool prevDormant = mIsDormant; mIsDormant = false; - if (!mOwner->IsActive() && mOwner->GetVideoFrameContainer()) { + if (!mOwner->IsActive()) { mIsDormant = true; } #ifdef MOZ_WIDGET_GONK - if (mOwner->IsHidden() && mOwner->GetVideoFrameContainer()) { + if (mOwner->IsHidden()) { mIsDormant = true; } #endif + // Try to enable dormant by idle heuristic, when the owner is hidden. + bool prevHeuristicDormant = mIsHeuristicDormant; + mIsHeuristicDormant = false; + if (mIsHeuristicDormantSupported && mOwner->IsHidden()) { + if (aDormantTimeout && !aActivity && + (mPlayState == PLAY_STATE_PAUSED || mPlayState == PLAY_STATE_ENDED)) { + // Enable heuristic dormant + mIsHeuristicDormant = true; + } else if(prevHeuristicDormant && !aActivity) { + // Continue heuristic dormant + mIsHeuristicDormant = true; + } + + if (mIsHeuristicDormant) { + mIsDormant = true; + } + } if (prevDormant == mIsDormant) { // No update to dormant state @@ -167,6 +199,47 @@ void MediaDecoder::NotifyOwnerActivityChanged() } } +void MediaDecoder::DormantTimerExpired(nsITimer* aTimer, void* aClosure) +{ + MOZ_ASSERT(aClosure); + MediaDecoder* decoder = static_cast(aClosure); + ReentrantMonitorAutoEnter mon(decoder->GetReentrantMonitor()); + decoder->UpdateDormantState(true /* aDormantTimeout */, + false /* aActivity */); +} + +void MediaDecoder::StartDormantTimer() +{ + if (!mIsHeuristicDormantSupported) { + return; + } + + if (mIsHeuristicDormant || + mShuttingDown || + !mOwner || + !mOwner->IsHidden() || + (mPlayState != PLAY_STATE_PAUSED && + mPlayState != PLAY_STATE_ENDED)) + { + return; + } + + if (!mDormantTimer) { + mDormantTimer = do_CreateInstance("@mozilla.org/timer;1"); + } + mDormantTimer->InitWithFuncCallback(&MediaDecoder::DormantTimerExpired, + this, + mHeuristicDormantTimeout, + nsITimer::TYPE_ONE_SHOT); +} + +void MediaDecoder::CancelDormantTimer() +{ + if (mDormantTimer) { + mDormantTimer->Cancel(); + } +} + void MediaDecoder::Pause() { MOZ_ASSERT(NS_IsMainThread()); @@ -472,7 +545,13 @@ MediaDecoder::MediaDecoder() : mPausedForPlaybackRateNull(false), mMinimizePreroll(false), mMediaTracksConstructed(false), - mIsDormant(false) + mIsDormant(false), + mIsHeuristicDormantSupported( + Preferences::GetBool("media.decoder.heuristic.dormant.enabled", false)), + mHeuristicDormantTimeout( + Preferences::GetInt("media.decoder.heuristic.dormant.timeout", + DEFAULT_HEURISTIC_DORMANT_TIMEOUT_MSECS)), + mIsHeuristicDormant(false) { MOZ_COUNT_CTOR(MediaDecoder); MOZ_ASSERT(NS_IsMainThread()); @@ -521,6 +600,8 @@ void MediaDecoder::Shutdown() mResource->Close(); } + CancelDormantTimer(); + ChangeState(PLAY_STATE_SHUTDOWN); mOwner = nullptr; @@ -622,6 +703,8 @@ nsresult MediaDecoder::Play() { MOZ_ASSERT(NS_IsMainThread()); ReentrantMonitorAutoEnter mon(GetReentrantMonitor()); + UpdateDormantState(false /* aDormantTimeout */, true /* aActivity */); + NS_ASSERTION(mDecoderStateMachine != nullptr, "Should have state machine."); if (mPausedForPlaybackRateNull) { return NS_OK; @@ -644,6 +727,7 @@ nsresult MediaDecoder::Seek(double aTime, SeekTarget::Type aSeekType) { MOZ_ASSERT(NS_IsMainThread()); ReentrantMonitorAutoEnter mon(GetReentrantMonitor()); + UpdateDormantState(false /* aDormantTimeout */, true /* aActivity */); NS_ABORT_IF_FALSE(aTime >= 0.0, "Cannot seek to a negative value."); @@ -1192,6 +1276,10 @@ void MediaDecoder::ChangeState(PlayState aState) ApplyStateToStateMachine(mPlayState); + CancelDormantTimer(); + // Start dormant timer if necessary + StartDormantTimer(); + GetReentrantMonitor().NotifyAll(); } diff --git a/dom/media/MediaDecoder.h b/dom/media/MediaDecoder.h index 33fc2f41d9a..eae936b49f7 100644 --- a/dom/media/MediaDecoder.h +++ b/dom/media/MediaDecoder.h @@ -188,6 +188,7 @@ destroying the MediaDecoder object. #include "nsCOMPtr.h" #include "nsIObserver.h" #include "nsAutoPtr.h" +#include "nsITimer.h" #include "MediaResource.h" #include "mozilla/dom/AudioChannelBinding.h" #include "mozilla/gfx/Rect.h" @@ -367,6 +368,8 @@ public: // It is used to share scarece media resources in system. virtual void NotifyOwnerActivityChanged(); + void UpdateDormantState(bool aDormantTimeout, bool aActivity); + // Pause video playback. virtual void Pause(); // Adjust the speed of the playback, optionally with pitch correction, @@ -1022,6 +1025,14 @@ protected: virtual ~MediaDecoder(); void SetStateMachineParameters(); + static void DormantTimerExpired(nsITimer *aTimer, void *aClosure); + + // Start a timer for heuristic dormant. + void StartDormantTimer(); + + // Cancel a timer for heuristic dormant. + void CancelDormantTimer(); + /****** * The following members should be accessed with the decoder lock held. ******/ @@ -1219,6 +1230,18 @@ protected: // True if MediaDecoder is in dormant state. bool mIsDormant; + + // True if heuristic dormant is supported. + const bool mIsHeuristicDormantSupported; + + // Timeout ms of heuristic dormant timer. + const int mHeuristicDormantTimeout; + + // True if MediaDecoder is in dormant by heuristic. + bool mIsHeuristicDormant; + + // Timer to schedule updating dormant state. + nsCOMPtr mDormantTimer; }; } // namespace mozilla From 9822c711da828b78648456d937bedb3d0ea38e16 Mon Sep 17 00:00:00 2001 From: Jeff Muizelaar Date: Tue, 27 Jan 2015 20:09:34 -0500 Subject: [PATCH 16/53] Bug 1124847. Track D3D11 shared texture usage in about:memory. r=bas This will hopefully help us track down large amounts of write-combined mappings that we see. --- gfx/layers/d3d11/TextureD3D11.cpp | 57 +++++++++++++++++++++++++++++++ gfx/thebes/gfxWindowsPlatform.cpp | 20 +++++++++++ gfx/thebes/gfxWindowsPlatform.h | 3 ++ 3 files changed, 80 insertions(+) diff --git a/gfx/layers/d3d11/TextureD3D11.cpp b/gfx/layers/d3d11/TextureD3D11.cpp index c7be1fe8f68..af7884e3660 100644 --- a/gfx/layers/d3d11/TextureD3D11.cpp +++ b/gfx/layers/d3d11/TextureD3D11.cpp @@ -364,6 +364,54 @@ TextureClientD3D11::BorrowDrawTarget() return mDrawTarget; } +static const GUID sD3D11TextureUsage = +{ 0xd89275b0, 0x6c7d, 0x4038, { 0xb5, 0xfa, 0x4d, 0x87, 0x16, 0xd5, 0xcc, 0x4e } }; + +/* This class get's it's lifetime tied to a D3D texture + * and increments memory usage on construction and decrements + * on destruction */ +class TextureMemoryMeasurer : public IUnknown +{ +public: + TextureMemoryMeasurer(size_t aMemoryUsed) + { + mMemoryUsed = aMemoryUsed; + gfxWindowsPlatform::sD3D11MemoryUsed += mMemoryUsed; + mRefCnt = 0; + } + STDMETHODIMP_(ULONG) AddRef() { + mRefCnt++; + return mRefCnt; + } + STDMETHODIMP QueryInterface(REFIID riid, + void **ppvObject) + { + IUnknown *punk = nullptr; + if (riid == IID_IUnknown) { + punk = this; + } + *ppvObject = punk; + if (punk) { + punk->AddRef(); + return S_OK; + } else { + return E_NOINTERFACE; + } + } + + STDMETHODIMP_(ULONG) Release() { + int refCnt = --mRefCnt; + if (refCnt == 0) { + gfxWindowsPlatform::sD3D11MemoryUsed -= mMemoryUsed; + delete this; + } + return refCnt; + } +private: + int mRefCnt; + int mMemoryUsed; +}; + bool TextureClientD3D11::AllocateForSurface(gfx::IntSize aSize, TextureAllocationFlags aFlags) { @@ -386,6 +434,10 @@ TextureClientD3D11::AllocateForSurface(gfx::IntSize aSize, TextureAllocationFlag newDesc.MiscFlags = D3D11_RESOURCE_MISC_SHARED; hr = d3d11device->CreateTexture2D(&newDesc, nullptr, byRef(mTexture)); + mTexture->SetPrivateDataInterface(sD3D11TextureUsage, + new TextureMemoryMeasurer(newDesc.Width * newDesc.Height * + (mFormat == SurfaceFormat::A8 ? + 1 : 4))); } else { ID3D10Device* device = gfxWindowsPlatform::GetPlatform()->GetD3D10Device(); @@ -397,8 +449,13 @@ TextureClientD3D11::AllocateForSurface(gfx::IntSize aSize, TextureAllocationFlag newDesc.MiscFlags = D3D10_RESOURCE_MISC_SHARED; hr = device->CreateTexture2D(&newDesc, nullptr, byRef(mTexture10)); + mTexture10->SetPrivateDataInterface(sD3D11TextureUsage, + new TextureMemoryMeasurer(newDesc.Width * newDesc.Height * + (mFormat == SurfaceFormat::A8 ? + 1 : 4))); } + if (FAILED(hr)) { gfxCriticalError(CriticalLog::DefaultOptions(Factory::ReasonableSurfaceSize(aSize))) << "[D3D11] 2 CreateTexture2D failure " << aSize << " Code: " << gfx::hexa(hr); return false; diff --git a/gfx/thebes/gfxWindowsPlatform.cpp b/gfx/thebes/gfxWindowsPlatform.cpp index 317cc3d5b98..d3c6871795c 100644 --- a/gfx/thebes/gfxWindowsPlatform.cpp +++ b/gfx/thebes/gfxWindowsPlatform.cpp @@ -325,6 +325,25 @@ public: NS_IMPL_ISUPPORTS(GPUAdapterReporter, nsIMemoryReporter) + +Atomic gfxWindowsPlatform::sD3D11MemoryUsed; + +class D3D11TextureReporter MOZ_FINAL : public nsIMemoryReporter +{ +public: + NS_DECL_ISUPPORTS + + NS_IMETHOD CollectReports(nsIHandleReportCallback *aHandleReport, + nsISupports* aData, bool aAnonymize) MOZ_OVERRIDE + { + return MOZ_COLLECT_REPORT("d3d11-shared-textures", KIND_OTHER, UNITS_BYTES, + gfxWindowsPlatform::sD3D11MemoryUsed, + "Memory used for D3D11 shared textures"); + } +}; + +NS_IMPL_ISUPPORTS(D3D11TextureReporter, nsIMemoryReporter) + gfxWindowsPlatform::gfxWindowsPlatform() : mD3D11DeviceInitialized(false) , mIsWARP(false) @@ -352,6 +371,7 @@ gfxWindowsPlatform::gfxWindowsPlatform() UpdateRenderMode(); RegisterStrongMemoryReporter(new GPUAdapterReporter()); + RegisterStrongMemoryReporter(new D3D11TextureReporter()); } gfxWindowsPlatform::~gfxWindowsPlatform() diff --git a/gfx/thebes/gfxWindowsPlatform.h b/gfx/thebes/gfxWindowsPlatform.h index 3ded93a6c18..b5d07b81845 100644 --- a/gfx/thebes/gfxWindowsPlatform.h +++ b/gfx/thebes/gfxWindowsPlatform.h @@ -22,6 +22,7 @@ #include "gfxPlatform.h" #include "gfxTypes.h" #include "mozilla/Attributes.h" +#include "mozilla/Atomics.h" #include "nsTArray.h" #include "nsDataHashtable.h" @@ -252,6 +253,8 @@ public: bool IsWARP() { return mIsWARP; } + static mozilla::Atomic sD3D11MemoryUsed; + protected: RenderMode mRenderMode; From c6ce6a53fd9e786b0ad32c0668a4c108ccffcc9a Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Sat, 17 Jan 2015 17:30:21 -0500 Subject: [PATCH 17/53] Bug 1119503 - Part 1: Determine whether an element is a block element based on the style, not the tag; r=bzbarsky This probably fixes a whole bunch of edge cases where content uses elements other than div. --- dom/base/nsPlainTextSerializer.cpp | 19 ++++++++++++++++--- dom/base/nsPlainTextSerializer.h | 1 + 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/dom/base/nsPlainTextSerializer.cpp b/dom/base/nsPlainTextSerializer.cpp index be3a3b26e96..aaf5d44ee39 100644 --- a/dom/base/nsPlainTextSerializer.cpp +++ b/dom/base/nsPlainTextSerializer.cpp @@ -670,7 +670,7 @@ nsPlainTextSerializer::DoOpenContainer(nsIAtom* aTag) // Else make sure we'll separate block level tags, // even if we're about to leave, before doing any other formatting. - else if (nsContentUtils::IsHTMLBlock(aTag)) { + else if (IsElementBlock(mElement)) { EnsureVerticalSpace(0); } @@ -887,8 +887,7 @@ nsPlainTextSerializer::DoCloseContainer(nsIAtom* aTag) else if (aTag == nsGkAtoms::q) { Write(NS_LITERAL_STRING("\"")); } - else if (nsContentUtils::IsHTMLBlock(aTag) - && aTag != nsGkAtoms::script) { + else if (IsElementBlock(mElement) && aTag != nsGkAtoms::script) { // All other blocks get 1 vertical space after them // in formatted mode, otherwise 0. // This is hard. Sometimes 0 is a better number, but @@ -1778,6 +1777,20 @@ nsPlainTextSerializer::IsElementPreformatted(Element* aElement) return GetIdForContent(aElement) == nsGkAtoms::pre; } +bool +nsPlainTextSerializer::IsElementBlock(Element* aElement) +{ + nsRefPtr styleContext = + nsComputedDOMStyle::GetStyleContextForElementNoFlush(aElement, nullptr, + nullptr); + if (styleContext) { + const nsStyleDisplay* displayStyle = styleContext->StyleDisplay(); + return displayStyle->IsBlockOutsideStyle(); + } + // Fall back to looking at the tag, in case there is no style information. + return nsContentUtils::IsHTMLBlock(GetIdForContent(aElement)); +} + /** * This method is required only to identify LI's inside OL. * Returns TRUE if we are inside an OL tag and FALSE otherwise. diff --git a/dom/base/nsPlainTextSerializer.h b/dom/base/nsPlainTextSerializer.h index 479272a76ad..ea2d049ca9a 100644 --- a/dom/base/nsPlainTextSerializer.h +++ b/dom/base/nsPlainTextSerializer.h @@ -115,6 +115,7 @@ private: bool ShouldReplaceContainerWithPlaceholder(nsIAtom* aTag); bool IsElementPreformatted(mozilla::dom::Element* aElement); + bool IsElementBlock(mozilla::dom::Element* aElement); private: nsString mCurrentLine; From 7fdf2ea7a1f5f0062c335e924837cb9fd9e6da48 Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Sat, 17 Jan 2015 17:31:59 -0500 Subject: [PATCH 18/53] Bug 1119503 - Part 2: Insert a line break between preformatted block boundaries when creating raw output; r=bzbarsky --- dom/base/nsCopySupport.cpp | 3 ++- dom/base/nsIDocumentEncoder.idl | 7 +++++++ dom/base/nsPlainTextSerializer.cpp | 24 ++++++++++++++++++++++++ dom/base/nsPlainTextSerializer.h | 4 +++- dom/base/test/test_bug116083.html | 8 ++++++++ 5 files changed, 44 insertions(+), 2 deletions(-) diff --git a/dom/base/nsCopySupport.cpp b/dom/base/nsCopySupport.cpp index 56ac20eb145..3f6db3928da 100644 --- a/dom/base/nsCopySupport.cpp +++ b/dom/base/nsCopySupport.cpp @@ -97,7 +97,8 @@ SelectionCopyHelper(nsISelection *aSel, nsIDocument *aDoc, // Do the first and potentially trial encoding as preformatted and raw. uint32_t flags = aFlags | nsIDocumentEncoder::OutputPreformatted - | nsIDocumentEncoder::OutputRaw; + | nsIDocumentEncoder::OutputRaw + | nsIDocumentEncoder::OutputForPlainTextClipboardCopy; nsCOMPtr domDoc = do_QueryInterface(aDoc); NS_ASSERTION(domDoc, "Need a document"); diff --git a/dom/base/nsIDocumentEncoder.idl b/dom/base/nsIDocumentEncoder.idl index b5963bb2db8..155e5c375c6 100644 --- a/dom/base/nsIDocumentEncoder.idl +++ b/dom/base/nsIDocumentEncoder.idl @@ -227,6 +227,13 @@ interface nsIDocumentEncoder : nsISupports */ const unsigned long OutputDontRemoveLineEndingSpaces = (1 << 24); + /** + * Serialize in a way that is suitable for copying a plaintext version of the + * document to the clipboard. This can for example cause line endings to be + * injected at preformatted block element boundaries. + */ + const unsigned long OutputForPlainTextClipboardCopy = (1 << 25); + /** * Initialize with a pointer to the document and the mime type. * @param aDocument Document to encode. diff --git a/dom/base/nsPlainTextSerializer.cpp b/dom/base/nsPlainTextSerializer.cpp index aaf5d44ee39..eb7e4b1cff4 100644 --- a/dom/base/nsPlainTextSerializer.cpp +++ b/dom/base/nsPlainTextSerializer.cpp @@ -91,6 +91,8 @@ nsPlainTextSerializer::nsPlainTextSerializer() mPreFormatted = false; mStartedOutput = false; + mPreformattedBlockBoundary = false; + // initialize the tag stack to zero: // The stack only ever contains pointers to static atoms, so they don't // need refcounting. @@ -167,6 +169,8 @@ nsPlainTextSerializer::Init(uint32_t aFlags, uint32_t aWrapColumn, mLineBreakDue = false; mFloatingLines = -1; + mPreformattedBlockBoundary = false; + if (mFlags & nsIDocumentEncoder::OutputFormatted) { // Get some prefs that controls how we do formatted output mStructs = Preferences::GetBool(PREF_STRUCTS, mStructs); @@ -437,6 +441,16 @@ nsPlainTextSerializer::DoOpenContainer(nsIAtom* aTag) return NS_OK; } + if (mFlags & nsIDocumentEncoder::OutputForPlainTextClipboardCopy) { + if (mPreformattedBlockBoundary && DoOutput()) { + // Should always end a line, but get no more whitespace + if (mFloatingLines < 0) + mFloatingLines = 0; + mLineBreakDue = true; + } + mPreformattedBlockBoundary = false; + } + if (mFlags & nsIDocumentEncoder::OutputRaw) { // Raw means raw. Don't even think about doing anything fancy // here like indenting, adding line breaks or any other @@ -767,6 +781,14 @@ nsPlainTextSerializer::DoCloseContainer(nsIAtom* aTag) return NS_OK; } + if (mFlags & nsIDocumentEncoder::OutputForPlainTextClipboardCopy) { + if (DoOutput() && IsInPre() && IsElementBlock(mElement)) { + // If we're closing a preformatted block element, output a line break + // when we find a new container. + mPreformattedBlockBoundary = true; + } + } + if (mFlags & nsIDocumentEncoder::OutputRaw) { // Raw means raw. Don't even think about doing anything fancy // here like indenting, adding line breaks or any other @@ -1036,6 +1058,8 @@ nsPlainTextSerializer::DoAddText(bool aIsLineBreak, const nsAString& aText) nsresult nsPlainTextSerializer::DoAddLeaf(nsIAtom* aTag) { + mPreformattedBlockBoundary = false; + // If we don't want any output, just return if (!DoOutput()) { return NS_OK; diff --git a/dom/base/nsPlainTextSerializer.h b/dom/base/nsPlainTextSerializer.h index ea2d049ca9a..9535a59d9d5 100644 --- a/dom/base/nsPlainTextSerializer.h +++ b/dom/base/nsPlainTextSerializer.h @@ -170,7 +170,9 @@ private: // While handling a new tag, this variable should remind if any line break // is due because of a closing tag. Setting it to "TRUE" while closing the tags. // Hence opening tags are guaranteed to start with appropriate line breaks. - bool mLineBreakDue; + bool mLineBreakDue; + + bool mPreformattedBlockBoundary; nsString mURL; int32_t mHeaderStrategy; /* Header strategy (pref) diff --git a/dom/base/test/test_bug116083.html b/dom/base/test/test_bug116083.html index 4244a13aef6..5ac1181536e 100644 --- a/dom/base/test/test_bug116083.html +++ b/dom/base/test/test_bug116083.html @@ -20,6 +20,14 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=116083
bar baz
bar baz
bar baz
+
foo
bar

!


baz
+
foo
bar

!


baz
+
foo
bar

!


baz
+
foo
bar

!


baz
+
foo
bar

!


baz
+
foo
bar

!


baz
+
foo
bar

!


baz
+
foo
bar

!


baz
foo bar